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

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

On Thu, 22 Apr 2021 14:07:20 GMT, Сергей Цыпанов <github.com+10835776+stsypanov 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.

This pull request has now been integrated.

Changeset: 6c4c48fa
Author:    Sergey Tsypanov <sergei.tsypanov at yandex.ru>
Committer: Claes Redestad <redestad at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/6c4c48faea8a8309a71d7a5d261f776031014881
Stats:     5 lines in 1 file changed: 0 ins; 0 del; 5 mod

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

Reviewed-by: redestad

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

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