parker89 opened a new pull request, #912: URL: https://github.com/apache/httpcomponents-client/pull/912
`RequestSupport#extractPathPrefix` runs on every request through `ProtocolExec`, `AsyncProtocolExec` and `RequestAuthCache`, whether or not authentication is configured. It builds a `URIBuilder` from the whole request target, so every query parameter gets percent-decoded only to be thrown away by `clearParameters()`. Then the path is parsed by `java.net.URI`, decoded segment by segment and re-encoded, even when there is nothing to normalize. In our production profiles this method showed up in almost every service using the classic client, at a median of about 0.6% CPU and 1.2% of allocation per service and up to about 6.6% CPU / 11.6% allocation for services that send long query strings. This change: - stops at the first `?` or `#`, so the query and fragment are never parsed - returns the prefix directly when the path is already in normalized form (leading `/`, unreserved characters only, no empty or dot segments), which is the common case - otherwise runs the existing `URIBuilder` normalization on the path alone, falling back to the full request target if the path by itself isn't a valid URI reference (e.g. `//?x`) The result is the same as before for every request target that is a valid URI, including `%2F`, non-normalized percent encoding, dot segments, empty segments and `;` parameters. The existing assertions are unchanged. I added a randomized test that compares the new code against the previous implementation over a mix of those cases, plus explicit cases for queries with encoded characters, fragments, trailing slashes and encoded slashes. One intentional difference: when the path is valid but the query or fragment has characters `java.net.URI` rejects (e.g. `/a/b?q=x y`), the old code returned the whole request target, query included, as the prefix. Now it returns `/a/`. JMH, JDK 21, average time and `gc.alloc.rate.norm`: | Request target | before ns/op | after ns/op | before B/op | after B/op | |---|---:|---:|---:|---:| | no query | 651 | 41 | 1,664 | 56 | | 3 params | 2,093 | 97 | 4,824 | 96 | | 6 params, percent-encoded values | 2,888 | 42 | 6,340 | 56 | | 10 params | 4,646 | 115 | 10,232 | 104 | | 4 params, `%2F` in the path (still uses `URIBuilder`) | 2,882 | 1,462 | 5,872 | 2,848 | Happy to share the benchmark if useful, I didn't want to add a JMH module just for this. The commit also applies cleanly to 5.6.x and passes there against httpcore 5.4.4, so it would be great to get it backported for the next 5.6.x release. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
