On Fri, 26 Apr 2024 10:54:49 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> This PR makes ClassDesc.ofDescriptor return the shared constant for
>> primitive descriptor strings ("I" etc..), and leverages this further by
>> refactoring `MethodTypeDescImpl.ofDescriptor` to avoid the intermediate
>> substring for primitives.
>>
>> Microbenchmarks results look good with expected speedups and allocation
>> reductions any time a primitive is part of the descriptor string, and a
>> non-significant cost otherwise:
>>
>>
>> Name
>> (descString) Cnt Base Error Test Error Unit Change
>> ClassDescFactories.ofDescriptor
>> Ljava/lang/Object; 6 13,941 ± 1,643 14,071 ± 1,333 ns/op
>> 0,99x (p = 0,681 )
>> :gc.alloc.rate.norm
>> 16,000 ± 0,000 16,000 ± 0,000 B/op 1,00x (p = 0,617 )
>> ClassDescFactories.ofDescriptor
>> V 6 9,212 ± 1,045 1,405 ± 0,049 ns/op 6,55x (p = 0,000*)
>> :gc.alloc.rate.norm
>> 48,000 ± 0,000 0,000 ± 0,000 B/op 0,00x (p = 0,000*)
>> ClassDescFactories.ofDescriptor
>> I 6 9,009 ± 0,035 1,431 ± 0,192 ns/op 6,30x (p = 0,000*)
>> :gc.alloc.rate.norm
>> 48,000 ± 0,000 0,000 ± 0,000 B/op 0,00x (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> (Ljava/lang/Object;Ljava/lang/String;)I 6 182,050 ± 4,333 141,644 ±
>> 2,685 ns/op 1,29x (p = 0,000*)
>> :gc.alloc.rate.norm
>> 360,001 ± 0,000 264,001 ± 0,000 B/op 0,73x (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> ()V 6 17,169 ± 2,008 9,915 ± 0,018 ns/op 1,73x (p = 0,000*)
>> :gc.alloc.rate.norm
>> 120,000 ± 0,000 104,000 ± 0,000 B/op 0,87x (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> ([IJLjava/lang/String;Z)Ljava/util/List; 6 270,372 ± 3,624 217,050 ±
>> 3,170 ns/op 1,25x (p = 0,000*)
>> :gc.alloc.rate.norm
>> 520,002 ± 0,000 328,001 ± 0,000 B/op 0,63x (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> ()[Ljava/lang/String; 6 35,036 ± 0,351 36...
>
> Claes Redestad has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix ClassDescTest; remove redundant validation performed by
> ReferenceClassDescImpl
Looks like your `MethodTypeDescFactories` benchmark is missing form the PR?
Does removing the explicit null checks make that much difference for
performance? They are kind of nice for clarity.
Also, looks like the copyright year still needs to be updated on some of the
files.
src/java.base/share/classes/java/lang/constant/MethodTypeDescImpl.java line 97:
> 95: int args = ptypes.size() - 1;
> 96: ClassDesc[] paramTypes = args > 0
> 97: ? ptypes.subList(1, args + 1).toArray(new ClassDesc[0])
I suppose this could also use `EMPTY_CLASSDESC` instead of creating a new array
-------------
PR Review: https://git.openjdk.org/jdk/pull/18971#pullrequestreview-2024805435
PR Review Comment: https://git.openjdk.org/jdk/pull/18971#discussion_r1580881761
