RFR: 8266972: Use String.concat() in j.l.Class where invokedynamic-based String concatenation is not available
Michael Kroll
michaelkroll at mail.de
Thu May 13 11:52:53 UTC 2021
Okay, that makes sense.
Thanks for clarifying.
Greetings,
Michael
Am 13. Mai 2021 00:45:13 MESZ schrieb Claes Redestad <claes.redestad at oracle.com>:
>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 "Сергей Цыпанов"
><github.com+10835776+stsypanov at openjdk.java.net>:
>>> 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.
>>>
>>> -------------
>>>
>>> Commit messages:
>>> - Use String.concat() where invokedynamic-based String concatenation
>is
>>> not available
>>>
>>> Changes: https://git.openjdk.java.net/jdk/pull/3627/files
>>> Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3627&range=00
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8266972
>>> Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
>>> Patch: https://git.openjdk.java.net/jdk/pull/3627.diff
>>> Fetch: git fetch https://git.openjdk.java.net/jdk
>>> pull/3627/head:pull/3627
>>>
>>> PR: https://git.openjdk.java.net/jdk/pull/3627
More information about the core-libs-dev
mailing list