Remove redundant CanonicalizeHeaderKey from metadata iteration paths - #2443
Open
kshitijsuri90 wants to merge 2 commits into
Open
Remove redundant CanonicalizeHeaderKey from metadata iteration paths#2443kshitijsuri90 wants to merge 2 commits into
kshitijsuri90 wants to merge 2 commits into
Conversation
kshitijsuri90
force-pushed
the
kshitij/remove-canonicalize-metadata
branch
from
April 8, 2026 12:28
bb5c41a to
fc376ba
Compare
| headers := transport.NewHeadersWithCapacity(md.Len()) | ||
| for header, values := range md { | ||
| header = transport.CanonicalizeHeaderKey(header) | ||
| // gRPC metadata keys are already lowercase. |
There was a problem hiding this comment.
nit: comment is a bit confusing for first time readers, maybe rephrase as method comment, saying, "this function assumes incoming headers are lowercase because ..."
MahammadAgayev
approved these changes
Jun 19, 2026
kshitijsuri90
force-pushed
the
kshitij/remove-canonicalize-metadata
branch
from
July 27, 2026 08:32
fc376ba to
8ef7dcc
Compare
rabbbit
approved these changes
Jul 29, 2026
Problem: metadataToTransportRequest and getApplicationHeaders both call transport.CanonicalizeHeaderKey (strings.ToLower) on every key while iterating gRPC metadata.MD. These keys are already guaranteed to be lowercase, making the calls redundant CPU work on every inbound and response path. Fix: Remove the CanonicalizeHeaderKey call from both functions. Also normalize a test case in TestGetApplicationHeaders that used a mixed-case metadata key via a raw metadata.MD literal, which no longer reflects a realistic scenario. Safety — three independent guarantees that metadata keys are lowercase: 1. HTTP/2 spec (RFC 7540 s8.1.2): header field names MUST be converted to lowercase prior to their encoding in HTTP/2. https://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2 2. grpc-go PR #4416 (merged v1.39, June 2021): FromIncomingContext now explicitly lowercases all keys. grpc/grpc-go#4416 3. yarpc-go uses grpc-go v1.67.3 (go.mod), well past v1.39. Call sites: - metadataToTransportRequest: called from handler.go via metadata.FromIncomingContext (server inbound path) - getApplicationHeaders: called from outbound.go with response trailer metadata (client response path) Impact: Eliminates one strings.ToLower scan per metadata entry on both the server inbound and client response paths. Benchmark (count=6, AMD EPYC 9B45): MetadataToTransportRequest: 984.8 ns/op → 907.7 ns/op (-7.83%, p=0.002) GetApplicationHeaders: 992.0 ns/op → 895.9 ns/op (-9.69%, p=0.002) Rebase note (2026-07-26): rebased onto current main; resolved a positional conflict in transport/grpc/headers_test.go where main's newly-added BenchmarkIsReserved and this PR's benchmarks were both appended at the same spot — kept both functions. Made-with: Cursor
kshitijsuri90
force-pushed
the
kshitij/remove-canonicalize-metadata
branch
from
August 18, 2026 09:34
8ef7dcc to
fc90a11
Compare
bananacocodrilo
approved these changes
Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
transport.CanonicalizeHeaderKeycalls frommetadataToTransportRequestandgetApplicationHeadersintransport/grpc/headers.go. gRPC metadata keys are guaranteed lowercase by HTTP/2 spec and grpc-go implementation.Problem
Both functions call
CanonicalizeHeaderKey(strings.ToLower) on every key while iteratingmetadata.MD, but these keys are already guaranteed lowercase — making the calls redundant CPU work.Safety — three independent guarantees:
FromIncomingContextexplicitly lowercases all keys — metadata: convert keys to lowercase in FromContext() grpc/grpc-go#4416Benchmark
count=6, AMD EPYC 9B45:Test plan
RELEASE NOTES: N/a
Update (2026-07-26): rebased onto current
main. One positional conflict intransport/grpc/headers_test.go: main's newly-addedBenchmarkIsReservedand this PR's benchmarks were both appended at the same location — resolved by keeping both functions.