On Mon, 15 May 2023 14:48:58 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 two additional
> commits since the last revision:
>
> - changed LinkedList to ArrayList in RebuildMethodBodies benchmark
> - added RepeatedModelTraversal JMH benchmark
src/java.base/share/classes/jdk/internal/classfile/impl/StackMapDecoder.java
line 85:
> 83: for(var arg : methodType.parameterList()) {
> 84: vtis[i++] = switch (arg.descriptorString().charAt(0)) {
> 85: case 'I', 'S', 'C' ,'B', 'Z' ->
> SimpleVerificationTypeInfo.ITEM_INTEGER;
Suggestion:
case 'I', 'S', 'C' ,'B', 'Z' ->
SimpleVerificationTypeInfo.ITEM_INTEGER;
test/micro/org/openjdk/bench/jdk/classfile/RebuildMethodBodies.java line 58:
> 56: shared = new ArrayList<>();
> 57: unshared = new ArrayList<>();
> 58:
> Files.walk(FileSystems.getFileSystem(URI.create("jrt:/")).getPath("modules/java.base/java")).forEach(p
> -> {
Suggestion:
Files.walk(FileSystems.getFileSystem(URI.create("jrt:/")).getPath("modules/java.base/java")).forEach(p
-> {
test/micro/org/openjdk/bench/jdk/classfile/RepeatedModelTraversal.java line 55:
> 53: public void setup() throws IOException {
> 54: models = new LinkedList<>();
> 55:
> Files.walk(FileSystems.getFileSystem(URI.create("jrt:/")).getPath("modules/java.base/java/util")).forEach(p
> -> {
Suggestion:
Files.walk(FileSystems.getFileSystem(URI.create("jrt:/")).getPath("modules/java.base/java/util")).forEach(p
-> {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13671#discussion_r1194695545
PR Review Comment: https://git.openjdk.org/jdk/pull/13671#discussion_r1194695808
PR Review Comment: https://git.openjdk.org/jdk/pull/13671#discussion_r1194695081
