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

2021-08-02 Thread Claes Redestad
On Mon, 26 Jul 2021 08:31:23 GMT, Сергей Цыпанов 
 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 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   6091.480 ±   
>> 1.839   ns/op
>> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
>> 4.033B/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.047B/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   6065.016 ±   
>> 3.446   ns/op
>> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
>> 115.719  MB/sec
>> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
>> 0.001B/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.853B/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   6034.273 ±   
>> 0.480   ns/op
>> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
>> 45.113  MB/sec
>> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
>> 0.001B/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.595B/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   6015.787 ±   
>> 0.214   ns/op
>> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
>> 32.339  MB/sec
>> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
>> 0.001B/op
>> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
>> 147.774  MB/sec
>> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
>> 3.511B/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 

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

2021-07-26 Thread Сергей Цыпанов
> 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 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   6091.480 ±   
> 1.839   ns/op
> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
> 4.033B/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.047B/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   6065.016 ±   
> 3.446   ns/op
> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
> 115.719  MB/sec
> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
> 0.001B/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.853B/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   6034.273 ±   
> 0.480   ns/op
> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
> 45.113  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
> 0.001B/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.595B/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   6015.787 ±   
> 0.214   ns/op
> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
> 32.339  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
> 0.001B/op
> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
> 147.774  MB/sec
> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
> 3.511B/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 reflecti

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

2021-07-08 Thread Сергей Цыпанов
> 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 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   6091.480 ±   
> 1.839   ns/op
> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
> 4.033B/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.047B/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   6065.016 ±   
> 3.446   ns/op
> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
> 115.719  MB/sec
> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
> 0.001B/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.853B/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   6034.273 ±   
> 0.480   ns/op
> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
> 45.113  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
> 0.001B/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.595B/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   6015.787 ±   
> 0.214   ns/op
> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
> 32.339  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
> 0.001B/op
> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
> 147.774  MB/sec
> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
> 3.511B/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 reflecti

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

2021-07-08 Thread Сергей Цыпанов
On Wed, 12 May 2021 13:12:07 GMT, Claes Redestad  wrote:

>> But isn't `componentType.descriptorString()` does this itself? Also 
>> multi-dimensional arrays are quite an infrequent usecase, aren't they?
>
> Yeah, it's just an optimization. Current code wouldbuild "[[..[[LFoo;" by 
> recursively going down to `Foo`, build and return "LFoo;", then do "[" + 
> "LFoo;", and so on. Infrequent enough that we can ignore it, sure, but since 
> it's effectively reducing the algorithmic complexity from O(n*m) to O(n+m) I 
> should at least mention it.

I see that `TestPrimitiveArrayCriticalWithBadParam` constantly fails, so I'd 
probably revert this change

-

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


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

2021-07-06 Thread Сергей Цыпанов
> 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 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   6091.480 ±   
> 1.839   ns/op
> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
> 4.033B/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.047B/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   6065.016 ±   
> 3.446   ns/op
> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
> 115.719  MB/sec
> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
> 0.001B/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.853B/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   6034.273 ±   
> 0.480   ns/op
> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
> 45.113  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
> 0.001B/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.595B/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   6015.787 ±   
> 0.214   ns/op
> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
> 32.339  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
> 0.001B/op
> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
> 147.774  MB/sec
> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
> 3.511B/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 reflecti

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

2021-07-02 Thread Сергей Цыпанов
> 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 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   6091.480 ±   
> 1.839   ns/op
> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
> 4.033B/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.047B/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   6065.016 ±   
> 3.446   ns/op
> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
> 115.719  MB/sec
> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
> 0.001B/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.853B/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   6034.273 ±   
> 0.480   ns/op
> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
> 45.113  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
> 0.001B/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.595B/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   6015.787 ±   
> 0.214   ns/op
> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
> 32.339  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
> 0.001B/op
> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
> 147.774  MB/sec
> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
> 3.511B/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 reflecti

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

2021-06-30 Thread Сергей Цыпанов
> 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 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   6091.480 ±   
> 1.839   ns/op
> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
> 4.033B/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.047B/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   6065.016 ±   
> 3.446   ns/op
> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
> 115.719  MB/sec
> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
> 0.001B/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.853B/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   6034.273 ±   
> 0.480   ns/op
> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
> 45.113  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
> 0.001B/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.595B/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   6015.787 ±   
> 0.214   ns/op
> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
> 32.339  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
> 0.001B/op
> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
> 147.774  MB/sec
> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
> 3.511B/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 reflecti

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

2021-06-10 Thread Сергей Цыпанов
> 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 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   6091.480 ±   
> 1.839   ns/op
> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
> 4.033B/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.047B/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   6065.016 ±   
> 3.446   ns/op
> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
> 115.719  MB/sec
> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
> 0.001B/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.853B/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   6034.273 ±   
> 0.480   ns/op
> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
> 45.113  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
> 0.001B/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.595B/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   6015.787 ±   
> 0.214   ns/op
> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
> 32.339  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
> 0.001B/op
> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
> 147.774  MB/sec
> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
> 3.511B/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 reflecti

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

