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

If you have the time, then looking for usages of getPackageName where it checks for null would be useful. We can close this PR and open a new issue/PR for that cleanup.

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

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