On Wed, 26 Apr 2023 15:04:50 GMT, Adam Sotona <asot...@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
