Skip to content

router: avoid per-request host copy in virtual host lookup#46112

Open
davidvo-lyft wants to merge 1 commit into
envoyproxy:mainfrom
davidvo-lyft:davidvo/router-vhost-lowercase-alloc
Open

router: avoid per-request host copy in virtual host lookup#46112
davidvo-lyft wants to merge 1 commit into
envoyproxy:mainfrom
davidvo-lyft:davidvo/router-vhost-lowercase-alloc

Conversation

@davidvo-lyft

Copy link
Copy Markdown
Contributor

Commit Message: router: avoid per-request host copy in virtual host lookup

Additional Description:
RouteMatcher::findVirtualHost() lower-cased the host header into a freshly allocated std::string
on every routed request, even though the value is almost always already lower-case (DNS names
normalize to lower-case per RFC 3986 section 3.2.2) and every downstream consumer
(virtual_hosts_.find and findWildcardVirtualHost) already accepts a string_view. For hosts
longer than the std::string small-string-optimization capacity this was a heap allocation and free
on every request. This scans the host first and only builds the lower-cased copy when an upper-case
byte is actually present, keeping the common path allocation free. Behavior is unchanged:
absl::ascii_isupper matches exactly the bytes absl::AsciiStrToLower would rewrite, and non ASCII
bytes are untouched either way. This completes the string_view conversion of this function started
in PR #26964, which left this copy in place.

Design note: an alternative is a single pass that lower-cases into a stack buffer or InlinedVector,
avoiding the extra scan on the rare uppercase path. This PR uses the two pass form because uppercase
authorities are rare on the wire, the scan cost stays on the cold path, and the diff stays minimal.
Happy to switch if reviewers prefer.

Risk Level: Low (behavior identical, no config or API change).

Testing: //test/common/router:config_impl_test passes unchanged (behavior preservation). Added
a bmVirtualHostLookup benchmark that exercises the named virtual host lookup path (the existing
bmRouteTableSize configs use a wildcard * domain and take the default virtual host fast path,
never reaching the lower-casing).

Allocation measurement (CI docker image, clang, -c opt, --define tcmalloc=disabled, aarch64;
bmVirtualHostLookup/64 with a 33 byte lower-case authority; malloc interposition, per iteration
counts isolated by differencing 400x and 200x fixed-iteration runs, deterministic across repeats):

heap allocations per routed request
before 1 malloc + 1 free
after 0 (allocation free)

The virtual host lookup path becomes allocation free for lower-case authorities.

Docs Changes: N/A

Release Notes: N/A (performance improvement, no behavior change)

Platform Specific Features: N/A

Generative AI disclosure: This change was developed with AI assistance (Claude Code). I have
reviewed and understand every line, the string_view lifetime and behavior equivalence, and the
allocation measurement methodology, and I take full ownership of the submission and of addressing
review feedback.

RouteMatcher::findVirtualHost() lower-cased the host header into a freshly
allocated std::string on every routed request, even though the value is
almost always already lower-case (DNS names normalize to lower-case per RFC
3986 section 3.2.2) and every downstream consumer accepts a string_view. For
hosts longer than the small-string-optimization capacity this was a heap
allocation and free per request. Scan the host first and only build the
lower-cased copy when an upper-case byte is present, keeping the common path
allocation-free. Behavior is unchanged: absl::ascii_isupper matches exactly
the bytes absl::AsciiStrToLower would rewrite. Completes the string_view
conversion of this function started in PR envoyproxy#26964.

Add a bmVirtualHostLookup benchmark that exercises the named virtual host
lookup path (the existing configs use a wildcard domain and take the default
virtual host fast path).

Signed-off-by: David Vo <davidvo@lyft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant