On Wed, 15 May 2024 07:20:37 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 one additional 
> commit since the last revision:
> 
>   Use sb.repeat, consolidate

src/java.base/share/classes/java/lang/constant/ClassDesc.java line 341:

> 339:         String desc = descriptorString();
> 340:         int index = desc.lastIndexOf('/');
> 341:         return (index == -1) ? "" : internalToBinary(desc.substring(1, 
> index - 1));

Here it cuts the package name, for example 
`ConstantDescs.CD_Integer.packageName()` returns `java.lan`

Suggestion:

        return (index == -1) ? "" : internalToBinary(desc.substring(1, index));

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19105#discussion_r1601328026

Reply via email to