On Mon, 26 Jul 2021 08:31:23 GMT, Сергей Цыпанов <github.com+10835776+stsypa...@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
