On Tue, 16 May 2023 08:17:02 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
>
> Adam Sotona has updated the pull request incrementally with one additional
> commit since the last revision:
>
> LinkedList replaced with ArrayList in benchmarks
Looks like a good set of improvements for throughput cases.
For the future I think we should probably add something that measures
startup/warmup overheads of the classfile API, since it's likely that usage
pattern in many applications will be heavy during bootstrap and then subside
(similar to how ASM usage can be startup sensitive but will then disappear from
sight once an application is up and running)
src/java.base/share/classes/jdk/internal/classfile/impl/StackMapDecoder.java
line 83:
> 81: vtis = new VerificationTypeInfo[methodType.parameterCount()];
> 82: }
> 83: for(var arg : methodType.parameterList()) {
`MethodTypeDesc::parameterList` copies the internal `ClassDesc[]`, so
desugaring loop from `0` to `mdesc.parameterCount()` might be a small
improvement for this and a few other cases.
-------------
Marked as reviewed by redestad (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13671#pullrequestreview-1428657206
PR Review Comment: https://git.openjdk.org/jdk/pull/13671#discussion_r1195248228
