RFR: 8266972: Use String.concat() in j.l.Class where invokedynamic-based String concatenation is not available [v7]
Claes Redestad
redestad at openjdk.java.net
Mon Aug 2 10:52:40 UTC 2021
On Mon, 26 Jul 2021 08:31:23 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.
>
> Сергей Цыпанов has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision:
>
> - Merge branch 'master' into class-concat
> - Merge remote-tracking branch 'origin/class-concat' into class-concat
> - Merge branch 'master' into class-concat
> - Merge branch 'master' into class-concat
> - 8266972: Revert change in Class.descriptorString()
> - Merge branch 'master' into class-concat
> - Merge branch 'master' into class-concat
> - 8266972: Use String.concat() in j.l.Class.toSting()
> - Use String.concat() where invokedynamic-based String concatenation is not available
Looks fine to me.
-------------
Marked as reviewed by redestad (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3627
More information about the core-libs-dev
mailing list