RFR: 8266622: Optimize Class.descriptorString() and Class.getCanonicalName0()

[Peter Levart]

On Thu, 6 May 2021 16:34:04 GMT, Peter Levart <plevart at openjdk.org> wrote:

>> Hello, from discussion in https://github.com/openjdk/jdk/pull/3464 and https://github.com/openjdk/jdk/pull/2212 it appears, that in `j.l.Class` expressions like
>> 
>> String str = baseName.replace('.', '/') + '/' + name;
>> 
>> are not compiled into invokedynamic-based code, but into one using `StringBuilder`.
>> 
>> This happens due to some bootstraping issues. Currently the bytecode for the last (most often used) branch of `Class.descriptorString()` looks like
>> 
>> public sb()Ljava/lang/String;
>>    L0
>>     LINENUMBER 21 L0
>>     NEW java/lang/StringBuilder
>>     DUP
>>     INVOKESPECIAL java/lang/StringBuilder.<init> ()V
>>     ASTORE 1
>>    L1
>>     LINENUMBER 23 L1
>>     ALOAD 1
>>     LDC "a"
>>     INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder;
>>     POP
>>    L2
>>     LINENUMBER 24 L2
>>     ALOAD 1
>>     LDC "b"
>>     INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder;
>>     POP
>>    L3
>>     LINENUMBER 26 L3
>>     ALOAD 1
>>     INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String;
>>     ARETURN
>> 
>> Here the `StringBuilder` is created with default constructor and then expands if necessary while appending. 
>> 
>> This can be improved by manually allocating `StringBuilder` of exact size. The benchmark demonstrates measurable improvement:
>> 
>> @State(Scope.Benchmark)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class ClassDescriptorStringBenchmark {
>> 
>>     private final Class<?> clazzWithShortDescriptor = Object.class;
>>     private final Class<?> clazzWithLongDescriptor = getClass();
>> 
>>     @Benchmark
>>     public String descriptorString_short() {
>>         return clazzWithShortDescriptor.descriptorString();
>>     }
>> 
>>     @Benchmark
>>     public String descriptorString_long() {
>>         return clazzWithLongDescriptor.descriptorString();
>>     }
>> }
>> 
>> 
>> 
>> original
>> -Xint
>>                                                Mode     Score     Error   Units
>> descriptorString_long                          avgt  6326.478 ± 107.251   ns/op
>> descriptorString_short                         avgt  5220.729 ± 103.545   ns/op
>> descriptorString_long:·gc.alloc.rate.norm      avgt   528.089 ±   0.021    B/op
>> descriptorString_short:·gc.alloc.rate.norm     avgt   232.036 ±   0.015    B/op
>> 
>> -XX:TieredStopAtLevel=1
>>                                                Mode      Score    Error   Units
>> descriptorString_long                          avgt    230.223 ±  1.254   ns/op
>> descriptorString_short                         avgt    164.255 ±  0.755   ns/op
>> descriptorString_long:·gc.alloc.rate.norm      avgt    528.046 ±  0.002    B/op
>> descriptorString_short:·gc.alloc.rate.norm     avgt    232.022 ±  0.001    B/op
>> 
>> full
>>                                                Mode      Score     Error   Units
>> descriptorString_long                          avgt     74.835 ±   0.262   ns/op
>> descriptorString_short                         avgt     43.822 ±   0.788   ns/op
>> descriptorString_long:·gc.alloc.rate.norm      avgt    504.010 ±   0.001    B/op
>> descriptorString_short:·gc.alloc.rate.norm     avgt    208.004 ±   0.001    B/op
>> 
>> ------------------------
>> patched
>> -Xint
>>                                                Mode      Score     Error   Units
>> descriptorString_long                          avgt   4485.994 ±  60.173   ns/op
>> descriptorString_short                         avgt   3949.965 ± 278.143   ns/op
>> descriptorString_long:·gc.alloc.rate.norm      avgt    336.051 ±   0.004    B/op
>> descriptorString_short:·gc.alloc.rate.norm     avgt    184.027 ±   0.010    B/op
>> 
>> -XX:TieredStopAtLevel=1
>>                                                Mode        Score    Error   Units
>> descriptorString_long                          avgt      185.774 ±  1.100   ns/op
>> descriptorString_short                         avgt      135.338 ±  1.066   ns/op
>> descriptorString_long:·gc.alloc.rate.norm      avgt      336.030 ±  0.001    B/op
>> descriptorString_short:·gc.alloc.rate.norm     avgt      184.019 ±  0.001    B/op
>> 
>> full
>>                                                Mode      Score     Error   Units
>> descriptorString_long                          avgt     42.864 ±   0.160   ns/op
>> descriptorString_short                         avgt     27.255 ±   0.381   ns/op
>> descriptorString_long:·gc.alloc.rate.norm      avgt    224.005 ±   0.001    B/op
>> descriptorString_short:·gc.alloc.rate.norm     avgt    120.002 ±   0.001    B/op
>> 
>> Same can be done also for Class.isHidden() branch in Class.descriptorString() and for Class.getCanonicalName0()
>
> Hi Sergei,
> You are right that what javac generates is sub-optimal as it doesn't take into account the possible known final lenght of the string. So manually doing so is better. Since your 1st attempt a patch for String.join() method improved it quite a bit and is now not using StringBuilder under the hood any more. Could you try to measure for example the following:
> 
> 
> String str = String.join("/", baseName.replace('.', '/'), name);
> 
> as an alternative to:
> 
> String str = baseName.replace('.', '/') + '/' + name;
> 
> 
> or for example:
> 
> 
>             return String.join(
>                         "", // delimiter
>                         "L", name.substring(0, index).replace('.', '/'),
>                         ".", name.substring(index+1), ";");
> 
> as an alternative for:
> 
>             return "L" + name.substring(0, index).replace('.', '/')
>                        + "." + name.substring(index+1) + ";";
> 
> and see how it compares?
> 
> Regards, Peter

> @plevart hi, to my surprise `String.join()` makes it worse:
> 

Yeah, it seems JIT does a very good job with StringBuilder in this form.

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

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