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