RFR: 8306842: Classfile API performance improvements

[Adam Sotona]

On Wed, 26 Apr 2023 15:04:50 GMT, Adam Sotona <asotona at openjdk.org> wrote:

> Following improvements implemented:
> - Switch over `String` replaced with switch single char
> - Binary search for frames in `StackMapGenerator`
> - `StackMapGenerator.rawHandlers` with pre-calculated offsets
> - `ClassEntry` is caching `ClassDesc` symbol
> - Caching of type symbols in `NameAndTypeEntry` and `MethodTypeEntry`
> - Caching `MethodTypeDesc` in `MethodInfo` implementations
> - `StackMapGenerator` and `Utils` delegating to cached `MethodTypeDesc` instead of custom parsing
> 
> No API change.
> 
> Benchmarks show stack map generation improved by 21% and code generation from symbols improved by 30%.
> 
> 
> Benchmark                     Mode  Cnt       Score       Error  Units
> GenerateStackMaps.benchmark  thrpt   10  407931.202 ± 13101.023  ops/s
> RebuildMethodBodies.shared   thrpt    4   10258.597 ±   383.699  ops/s
> RebuildMethodBodies.unshared thrpt    4    7224.543 ±   256.800  ops/s
> 
> 
> 
> Benchmark                     Mode  Cnt       Score      Error  Units
> GenerateStackMaps.benchmark  thrpt   10  495101.110 ± 2389.628  ops/s
> RebuildMethodBodies.shared    thrpt   4   13380.272 ±  810.113  ops/s
> RebuildMethodBodies.unshared  thrpt   4    9399.863 ±  557.060  ops/s

Caching of `ClassDesc` in `ClassEntry` is already a part of this PR and the internal name is key for `ClassEntry` from the very beginning, so obtaining internal name from `ClassEntry` was never an issue.
However `ClassEntry` is a short-live object, newly created for each generated or transformed class and it exists within a constant pool context only.
In order to reach the same performance as with cached internal names in `ClassDesc` instances, we would have to change the user coding schema and not the Classfile API internals.
Every code generation or transformation would have to start with static declaration of `ClassEntry` constants (instead of `ClassDesc`), constructed from to `TemporaryConstantPool` (which is for internal purpose yet).

I've added new benchmarks measuring repeated transformations rebuilding method bodies (down to the symbols expansion) from already expanded models.
I’ve added shared and unshared CP alternatives (for curiosity), however the unshared is closer to simulation of building from symbols.
 
Here are results from the actual code base:
 
Benchmark                      Mode  Cnt      Score     Error  Units
RebuildMethodBodies.shared    thrpt    4  10258.597 ± 383.699  ops/s
RebuildMethodBodies.unshared  thrpt    4   7224.543 ± 256.800  ops/s
 
And here is already visible approx. 22% improvement in both (without the rest of proposed improvements yet):
 
Benchmark                      Mode  Cnt      Score      Error  Units
RebuildMethodBodies.shared    thrpt    4  12498.597 ± 309.585  ops/s
RebuildMethodBodies.unshared  thrpt    4   8807.229 ±  167.247  ops/s

It would be also interesting to see a difference with cached internal names in `ClassDesc`.

Now we have following benchamrk numbers:

Benchmark                      Mode  Cnt      Score     Error  Units
RebuildMethodBodies.shared    thrpt    4  13380.272 ± 810.113  ops/s
RebuildMethodBodies.unshared  thrpt    4   9399.863 ± 557.060  ops/s


Which is 30% performance improvement :)

I benchmarked merged this PR with #13598 and benefits of caching of internal names in ClassDesc are insignificant (~2% performance boost).

Problem with ClassDesc "embedding" into the Utf8Entry is with its ambiguity.
Utf8Entry mainly contains some name or descriptor (MethodTypeDesc, ClassDesc, PackageDesc, ModuleDesc, etc...) or signature (Signature, MethodSignature, ClassSignature, etc...).
And even for ClassDesc there is ambiguity of the serialized form. When the Utf8Entry is related to ClassEntry it contains descriptor for arrays or internal name for classes. However when the Utf8Entry is related for example to annotation it contains always class descriptor (even for classes).
>From this perspective the Utf8Entry should not be responsible for conversions to and from symbols, because it does not know the context and so the right serialised form.
ClassEntry knows the  exact form of its Utf8Entry, Annotation, MethodInfo or (with just a minor ambiguity) also NameAndTypeEntry know which symbols to use and how to serialize/deserialize them.
I think that having Utf8Entry as a big central dispatcher for so many types of symbols and serialisation forms is not the best design and will include a lot of complexity and confusion. 
Current architecture lets interaction with symbols responsibility on the right layer.

 It is unlikely that one Utf8Entry will hold two different symbols, however still possible (for example when Signature and ClassDesc are the same).
And it is still very likely that one symbol describing the same class will be stored in two different Utf8Entries (for example the same class stored as reference from ClassEntry vs. as reference from Annotation).

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

PR Comment: https://git.openjdk.org/jdk/pull/13671#issuecomment-1527068370
PR Comment: https://git.openjdk.org/jdk/pull/13671#issuecomment-1527081241
PR Comment: https://git.openjdk.org/jdk/pull/13671#issuecomment-1527241784
PR Comment: https://git.openjdk.org/jdk/pull/13671#issuecomment-1527327378
PR Comment: https://git.openjdk.org/jdk/pull/13671#issuecomment-1531104946