RFR: 8260710: Inline and simplify String*::{coder, value, isLatin1} methods

[Roger Riggs]

Hi,

It would be useful to drive a discussion of removing fallback provided 
by COMPACT_STRINGS  entirely.
It was retained initially as a hedge against bugs in the new implementation.
It adds complexity to the implementation and can hide implementation bugs.

Regards, Roger


On 2/9/21 7:50 AM, Aleksey Shipilev wrote:
> Since Compact Strings implementation, there are simple methods in String and StringBuilders: `coder()`, `value()`, `isLatin1()`. They are mostly there to capture `COMPACT_STRINGS` flag that would fold to "false" when compact strings are disabled with `-XX:-CompactStrings`.
>
> This was to guarantee the same performance when Compact String are unimplemented and/or have surprising performance behaviors. Quite some time had passed since JDK 9 release, and we can consider simplifying these. For example, ignoring `COMPACT_STRINGS` and inlining the methods simplifies the code. Some new code added to `String` after JDK 9 already has these methods inlined, checking `coder` directly.
>
> The major drawback is that supplying `-XX:-CompactStrings` would have to go through the same `coder == UTF16` paths the "normal" Compact Strings are going. It would not be folded to the most optimal form by the optimizer, because `coder` field value is generally opaque to optimizers.
>
> On the flip side, there is an advantage on paths that do not have optimizer ready (yet), for example at startup. Hello World startup improves for about 0.3 msec (or ~1.3%). The improvement is quite probably more visible on larger workloads, i.e. large application servers. (In fact, this is where I spotted this opportunity: OpenLiberty startup profiles).
>
> Interpreter-only "Hello World":
>
>   Performance counter stats for 'taskset -c 13 build/baseline/bin/java -Xms128m -Xmx128m -Xint Hello' (5000 runs):
>
>               23.40 msec task-clock                #    0.933 CPUs utilized            ( +-  0.01% )
>          86,133,190      cycles                    #    3.680 GHz                      ( +-  0.01% )
>          79,854,588      instructions              #    0.93  insn per cycle           ( +-  0.00% )
>
>          0.02507105 +- 0.00000343 seconds time elapsed  ( +-  0.01% )
>
>   Performance counter stats for 'taskset -c 13 build/linux-x86_64-server-release/images/jdk/bin/java -Xms128m -Xmx128m -Xint Hello' (5000 runs):
>
>               23.07 msec task-clock                #    0.935 CPUs utilized            ( +-  0.01% )
>          84,877,118      cycles                    #    3.679 GHz                      ( +-  0.01% )
>          79,231,598      instructions              #    0.93  insn per cycle           ( +-  0.00% )
>
>          0.02466467 +- 0.00000382 seconds time elapsed  ( +-  0.02% )
>
>
> OpenLiberty startup:
>
>   Before: 2.296, 2.281, 2.291 (seconds)
>   After:  2.254, 2.267, 2.272 (seconds)
>
> Additional tests:
>   - [x] Linux `tier1` default (passes)
>   - [x] Linux `tier1` `-XX:-CompactStrings`
>
> -------------
>
> Commit messages:
>   - Merge branch 'master' into JDK-8260710-string-inline
>   - Merge branch 'master' into JDK-8260710-string-inline
>   - Initial prototype
>
> Changes: https://git.openjdk.java.net/jdk/pull/2334/files
>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2334&range=00
>    Issue: https://bugs.openjdk.java.net/browse/JDK-8260710
>    Stats: 173 lines in 5 files changed: 5 ins; 48 del; 120 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/2334.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/2334/head:pull/2334
>
> PR: https://git.openjdk.java.net/jdk/pull/2334