router: avoid per-request host copy in virtual host lookup#46112
Open
davidvo-lyft wants to merge 1 commit into
Open
router: avoid per-request host copy in virtual host lookup#46112davidvo-lyft wants to merge 1 commit into
davidvo-lyft wants to merge 1 commit into
Conversation
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>
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
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.
Commit Message: router: avoid per-request host copy in virtual host lookup
Additional Description:
RouteMatcher::findVirtualHost()lower-cased the host header into a freshly allocatedstd::stringon 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_.findandfindWildcardVirtualHost) already accepts astring_view. For hostslonger than the
std::stringsmall-string-optimization capacity this was a heap allocation and freeon 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_isuppermatches exactly the bytesabsl::AsciiStrToLowerwould rewrite, and non ASCIIbytes are untouched either way. This completes the
string_viewconversion of this function startedin 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_testpasses unchanged (behavior preservation). Addeda
bmVirtualHostLookupbenchmark that exercises the named virtual host lookup path (the existingbmRouteTableSizeconfigs 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/64with a 33 byte lower-case authority; malloc interposition, per iterationcounts isolated by differencing 400x and 200x fixed-iteration runs, deterministic across repeats):
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.