Optimize StringUtil and HttpURI - #15673
Open
lorban wants to merge 6 commits into
Open
Conversation
asciiToLowerCase and asciiToUpperCase scan for the first character needing conversion before deciding whether any work is needed at all, and most strings need none, so that scan decides the common case. It tested one character per iteration, with a table load and a comparison each. Pack 4 characters into a long, one per 16 bit lane, and test all 4 at once with a SWAR range comparison; the string is returned unchanged as soon as the scan runs out without a match. The top bit of each lane is cleared before the additions so they cannot carry into the next lane, and those lanes are then discarded, as such a character is far outside the ASCII range. The test can report a false positive for a non ASCII lane, which a per character confirmation rejects; it never reports a false negative for an ASCII lane, which is what makes the scan correct. The two loops of the previous implementation also collapse into one: scanning backwards for the last uppercase character and converting everything below it is the same as scanning forwards for the first and converting everything above it. Verified against the previous implementations over all 65536 characters, over all 4 character combinations of the values around every lane and range boundary, and over 2M random strings.
Parsing the path switched on every character and, for the ones that are not delimiters, consulted two boolean tables: one for whether the character is legal, one for whether it is suspicious. Almost every character of a real path is neither, so almost every character paid a switch dispatch and two table lookups to establish that there was nothing to do. Collect the characters that need attention, the delimiters that drive the state machine plus the illegal and suspicious ones, into a 128 bit set held as two longs, and test it before the switch. The test is a shift and a mask of a value the JIT can keep in a register, and it takes the common character out of the switch entirely. The same test short circuits validateSegment(). The set is derived from the same tables and delimiters in the static initializer, so it cannot drift from them. Verified against the previous implementation over 202k URIs through three entry points, comparing path, canonical path, decoded path, param, query, fragment, host, scheme and violations: identical throughout. The corpus covers every character in several positions, percent encodings, parameters, dot segments and random strings over an alphabet of the interesting characters. Parsing a long path drops 31%, an absolute URI 36%, a typical path with a query 19%. Signed-off-by: Ludovic Orban <lorban@bitronix.be>
The check for an illegal path character read the table before testing
the index against its length:
if (c > __pathCharacters.length || !__pathCharacters[c])
The table holds 128 entries, so for c == 128 the first test is false
and the second indexes one past the end. Every other character is
handled: below 128 the index is valid, above 128 the first test short
circuits. Only U+0080 lands in the gap.
The effect is an ArrayIndexOutOfBoundsException instead of an
ILLEGAL_PATH_CHARACTERS violation, so the URI never reaches the
compliance handling that decides what to do about it. A path can carry
that character over HTTP/2, where :path is decoded as ISO-8859-1.
The same expression appears in validateSegment(), fixed as well.
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
Every request parses a URI, from the request line over HTTP/1 and from the :path pseudo header over HTTP/2, and nothing measured it. Cover the shapes that behave differently: a short path, a typical path with a query, a long path, a percent encoded path, a path with a parameter, and an absolute URI.
HttpURI.Mutable.scheme(HttpScheme) went through scheme(String), which lowercases via URIUtil.normalizeScheme. All four HttpScheme values are lowercase already, so the scan can only ever return the string it was given. This is on the path of every HTTP/2 request, whose :scheme pseudo header resolves to an HttpScheme before the URI is built. Verified that the enum and string paths still produce the same scheme for every value. Decoding a request in steady state drops a further 4%.
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
Contributor
Author
|
The baseline (12.1.x vanilla): with this PR: There is a significant degradation for long strings with late conversion and a small degradation for long strings that do not need to be changed, with marginal changes in the other cases. @wendigo what motivated you to modify the |
joakime
reviewed
Aug 27, 2026
Comment on lines
1238
to
1246
| public Mutable scheme(HttpScheme scheme) | ||
| { | ||
| return scheme(scheme.asString()); | ||
| // The known schemes are lowercase already, so skip normalizing them; | ||
| // this is on the path of every HTTP/2 request, whose :scheme pseudo | ||
| // header resolves to one of them. | ||
| _scheme = scheme.asString(); | ||
| _uri = null; | ||
| return this; | ||
| } |
Contributor
There was a problem hiding this comment.
Why change HTTP/1 behaviors?
These changes should not favor HTTP/2 over HTTP/1.
sbordet
added
the
Sponsored
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.
The
StringUtilandHttpURIparts of #15498 made by @wendigo