2021-06-10 Thread Сергей Цыпанов
On Thu, 22 Apr 2021 14:07:20 GMT, Сергей Цыпанов 
 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 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   6091.480 ±   
> 1.839   ns/op
> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
> 4.033B/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.047B/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   6065.016 ±   
> 3.446   ns/op
> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
> 115.719  MB/sec
> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
> 0.001B/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.853B/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   6034.273 ±   
> 0.480   ns/op
> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
> 45.113  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
> 0.001B/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.595B/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   6015.787 ±   
> 0.214   ns/op
> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
> 32.339  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
> 0.001B/op
> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
> 147.774  MB/sec
> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
> 3.511B/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
>  

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

2021-05-13 Thread Michael Kroll
Okay, that makes sense.
Thanks for clarifying.

Greetings,
Michael


Am 13. Mai 2021 00:45:13 MESZ schrieb Claes Redestad 
:
>Hi,
>
>I gave this some thought the other day when looking at one of Sergey's
>patches.
>
>I'm no expert on javac but I think translating to String::concat is
>likely to be a bit harder than it seems. It's not a static method, so
>the left hand side must be non-null. This is something I believe javac
>can't prove in the general case. Corner cases such as when the left
>hand side is a String literal, of course, but that would be of limited
>utility.
>
>There are more convoluted solutions that might work if one squints (add
>public static concat methods and translate calls into those?), but the
>question we need to be asking ourselves is really if it is worth our
>time - and the added complexity? The answer is likely no.
>
>It might help some if you're targeting java 8 or using javac as a
>frontend for non-JVM targets (lol!), but going forward I think it would
>mostly help java.base and a few other JDK modules where the
>invokedynamic translation can't be used due bootstrapping issues. And
>in
>most of those cases it won't really matter for performance anyhow.
>
>That's why I think using String::concat on a case-by-case basis in
>those
>few OpenJDK modules where indified string concat is not applicable is
>enough. When we see that it helps. And then only sparingly.
>
>Hope this clarifies how I reason about this.
>
>/Claes
>
>
>On 2021-05-12 22:49, Michael Kroll wrote:
>> Hello,
>> 
>> just being curious here:
>> Before we start to change every usage of String+String into
>String.concat, shouldn't the compiler be so smart to do that for us?
>> Currently it compiles to invokedynamic if available and to using
>StringBuilder otherwise. Now why doesn't it compile to String.concat
>instead of StringBuilder for the case when invokedynamic is not
>available as target?
>> 
>> greets,
>> Michael
>> 
>> 
>> 
>> 
>> Am 12. Mai 2021 15:04:21 MESZ schrieb "Сергей Цыпанов"
>:
>>> 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 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.033B/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.047B/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.rateavgt   60 
>2844.240
>>> ± 115.719  MB/sec
>>> descriptorString:·gc.alloc.rate.norm   avgt   60  
>288.006
>>> ±   0.001B/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.853B/op
>>> descriptorString:·gc.churn.G1_Survivor_Sp

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

2021-05-12 Thread Сергей Цыпанов
> Before we start to change every usage of String+String into String.concat, 
> shouldn't the compiler be so smart to do that for us?
> Currently it compiles to invokedynamic if available and to using 
> StringBuilder otherwise. Now why doesn't it compile to String.concat instead 
> of StringBuilder for the case when invokedynamic is not available as target?

I think the main reason is that

String str = "smth is " + null;

returns "smth is null" and with current implementation of String.concat()
the same expression throws NPE. See the code of String.concat():

public String concat(String str) {
if (str.isEmpty()) {
return this;
}
return StringConcatHelper.simpleConcat(this, str);
}

Regards,
Sergey


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

2021-05-12 Thread Claes Redestad

Hi,

I gave this some thought the other day when looking at one of Sergey's
patches.

I'm no expert on javac but I think translating to String::concat is
likely to be a bit harder than it seems. It's not a static method, so
the left hand side must be non-null. This is something I believe javac
can't prove in the general case. Corner cases such as when the left
hand side is a String literal, of course, but that would be of limited
utility.

There are more convoluted solutions that might work if one squints (add
public static concat methods and translate calls into those?), but the
question we need to be asking ourselves is really if it is worth our
time - and the added complexity? The answer is likely no.

It might help some if you're targeting java 8 or using javac as a
frontend for non-JVM targets (lol!), but going forward I think it would
mostly help java.base and a few other JDK modules where the
invokedynamic translation can't be used due bootstrapping issues. And in
most of those cases it won't really matter for performance anyhow.

That's why I think using String::concat on a case-by-case basis in those
few OpenJDK modules where indified string concat is not applicable is
enough. When we see that it helps. And then only sparingly.

Hope this clarifies how I reason about this.

/Claes


On 2021-05-12 22:49, Michael Kroll wrote:

Hello,

