On Thu, 6 May 2021 16:34:04 GMT, Peter Levart <plev...@openjdk.org> wrote:
>> Hello, from discussion in https://github.com/openjdk/jdk/pull/3464 and >> https://github.com/openjdk/jdk/pull/2212 it appears, that in `j.l.Class` >> expressions like >> >> String str = baseName.replace('.', '/') + '/' + name; >> >> are not compiled into invokedynamic-based code, but into one using >> `StringBuilder`. >> >> This happens due to some bootstraping issues. Currently the bytecode for the >> last (most often used) branch of `Class.descriptorString()` looks like >> >> public sb()Ljava/lang/String; >> L0 >> LINENUMBER 21 L0 >> NEW java/lang/StringBuilder >> DUP >> INVOKESPECIAL java/lang/StringBuilder.<init> ()V >> ASTORE 1 >> L1 >> LINENUMBER 23 L1 >> ALOAD 1 >> LDC "a" >> INVOKEVIRTUAL java/lang/StringBuilder.append >> (Ljava/lang/String;)Ljava/lang/StringBuilder; >> POP >> L2 >> LINENUMBER 24 L2 >> ALOAD 1 >> LDC "b" >> INVOKEVIRTUAL java/lang/StringBuilder.append >> (Ljava/lang/String;)Ljava/lang/StringBuilder; >> POP >> L3 >> LINENUMBER 26 L3 >> ALOAD 1 >> INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; >> ARETURN >> >> Here the `StringBuilder` is created with default constructor and then >> expands if necessary while appending. >> >> This can be improved by manually allocating `StringBuilder` of exact size. >> The benchmark demonstrates measurable improvement: >> >> @State(Scope.Benchmark) >> @BenchmarkMode(Mode.AverageTime) >> @OutputTimeUnit(TimeUnit.NANOSECONDS) >> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"}) >> public class ClassDescriptorStringBenchmark { >> >> private final Class<?> clazzWithShortDescriptor = Object.class; >> private final Class<?> clazzWithLongDescriptor = getClass(); >> >> @Benchmark >> public String descriptorString_short() { >> return clazzWithShortDescriptor.descriptorString(); >> } >> >> @Benchmark >> public String descriptorString_long() { >> return clazzWithLongDescriptor.descriptorString(); >> } >> } >> >> >> >> original >> -Xint >> Mode Score Error >> Units >> descriptorString_long avgt 6326.478 ± 107.251 >> ns/op >> descriptorString_short avgt 5220.729 ± 103.545 >> ns/op >> descriptorString_long:·gc.alloc.rate.norm avgt 528.089 ± 0.021 >> B/op >> descriptorString_short:·gc.alloc.rate.norm avgt 232.036 ± 0.015 >> B/op >> >> -XX:TieredStopAtLevel=1 >> Mode Score Error >> Units >> descriptorString_long avgt 230.223 ± 1.254 >> ns/op >> descriptorString_short avgt 164.255 ± 0.755 >> ns/op >> descriptorString_long:·gc.alloc.rate.norm avgt 528.046 ± 0.002 >> B/op >> descriptorString_short:·gc.alloc.rate.norm avgt 232.022 ± 0.001 >> B/op >> >> full >> Mode Score Error >> Units >> descriptorString_long avgt 74.835 ± 0.262 >> ns/op >> descriptorString_short avgt 43.822 ± 0.788 >> ns/op >> descriptorString_long:·gc.alloc.rate.norm avgt 504.010 ± 0.001 >> B/op >> descriptorString_short:·gc.alloc.rate.norm avgt 208.004 ± 0.001 >> B/op >> >> ------------------------ >> patched >> -Xint >> Mode Score Error >> Units >> descriptorString_long avgt 4485.994 ± 60.173 >> ns/op >> descriptorString_short avgt 3949.965 ± 278.143 >> ns/op >> descriptorString_long:·gc.alloc.rate.norm avgt 336.051 ± 0.004 >> B/op >> descriptorString_short:·gc.alloc.rate.norm avgt 184.027 ± 0.010 >> B/op >> >> -XX:TieredStopAtLevel=1 >> Mode Score Error >> Units >> descriptorString_long avgt 185.774 ± 1.100 >> ns/op >> descriptorString_short avgt 135.338 ± 1.066 >> ns/op >> descriptorString_long:·gc.alloc.rate.norm avgt 336.030 ± 0.001 >> B/op >> descriptorString_short:·gc.alloc.rate.norm avgt 184.019 ± 0.001 >> B/op >> >> full >> Mode Score Error >> Units >> descriptorString_long avgt 42.864 ± 0.160 >> ns/op >> descriptorString_short avgt 27.255 ± 0.381 >> ns/op >> descriptorString_long:·gc.alloc.rate.norm avgt 224.005 ± 0.001 >> B/op >> descriptorString_short:·gc.alloc.rate.norm avgt 120.002 ± 0.001 >> B/op >> >> Same can be done also for Class.isHidden() branch in >> Class.descriptorString() and for Class.getCanonicalName0() > > Hi Sergei, > You are right that what javac generates is sub-optimal as it doesn't take > into account the possible known final lenght of the string. So manually doing > so is better. Since your 1st attempt a patch for String.join() method > improved it quite a bit and is now not using StringBuilder under the hood any > more. Could you try to measure for example the following: > > > String str = String.join("/", baseName.replace('.', '/'), name); > > as an alternative to: > > String str = baseName.replace('.', '/') + '/' + name; > > > or for example: > > > return String.join( > "", // delimiter > "L", name.substring(0, index).replace('.', '/'), > ".", name.substring(index+1), ";"); > > as an alternative for: > > return "L" + name.substring(0, index).replace('.', '/') > + "." + name.substring(index+1) + ";"; > > and see how it compares? > > Regards, Peter > @plevart hi, to my surprise `String.join()` makes it worse: > Yeah, it seems JIT does a very good job with StringBuilder in this form. ------------- PR: https://git.openjdk.java.net/jdk/pull/3903