RFR: 8331187: Optimize MethodTypeDesc and ClassDesc.ofDescriptor for primitive types [v2]

[Jorn Vernee]

On Fri, 26 Apr 2024 10:54:49 GMT, Claes Redestad <redestad at 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