RFR: 8265075: Improve and simplify Class.resolveName()

[Peter Levart]

Hi, Sergey!


Have you measured the code change in the java.lang.Class itself or just 
equivalent code in the JMH test as you show us?


The JMH test may show better results as it is compiled to bytecode that 
uses special invokedynamic-based string concatenation with optimal MH 
based underlying strategy. The code in java.lang.Class can't be compiled 
to use this kind of concatenation because of bootstraping issues and is 
therefore compiled to bytecode that uses StringBuilder directly (much 
like the existing code of the patched method). So I'm wondering whether 
the improvement in speed is actually there...


Regards, Peter


On 4/13/21 2:55 PM, Сергей Цыпанов wrote:
> In mentioned method this code snippet
>
> int len = baseName.length() + 1 + name.length();
> StringBuilder sb = new StringBuilder(len);
> name = sb.append(baseName.replace('.', '/'))
>          .append('/')
>          .append(name)
>          .toString();
>
>
> can be simplified with performance improvement as
>
> name = baseName.replace('.', '/') + '/' + name;
>
> Also null check of `baseName` can be removed as Class.getPackageName() never returns null.
>
> This benchmark
>
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class ResolveNameBenchmark {
>
>    private final Class<? extends ResolveNameBenchmark> klazz = getClass();
>
>    @Benchmark
>    public Object original() {
>      return original("com/tsypanov/ovn/ResolveNameBenchmark.class");
>    }
>
>    @Benchmark
>    public Object patched() {
>      return patched("com/tsypanov/ovn/ResolveNameBenchmark.class");
>    }
>
>    private String original(String name) {
>      if (!name.startsWith("/")) {
>        String baseName = getPackageName();
>        if (baseName != null && !baseName.isEmpty()) {
>          int len = baseName.length() + 1 + name.length();
>          StringBuilder sb = new StringBuilder(len);
>          name = sb.append(baseName.replace('.', '/'))
>                  .append('/')
>                  .append(name)
>                  .toString();
>        }
>      } else {
>        name = name.substring(1);
>      }
>      return name;
>    }
>
>    private String patched(String name) {
>        if (!name.startsWith("/")) {
>            String baseName = getPackageName();
>            if (!baseName.isEmpty()) {
>                return baseName.replace('.', '/') + '/' + name;
>            }
>            return name;
>        }
>        return name.substring(1);
>    }
>
>    private String getPackageName() {
>      return klazz.getPackageName();
>    }
> }
>
> demonstrates good improvement, especially as of memory consumption:
>
>                                              Mode  Cnt     Score     Error   Units
>
> original                                    avgt   50    57.974 ±   0.365   ns/op
> original:·gc.alloc.rate                     avgt   50  3419.447 ±  21.154  MB/sec
> original:·gc.alloc.rate.norm                avgt   50   312.006 ±   0.001    B/op
> original:·gc.churn.G1_Eden_Space            avgt   50  3399.396 ± 149.836  MB/sec
> original:·gc.churn.G1_Eden_Space.norm       avgt   50   310.198 ±  13.628    B/op
> original:·gc.churn.G1_Survivor_Space        avgt   50     0.004 ±   0.001  MB/sec
> original:·gc.churn.G1_Survivor_Space.norm   avgt   50    ≈ 10⁻³              B/op
> original:·gc.count                          avgt   50   208.000            counts
> original:·gc.time                           avgt   50   224.000                ms
>
> patched                                     avgt   50    44.367 ±   0.162   ns/op
> patched:·gc.alloc.rate                      avgt   50  2749.265 ±  10.014  MB/sec
> patched:·gc.alloc.rate.norm                 avgt   50   192.004 ±   0.001    B/op
> patched:·gc.churn.G1_Eden_Space             avgt   50  2729.221 ± 193.552  MB/sec
> patched:·gc.churn.G1_Eden_Space.norm        avgt   50   190.615 ±  13.539    B/op
> patched:·gc.churn.G1_Survivor_Space         avgt   50     0.003 ±   0.001  MB/sec
> patched:·gc.churn.G1_Survivor_Space.norm    avgt   50    ≈ 10⁻⁴              B/op
> patched:·gc.count                           avgt   50   167.000            counts
> patched:·gc.time                            avgt   50   181.000                ms
>
> -------------
>
> Commit messages:
>   - 8265075: Improve and simplify Class.resolveName()
>
> Changes: https://git.openjdk.java.net/jdk/pull/3464/files
>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3464&range=00
>    Issue: https://bugs.openjdk.java.net/browse/JDK-8265075
>    Stats: 11 lines in 1 file changed: 0 ins; 7 del; 4 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/3464.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/3464/head:pull/3464
>
> PR: https://git.openjdk.java.net/jdk/pull/3464