RFR: 8268622: Performance issues in javac `Name` class [v2]

[Vicente Romero]

On Tue, 26 Sep 2023 21:41:54 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> Quoting [JDK-8268622](https://bugs.openjdk.org/browse/JDK-8268622):
>>> The javadoc team has identified performance issues in the javac `Name` class, particularly when it is used via its `CharSequence` interface ([JDK-8263321](https://bugs.openjdk.org/browse/JDK-8263321)). The main issue there is that the `CharSequence` is specifically character-oriented, whereas the native form inside javac is a byte[] in modified-UTF8 form. Operations like `.length()` and `.charAt(int)` have "simple" but very inefficient implementations.
>> 
>> Thanks to [JDK-8269957](https://bugs.openjdk.org/browse/JDK-8269957) we now have the option to swap in a `String`-based `Name` table implementation in place of the current UTF-8-based one. This does in fact improve performance of the compiler.
>> 
>> This patch does two things:
>> * Make the `String`-based `Name` table implementation the default. The two other UTF-8-based implementations are still available via the flags `-XDuseUnsharedTable=true` and `-XDuseSharedTable=true` (the latter was the previous default). This results in a 19% speedup in my simple benchmark (see below).
>> * Adds a new flag `-XDinternStringTable=true`, which turns on `intern()`ing of the `String`s in the `String` table. This however is **not** the default because it results in lower performance (see below). But it's possible this could help in other scenarios so it's left in there for future testing purposes.
>> 
>> Here are results from the [JavacNameTable.java](https://github.com/openjdk/jdk/files/12466359/JavacNameTable.java.txt) benchmark.
>> 
>> Benchmark                             Mode  Cnt   Score   Error  Units
>> JavacNameTable.testSharedTable        avgt   25  54.145 ± 1.738  ms/op
>> JavacNameTable.testStringTable        avgt   25  45.305 ± 0.415  ms/op
>> JavacNameTable.testStringTableIntern  avgt   25  50.736 ± 0.245  ms/op
>> JavacNameTable.testUnsharedTable      avgt   25  59.877 ± 2.503  ms/op
>> 
>> To reiterate: `testSharedTable` reflects the previous default; `testStringTable` reflects the new default. The other two options are available via the flags `-XDinternStringTable=true` and `-XDuseUnsharedTable=true`.
>
> Archie Cobbs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8268622
>  - Change the default Name table implementation to StringNameTable.
>    
>    This removes the "useStringTable" flag (since it's now the default),
>    adds a "useSharedTable" flag (since it's no longer the default), and
>    adds an "internStringTable" flag enabling intern()'ing Name strings.

lgtm

-------------

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15470#pullrequestreview-1646854592