On Fri, 31 May 2024 17:59:18 GMT, jengebr <[email protected]> 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
