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+stsypa...@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
