RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor} [v5]

[Aleksey Shipilev]

On Fri, 31 May 2024 17:59:18 GMT, jengebr <duke at openjdk.org> wrote:

>> Improve `java/lang/reflect/Method.java`  by eliminating needless cloning of Class[0] instances.  This cloning is intended to prevent callers from changing array contents, but many Methods have zero exceptions or zero parameters, and returning the original `Class[0]` is sufficient.
>> 
>> Results from the included JMH benchmark:
>> Before:
>> 
>> Benchmark                                    Mode  Cnt  Score   Error  Units
>> ConstructorBenchmark.getExceptionTypes       avgt    5  6.526 ± 0.183  ns/op
>> ConstructorBenchmark.getExceptionTypesEmpty  avgt    5  5.803 ± 0.073  ns/op
>> ConstructorBenchmark.getParameterTypes       avgt    5  6.521 ± 0.188  ns/op
>> ConstructorBenchmark.getParameterTypesEmpty  avgt    5  5.747 ± 0.087  ns/op
>> MethodBenchmark.getExceptionTypes            avgt    5  6.525 ± 0.163  ns/op
>> MethodBenchmark.getExceptionTypesEmpty       avgt    5  5.783 ± 0.130  ns/op
>> MethodBenchmark.getParameterTypes            avgt    5  6.518 ± 0.195  ns/op
>> MethodBenchmark.getParameterTypesEmpty       avgt    5  5.742 ± 0.028  ns/op
>> 
>> 
>> After:
>> 
>> Benchmark                                    Mode  Cnt  Score   Error  Units
>> ConstructorBenchmark.getExceptionTypes       avgt    5  6.590 ± 0.124  ns/op
>> ConstructorBenchmark.getExceptionTypesEmpty  avgt    5  1.351 ± 0.061  ns/op
>> ConstructorBenchmark.getParameterTypes       avgt    5  6.651 ± 0.132  ns/op
>> ConstructorBenchmark.getParameterTypesEmpty  avgt    5  1.353 ± 0.150  ns/op
>> MethodBenchmark.getExceptionTypes            avgt    5  6.701 ± 0.151  ns/op
>> MethodBenchmark.getExceptionTypesEmpty       avgt    5  1.422 ± 0.025  ns/op
>> MethodBenchmark.getParameterTypes            avgt    5  6.629 ± 0.142  ns/op
>> MethodBenchmark.getParameterTypesEmpty       avgt    5  1.273 ± 0.169  ns/op
>
> jengebr has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixing copyright message, returning values from benchmark

Thanks for adding the benchmark. This looks good to me. Other nits below, your call if you want to fix them or not. (This is usually done if there are other changes requested, but they are not important enough to require standalone work.)

test/micro/org/openjdk/bench/java/lang/reflect/ConstructorBenchmark.java line 64:

> 62:             throw new RuntimeException(e);
> 63:         }
> 64: 

Here and in another benchmark.  Excess newline?

test/micro/org/openjdk/bench/java/lang/reflect/ConstructorBenchmark.java line 85:

> 83:     public Object[] getParameterTypes() throws Exception {
> 84:         return oneParameterConstructor.getParameterTypes();
> 85:     }

Here and in another benchmark. Order looks weird: non-empty, empty, empty, non-empty.

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19327#pullrequestreview-2091420074
PR Review Comment: https://git.openjdk.org/jdk/pull/19327#discussion_r1622774426
PR Review Comment: https://git.openjdk.org/jdk/pull/19327#discussion_r1622775072