RFR: 8266972: Use String.concat() in j.l.Class where invokedynamic-based String concatenation is not available

[Сергей Цыпанов]

On Wed, 12 May 2021 09:59:32 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Hello, from discussion in https://github.com/openjdk/jdk/pull/3464 I've found out, that in a few of JDK core classes, e.g. in `j.l.Class` expressions like `baseName.replace('.', '/') + '/' + name` are not compiled into `invokedynamic`-based code, but into one using `StringBuilder`. This happens due to some bootstraping issues.
>> 
>> However the code like
>> 
>> private String getSimpleName0() {
>>     if (isArray()) {
>>         return getComponentType().getSimpleName() + "[]";
>>     }
>>     //...
>> }
>> 
>> can be improved via replacement of `+` with `String.concat()` call.
>> 
>> I've used this benchmark to measure impact:
>> 
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class ClassMethodsBenchmark {
>>   private final Class<? extends Object[]> arrayClass = Object[].class;
>>   private Method descriptorString;
>> 
>>   @Setup
>>   public void setUp() throws NoSuchMethodException {
>>     //fore some reason compiler doesn't allow me to call Class.descriptorString() directly
>>     descriptorString = Class.class.getDeclaredMethod("descriptorString");
>>   }
>> 
>>   @Benchmark
>>   public Object descriptorString() throws Exception {
>>     return descriptorString.invoke(arrayClass);
>>   }
>> 
>>   @Benchmark
>>   public Object typeName() {
>>     return arrayClass.getTypeName();
>>   }
>> }
>> 
>> and got those results
>> 
>>                                                    Mode  Cnt     Score     Error   Units
>> descriptorString                                   avgt   60    91.480 ±   1.839   ns/op
>> descriptorString:·gc.alloc.rate.norm               avgt   60   404.008 ±   4.033    B/op
>> descriptorString:·gc.churn.G1_Eden_Space           avgt   60  2791.866 ± 181.589  MB/sec
>> descriptorString:·gc.churn.G1_Eden_Space.norm      avgt   60   401.702 ±  26.047    B/op
>> descriptorString:·gc.churn.G1_Survivor_Space       avgt   60     0.003 ±   0.001  MB/sec
>> descriptorString:·gc.churn.G1_Survivor_Space.norm  avgt   60    ≈ 10⁻³              B/op
>> descriptorString:·gc.count                         avgt   60   205.000            counts
>> descriptorString:·gc.time                          avgt   60   216.000                ms
>> 
>> patched
>>                                                    Mode  Cnt     Score     Error   Units
>> descriptorString                                   avgt   60    65.016 ±   3.446   ns/op
>> descriptorString:·gc.alloc.rate                    avgt   60  2844.240 ± 115.719  MB/sec
>> descriptorString:·gc.alloc.rate.norm               avgt   60   288.006 ±   0.001    B/op
>> descriptorString:·gc.churn.G1_Eden_Space           avgt   60  2832.996 ± 206.939  MB/sec
>> descriptorString:·gc.churn.G1_Eden_Space.norm      avgt   60   286.955 ±  17.853    B/op
>> descriptorString:·gc.churn.G1_Survivor_Space       avgt   60     0.003 ±   0.001  MB/sec
>> descriptorString:·gc.churn.G1_Survivor_Space.norm  avgt   60    ≈ 10⁻³              B/op
>> descriptorString:·gc.count                         avgt   60   208.000            counts
>> descriptorString:·gc.time                          avgt   60   228.000                ms
>> -----------------
>> typeName                                           avgt   60    34.273 ±   0.480   ns/op
>> typeName:·gc.alloc.rate                            avgt   60  3265.356 ±  45.113  MB/sec
>> typeName:·gc.alloc.rate.norm                       avgt   60   176.004 ±   0.001    B/op
>> typeName:·gc.churn.G1_Eden_Space                   avgt   60  3268.454 ± 134.458  MB/sec
>> typeName:·gc.churn.G1_Eden_Space.norm              avgt   60   176.109 ±   6.595    B/op
>> typeName:·gc.churn.G1_Survivor_Space               avgt   60     0.003 ±   0.001  MB/sec
>> typeName:·gc.churn.G1_Survivor_Space.norm          avgt   60    ≈ 10⁻⁴              B/op
>> typeName:·gc.count                                 avgt   60   240.000            counts
>> typeName:·gc.time                                  avgt   60   255.000                ms
>> 
>> patched
>> 
>> typeName                                           avgt   60    15.787 ±   0.214   ns/op
>> typeName:·gc.alloc.rate                            avgt   60  2577.554 ±  32.339  MB/sec
>> typeName:·gc.alloc.rate.norm                       avgt   60    64.001 ±   0.001    B/op
>> typeName:·gc.churn.G1_Eden_Space                   avgt   60  2574.039 ± 147.774  MB/sec
>> typeName:·gc.churn.G1_Eden_Space.norm              avgt   60    63.895 ±   3.511    B/op
>> typeName:·gc.churn.G1_Survivor_Space               avgt   60     0.003 ±   0.001  MB/sec
>> typeName:·gc.churn.G1_Survivor_Space.norm          avgt   60    ≈ 10⁻⁴              B/op
>> typeName:·gc.count                                 avgt   60   189.000            counts
>> typeName:·gc.time                                  avgt   60   199.000                ms
>> 
>> I think this patch is likely to improve reflection operations related to arrays.
>> 
>> If one finds this patch useful, then I'll create a ticket to track this. Also it'd be nice to have a list of classes, that are compiled in the same way as `j.l.Class` (i.e. have no access to `invokedynamic`) so I could look into them for other snippets where two String are joined so `concat`-based optimization is possible.
>> 
>> Pre-sizing of `StringBuilder` for `Class.gdescriptorString()` and `Class.getCanonicalName0()` is one more improvement opportunity here.
>
> src/java.base/share/classes/java/lang/Class.java line 4351:
> 
>> 4349: 
>> 4350:         if (isArray()) {
>> 4351:             return "[".concat(componentType.descriptorString());
> 
> This could be optimized further for multi-dimensional arrays:
> 
> if (isArray()) {
>   int arrayDepth = 1;
>   Class<?> ct = componentType;
>   while (ct.isArray()) {
>     arrayDepth++;
>     ct = ct.componentType;
>   }
>   return "[".repeat(arrayDepth).concat(ct.descriptorString());

But isn't `componentType.descriptorString()` does this itself? Also multi-dimensional arrays are quite an infrequent usecase, aren't they?

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

PR: https://git.openjdk.java.net/jdk/pull/3627