just being curious here:
Before we start to change every usage of String+String into String.concat, 
shouldn't the compiler be so smart to do that for us?
Currently it compiles to invokedynamic if available and to using StringBuilder 
otherwise. Now why doesn't it compile to String.concat instead of StringBuilder 
for the case when invokedynamic is not available as target?

greets,
Michael




Am 12. Mai 2021 15:04:21 MESZ schrieb "Сергей Цыпанов" 
:

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 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   6091.480
±   1.839   ns/op
descriptorString:·gc.alloc.rate.norm   avgt   60   404.008
±   4.033B/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.047B/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   6065.016
±   3.446   ns/op
descriptorString:·gc.alloc.rateavgt   60  2844.240
± 115.719  MB/sec
descriptorString:·gc.alloc.rate.norm   avgt   60   288.006
±   0.001B/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.853B/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   6034.273
±   0.480   ns/op
typeName:·gc.alloc.rateavgt   60  3265.356
±  45.113  MB/sec
typeName

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

2021-05-12 Thread Michael Kroll
Hello,

just being curious here:
Before we start to change every usage of String+String into String.concat, 
shouldn't the compiler be so smart to do that for us?
Currently it compiles to invokedynamic if available and to using StringBuilder 
otherwise. Now why doesn't it compile to String.concat instead of StringBuilder 
for the case when invokedynamic is not available as target?

greets,
Michael




Am 12. Mai 2021 15:04:21 MESZ schrieb "Сергей Цыпанов" 
:
>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 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   6091.480
>±   1.839   ns/op
>descriptorString:·gc.alloc.rate.norm   avgt   60   404.008
>±   4.033B/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.047B/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   6065.016
>±   3.446   ns/op
>descriptorString:·gc.alloc.rateavgt   60  2844.240
>± 115.719  MB/sec
>descriptorString:·gc.alloc.rate.norm   avgt   60   288.006
>±   0.001B/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.853B/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   6034.273
>±   0.480   ns/op
>typeName:·gc.alloc.rateavgt   60  3265.356
>±  45.113  MB/sec
>typeName:·gc.alloc.rate.norm   avgt   60   176.004
>±   0.001B/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.595B/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   6015.787
>±   0.214   ns/op
>typeName:·gc.alloc.rateavgt   60  2577.554
>±  32.339  MB/sec
>typeName:·gc.alloc.rate.norm   avgt   6064.001
>±   0.001B/op
>typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039
>± 147.774  MB/sec
>typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895
>±   3.511B/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⁻⁴ 
>

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

2021-05-12 Thread Claes Redestad
On Wed, 12 May 2021 11:00:25 GMT, Сергей Цыпанов 
 wrote:

>> 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?

Yeah, it's just an optimization. Current code wouldbuild "[[..[[LFoo;" by 
recursively going down to `Foo`, build and return "LFoo;", then do "[" + 
"LFoo;", and so on. Infrequent enough that we can ignore it, sure, but since 
it's effectively reducing the algorithmic complexity from O(n*m) to O(n+m) I 
should at least mention it.

-

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


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

2021-05-12 Thread Сергей Цыпанов
On Wed, 12 May 2021 09:59:32 GMT, Claes Redestad  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 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   6091.480 ±   
>> 1.839   ns/op
>> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
>> 4.033B/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.047B/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   6065.016 ±   
>> 3.446   ns/op
>> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
>> 115.719  MB/sec
>> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
>> 0.001B/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.853B/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   6034.273 ±   
>> 0.480   ns/op
>> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
>> 45.113  MB/sec
>> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
>> 0.001B/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.595B/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   6015.787 ±   
>> 0.214   ns/op
>> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
>> 32.339  MB/sec
>> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
>> 0.001B/op
>> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
>> 147.774  MB/sec
>> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
>> 3.511B/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  

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

2021-05-12 Thread Claes Redestad
On Thu, 22 Apr 2021 14:07:20 GMT, Сергей Цыпанов 
 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 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   6091.480 ±   
> 1.839   ns/op
> descriptorString:·gc.alloc.rate.norm   avgt   60   404.008 ±   
> 4.033B/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.047B/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   6065.016 ±   
> 3.446   ns/op
> descriptorString:·gc.alloc.rateavgt   60  2844.240 ± 
> 115.719  MB/sec
> descriptorString:·gc.alloc.rate.norm   avgt   60   288.006 ±   
> 0.001B/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.853B/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   6034.273 ±   
> 0.480   ns/op
> typeName:·gc.alloc.rateavgt   60  3265.356 ±  
> 45.113  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   60   176.004 ±   
> 0.001B/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.595B/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   6015.787 ±   
> 0.214   ns/op
> typeName:·gc.alloc.rateavgt   60  2577.554 ±  
> 32.339  MB/sec
> typeName:·gc.alloc.rate.norm   avgt   6064.001 ±   
> 0.001B/op
> typeName:·gc.churn.G1_Eden_Space   avgt   60  2574.039 ± 
> 147.774  MB/sec
> typeName:·gc.churn.G1_Eden_Space.norm  avgt   6063.895 ±   
> 3.511B/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
>