Skip to content

transport/http: make inbound header preallocation configurable - #2536

Merged
bananacocodrilo merged 5 commits into
mainfrom
wt/header-preallocation-strategy
Aug 26, 2026
Merged

transport/http: make inbound header preallocation configurable#2536
bananacocodrilo merged 5 commits into
mainfrom
wt/header-preallocation-strategy

Conversation

@bananacocodrilo

@bananacocodrilo bananacocodrilo commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Description

Intent:

  • Preserve PR transport/http: preallocate inbound headers #2521's fast post-system-header sizing as the default while giving services an escape hatch for requests dominated by unrelated HTTP headers.
  • Let downstream services choose the CPU and memory tradeoff that matches their inbound header mix.

Changes:

  • Added unfiltered, scan, and disabled inbound header preallocation strategies through the Go API and HTTP transport configuration.
  • Made the configuration field use the typed HeaderPreallocationStrategy enum while preserving omitted-versus-explicit configuration semantics.
  • Made unfiltered the default; it uses the unfiltered number of request headers remaining after YARPC system headers are removed.
  • Made the opt-in scan count eligible application, tracing, proxy, and grabbed HTTP header forms.
  • Intentionally count duplicate raw and prefixed forms separately, allowing a small capacity overestimate instead of adding collision tracking or a quadratic scan.
  • Removed the small-header threshold: scanning has already paid the counting cost, and benchmarks showed capacity zero and capacity four both used 744 B/op and 8 allocs/op.
  • Precomputed grabbed-header canonical keys per inbound so scanning adds no per-request allocations.

Test Plan

  • go test ./transport/http
  • aifx verify
  • go test ./transport/http -run '^$' -bench '^BenchmarkFromHTTPHeadersCapacity' -benchmem -benchtime=200ms -count=3

Revert Plan

Revert this PR to restore PR #2521's unconditional post-system-header request-header-count preallocation.

Issues

T3-RPC-11408

RELEASE NOTES:
Added configurable inbound HTTP header preallocation strategies.

Summary:
Intent:
- Preserve the default post-system-header sizing from PR #2521 while giving header-heavy services an escape hatch for requests dominated by unrelated HTTP headers.
- Let downstream services choose the CPU and memory tradeoff that matches their inbound header mix.

Changes:
- Added remaining, scan, and disabled inbound header preallocation strategies through the Go API and HTTP transport configuration.
- Made the opt-in scan count unique application, tracing, proxy, and grabbed headers, including collision and minimum-threshold handling.
- Precomputed grabbed-header lookup keys per inbound so scanning adds no per-request allocations, with focused tests and benchmarks for each strategy.

Jira Issues: T3-RPC-11408

---

<sub>Generated by the 🪄 [pr-create](https://sg.uberinternal.com/code.uber.internal/uber-code/devexp-agent-marketplace/-/blob/claude-code/plugins/dev/uber-dev/skills/pr-create/SKILL.md) skill in devexp-agent-marketplace</sub>
@bananacocodrilo
bananacocodrilo marked this pull request as ready for review August 19, 2026 11:20
Comment thread transport/http/handler.go Outdated
Comment thread transport/http/handler.go Outdated
grabHeaders []headerPreallocationGrabHeader,
) int {
switch strategy {
case "", HeaderPreallocationUnfiltered:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this "" option for?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok because it is the default option

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm removing it and making it so "no config" gets set to unfiltered to make it more clear

Comment thread transport/http/handler.go Outdated
grabHeaders []headerPreallocationGrabHeader,
) int {
switch strategy {
case "", HeaderPreallocationUnfiltered:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok because it is the default option

Comment thread transport/http/handler.go Outdated
Comment thread transport/http/handler.go Outdated
}
}

// Raw and prefixed forms can produce the same canonical transport key.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we move this comment to line 108?

Comment thread transport/http/handler.go Outdated
if len(values) > 0 {
return values[0]
}
return ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there possibility that value for canonicalKey is empty, but not for lowerKey? In this case we bypass the lowerKey one

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was!

Comment thread transport/http/handler.go
ttl := popHeader(req.Header, TTLMSHeader)

transportHeader := transport.NewHeadersWithCapacity(len(req.Header))
transportHeader := transport.NewHeadersWithCapacity(inboundHeaderCapacity(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inboundHeaderCapacity returns 0 for Disabled option, and we end up having an empty map without item or originalItem keys.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transport.Headers.With lazily allocates both maps when the first header is added. No headers ends with no allocations.

@bananacocodrilo
bananacocodrilo force-pushed the wt/header-preallocation-strategy branch from 24b179c to f096634 Compare August 26, 2026 13:59
@bananacocodrilo
bananacocodrilo requested a review from maryis August 26, 2026 13:59
@bananacocodrilo
bananacocodrilo merged commit 1ae4863 into main Aug 26, 2026
2 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants