On Mon, 4 Mar 2024 10:08:27 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update
>> src/java.base/share/classes/jdk/internal/classfile/impl/StackMapDecoder.java
>>
>> Co-authored-by: Andrey Turbanov <turban...@gmail.com>
>
> src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java
> line 36:
>
>> 34: import java.lang.classfile.constantpool.MemberRefEntry;
>> 35: import static java.lang.classfile.ClassFile.*;
>> 36: import java.lang.constant.MethodTypeDesc;
>
> Imports are strangely out-of-order and imports in the classfile package look
> haphazard. Is there some system to it that I don't see? I don't know if we
> have an applicable style guide to lean on, but alphabetically sorted and
> static imports split out at the end seem like the convention in the OpenJDK
> sources.
Fixed, thanks.
> test/micro/org/openjdk/bench/jdk/classfile/CodeAttributeTools.java line 108:
>
>> 106: @Benchmark
>> 107: public void benchmarkStackMapsGenerator() {
>> 108: for (var d : data) new StackMapGenerator(
>
> When we return something from a benchmark JMH makes sure to sink that
> something into a blackhole to avoid JIT assuming the result is unused and
> eliminating it, wholly or in parts. When it's impractical to return a single
> thing that captures all the computation in the benchmark it's good practice
> to let a `org.openjdk.jmh.infra.Blackhole` consume effects to attain the same
> effect:
>
> public void benchmarkStackMapsGenerator(Blackhole bh) {
> for (var d : data) bh.consume(new StackMapGenerator(
Fixed, thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17306#discussion_r1511040395
PR Review Comment: https://git.openjdk.org/jdk/pull/17306#discussion_r1511042556
