On Thu, 12 Mar 2026 11:14:13 GMT, Eirik Bjørsnøs <[email protected]> wrote:
>>> I suggest adding a comment block to the beginning of this method explaining >>> why it is fast. >> >> I'll be completely honest here and admit that my understanding of _why_ this >> is fast is a bit shallow: >> >> * I added the fast path(s) because of my intuition that URLs without query >> or ref are common. However, removing the fast path also seems to make slow >> paths slower. C2 magic? >> * Initially I also thought about indified string concats, but then javac >> generates plain StringBuilder.append calls for this method, probably because >> this is in `java.base` and we want to avoid any issues in early bootstrap. >> Ind >> * I observe that consecutive StringBuilder::append calls of locals without >> any interleaving conditional code is faster. Again, my guess is there is >> some C2 optimization at play which does not kick in with the interleaving >> conditionals. >> * A single concatenation of more components is faster than combining the >> result of other concatenations >> >> >>>Inspirational material: >> >> What do you think of the comment I just pushed: >> >> >> // Optimizations in this method are based on the following observations: >> // 1: The query and ref components are commonly empty and can be specialized >> // 2: String concatenation is faster when locals are concatenated with no >> interleaving code >> // 3: A single concatenation is faster than combining results of other >> concatenations > >> I've tried to outsmart @eirbjo, and failed beautifully: >> >> 1. In the original method >> ([5e58808](https://github.com/openjdk/jdk/commit/5e588085e9c333e9ad8e48dd7c60e665d30e278d)), >> replaced the conditional string concatenation with an explicit >> `StringBuilder` that is populated using `if` blocks: >> ``` >> var sb = new StringBuilder(u.getProtocol()).append(':'); >> var authority = u.getAuthority(); >> if (authority != null && !authority.isEmpty) { >> sb.append("//").append(authority); >> } >> // ... > > Since javac actually generates StringBuilder::append calls here, we can get > equal bytecode and performance by using StringBuilder directly. But for good > performance, the same rule of no interleaving code applies, so you'll need to > do: > > ```java > return new StringBuilder().append(u.getProtocol()).append("://").. > > > In fact it seems we can get _slightly better_ performance by initializing the > StringBuilder with the always-present protcol component: > > ```java > return new StringBuilder(u.getProtocol()).append("://").. > > > But the benefits are too marginal to deserve the code bloat. Running the benchmark with `-XX:-OptimizeStringConcat` shows that this PR leans on generating code shapes which are recognized by that optimization. However, results are still consistently better with that OptimizeStringConcat disabled, just not as much. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30151#discussion_r2924770243
