On Wed, 15 May 2024 11:17:42 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> This PR suggests refactoring the implementation classes of
>> java.lang.constant into a new package jdk.internal.constant to enable
>> sharing some trusted static factory methods with users elsewhere in
>> java.base, such as java.lang.invoke and java.lang.classfile. The refactoring
>> also adds some "trusted" methods for use when input is pre-validated, and
>> makes use of them where applicable in the current implementation. There are
>> more changes in the pipeline which will be folded into #17108 or PR'ed once
>> that is integrated.
>>
>> There are two optimizations mixed up here. One in
>> `MethodTypeDesc.ofDescriptor`:
>>
>> Name
>> (descString) Cnt Base Error Test Error Unit Change
>> MethodTypeDescFactories.ofDescriptor
>> (Ljava/lang/Object;Ljava/lang/String;)I 6 138,371 ± 0,767 136,939 ±
>> 1,126 ns/op 1,01x (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> ()V 6 10,528 ± 2,495 4,110 ± 0,047 ns/op 2,56x (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> ([IJLjava/lang/String;Z)Ljava/util/List; 6 209,390 ± 4,583 196,175 ±
>> 3,211 ns/op 1,07x (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> ()[Ljava/lang/String; 6 36,039 ± 8,684 20,794 ± 1,110 ns/op 1,73x
>> (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> (..IIJ)V 6 279,139 ± 6,248 187,934 ± 0,857 ns/op 1,49x (p = 0,000*)
>> MethodTypeDescFactories.ofDescriptor
>> (.....................). 6 2174,387 ± 132,879 1150,652 ± 3,158 ns/op
>> 1,89x (p = 0,000*)
>> * = significant
>>
>>
>> The other in `ClassDesc::nested`, where to get rid of a simple static method
>> in `ConstantUtils` I ended up speeding up and simplifying the public factory
>> method:
>>
>> Name Cnt Base Error Test
>> Error Unit Change
>> ClassDescFactories.ReferenceOnly.ofNested 6 209,853 ± 132,525 22,017 ±
>> 0,573 ns/op 9,53x (p = 0,000*)
>> * = significant
>>
>>
>> The optimizations could be split out and PRd independently, but I think they
>> are simple enough to include in this refactoring.
>
> Claes Redestad has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Update src/java.base/share/classes/java/lang/constant/ClassDesc.java
>
> Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
> - Update src/java.base/share/classes/java/lang/constant/ClassDesc.java
>
> Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
FTR I had to back out a change that would delegate to the now simplified and
optimized `arrayType()` from `arrayType(int rank)` when `rank` equals `1` since
the methods actually throw different exceptions, `IllegalStateException` vs
`IllegalArgumentException`. This was caught by a later test.
Unfortunate that `java.lang.constant` has chosen to use these two in particular
since it's easy to miss due to the similarity of the exception names. There are
also some unspecified exceptional behavior. For example on `arrayType()` only
`IllegalStateException` is specified to be thrown, but calling
`ClassDesc.ofDescriptor("V").arrayType()` throws an `IllegalArgumentException`.
This behavior is not changed by this PR.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19105#issuecomment-2112663111
