On Mon, 9 Mar 2026 21:09:23 GMT, Eirik Bjørsnøs <[email protected]> wrote:
>> This PR optimizes string concatenation in `URL::toExternalForm`. This method >> is called by `URL:toString` and executes often enough to be noticed during >> startup / classloading profiling. It was previously optimized in JDK 10 via >> JDK-8193034, but we can do more. >> >> Key observations: >> >> * The current method concatenates concatenated strings. We can get away with >> less by flattening and concatenating once per call. >> * The current method does not take advantage of the fact that most URL >> components are optional and several are commonly not present. We can >> specialize for those cheaper common concatenations. >> >> This PR reduces runtime with 60% or more, depending on present URL >> components. (Benchmark results in first comment). I also ran the benchmark >> with compilation excluded and found no regressions for the interpreter case. >> >> Reviewers may find that the use of SSA-style local variables and the >> optimizations makes the code less dense and more verbose. I opted to not try >> to be clever and instead let the code "show its work" here. The code is more >> bulky, but hopefully should be easy to read. I added some code comments, >> happy to dial down those if requested. Feedback welcome! >> >> Performance refactoring, `noreg-perf`. `URL::toString` seems well tested by >> current tests. > > Performance results: > > Baseline: > > > Benchmark (auth) (query) (ref) Mode Cnt Score Error > Units > URLToString.urlToString false false false avgt 15 54,344 ± 2,856 > ns/op > URLToString.urlToString false false true avgt 15 92,925 ± 3,976 > ns/op > URLToString.urlToString false true false avgt 15 86,129 ± 0,727 > ns/op > URLToString.urlToString false true true avgt 15 107,747 ± 6,620 > ns/op > URLToString.urlToString true false false avgt 15 68,900 ± 2,452 > ns/op > URLToString.urlToString true false true avgt 15 100,402 ± 1,416 > ns/op > URLToString.urlToString true true false avgt 15 107,148 ± 4,950 > ns/op > URLToString.urlToString true true true avgt 15 120,624 ± 2,909 > ns/op > > > PR: > > > Benchmark (auth) (query) (ref) Mode Cnt Score Error > Units > URLToString.urlToString false false false avgt 15 20,443 ± 0,745 > ns/op > URLToString.urlToString false false true avgt 15 27,122 ± 0,340 > ns/op > URLToString.urlToString false true false avgt 15 28,650 ± 0,521 > ns/op > URLToString.urlToString false true true avgt 15 37,867 ± 2,050 > ns/op > URLToString.urlToString true false false avgt 15 26,757 ± 0,273 > ns/op > URLToString.urlToString true false true avgt 15 37,808 ± 0,520 > ns/op > URLToString.urlToString true true false avgt 15 37,319 ± 2,002 > ns/op > URLToString.urlToString true true true avgt 15 45,303 ± 1,551 > ns/op > @eirbjo I wonder if the performance gain is worth the expansion of the code. Yeah, this is always a concern. My gut feeling is that URLs are often converted to and from String form and as such URL.toString and URL.toExternalForm may be called quite frequently in applications which parse, validate, normalize URLs. In JDK-8193034, Martin noted: > At Google we spend a *lot* of cycles on URL.toExternalForm. While "a lot" is not very sharply defined, I think this hints that some users would notice this improvement. Perhaps Oracle or others have actual data? Regarding the cost of expansion, I'm not sure denser code is universally better. While the current `URLStreamHandler.toExternalForm` is aesthetically pleasing and does an impressive amont of work in just a few lines, those lines are actually quite busy, and I think code cost should be measured in complexity, not just size. While the reuse of the local String variable is clever and allows for dense code, it may also require that maintainers squint their eyes a bit extra when reading this code. That being said, sure, the fast paths and the expansion / flattening does add to the code size. The current and PR implementations indeed differ in size, but perhaps somewhat less so in complexity. I did ponder a while how the PR code could be condensed, but nothing promising came out of that. ------------- PR Comment: https://git.openjdk.org/jdk/pull/30151#issuecomment-4032026706
