Optimize UserAgent version validation #864
Conversation
Generate changelog in
|
| Arguments.of("1.2.3-4-gabc", -1, true), | ||
| Arguments.of("1.2.3.4-rc5-6-gabc", -1, true), | ||
| Arguments.of("1.2.3-4", -1, false), | ||
| Arguments.of("0-0-0", -1, false)); |
There was a problem hiding this comment.
Might be helpful to add some test data with multi-digit numeric components e.g. 123.456.789 and 123.456.789-rc0
| * because primitive longs live on the stack and don't impact GC. | ||
| */ | ||
| @Immutable | ||
| final class VersionParser { |
There was a problem hiding this comment.
Fantastic documentation, code is also very readable
There was a problem hiding this comment.
I can't take credit for that, most of it is pulled from
| for (int i = 1; getIndex(state) < string.length(); i++) { | ||
| state = number(string, getIndex(state)); |
There was a problem hiding this comment.
We could probably dedup out repeated calls to getIndex(state) in places that state doesn't change, however it's a trivial operation and likely not worth deconstructing the loop
There was a problem hiding this comment.
I think I'm going to leave it as-is for now. Deduping the getIndex for loop condition & first use doesn't really change benchmark, so going to opt for simplicity and consistent use of getIndex(state)
|
Added some benchmarks comparing the custom parser validation to regex validation and the custom parser is ~3x faster for common cases than regex fallback. |
|
Released 2.26.0 |
Before this PR
com.palantir.conjure.java.api.config.service.UserAgents#isValidVersionis frequently used to validate the user agent version string, and currently uses an expensive regex which does a bunch of allocations and expensive processing. This generates unnecessary garbage and CPU cycles for services handling high volumes of HTTP requests.After this PR
We can use a hand rolled parser borrowed and simplified from palantir/sls-version-java#463 to validate
UserAgentversion strings up to 4 groups (commonly used by browsers).==COMMIT_MSG==
Optimize UserAgent version validation
==COMMIT_MSG==
Possible downsides?
Duplicates code locally from https://github.com/palantir/sls-version-java because browser version strings don't conform to SLS versioning.