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

[Alan Bateman]

On Tue, 13 Apr 2021 12:47:50 GMT, Сергей Цыпанов <github.com+10835776+stsypanov at openjdk.org> 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

We can remove the check for baseName being null, that's a left over from iteration during JDK 9.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3464