Re: Optimize sun.invoke.util.BytecodeDescriptor.unparse

[Roger Riggs]

Hi Chris,

Inlining and simplifying unparseSig(cl, sb) seems straightforward.
Though I wonder if performs differently than just calling 
t.descriptorString()?

The first action of Class.descriptorString is to check for primitives 
and return the basicTypeString
and if not a primitive it calls Class.descriptorString().

It should be equivalent, without extra checks.

Regards, Roger


On 8/13/20 1:31 PM, Christoph Dreis wrote:
Hi,

I just stumbled upon sun.invoke.util.BytecodeDescriptor.unparse that seems to 
unnecessarily create a StringBuilder and checks for the given type to be of 
Object.class twice in certain scenarios.

When I apply the attached patch below with the following isolated benchmark:

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.Thread)
public class MyBenchmark {

        @State(Scope.Thread)
        public static class BenchmarkState {
                private Class<?> test = String.class; // long.class;
        }

        @Benchmark
        public String unparseNew(BenchmarkState state) {
                return BytecodeDescriptor.unparseNew(state.test);
        }

        @Benchmark
        public String unparseOld(BenchmarkState state) {
                return BytecodeDescriptor.unparseOld(state.test);
        }
}

I get the following results:
String.class
Benchmark                                   Mode  Cnt     Score     Error   
Units
MyBenchmark.unparseNew                      avgt   10    47,207 ±   1,918   
ns/op
MyBenchmark.unparseNew:·gc.alloc.rate.norm  avgt   10   232,011 ±   0,002    
B/op
MyBenchmark.unparseOld                      avgt   10    87,197 ±  22,843   
ns/op
MyBenchmark.unparseOld:·gc.alloc.rate.norm  avgt   10   384,020 ±   0,001    
B/op
                                                                                   
long.class
Benchmark                                   Mode  Cnt     Score     Error   
Units
MyBenchmark.unparseNew                      avgt   10     4,996 ±    0,022   
ns/op
MyBenchmark.unparseNew:·gc.alloc.rate.norm  avgt   10    ≈ 10⁻⁶               
B/op
MyBenchmark.unparseOld                      avgt   10    13,303 ±    6,305   
ns/op
MyBenchmark.unparseOld:·gc.alloc.rate.norm  avgt   10    80,003 ±    0,001    
B/op

As you can see the new way makes things allocation free for primitives and also 
improves normal classes.

It seems like a relatively trivial improvement. In case you think this is 
worthwhile, I would appreciate it if someone could sponsor the change.

Cheers,
Christoph

======= PATCH =======
--- a/src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java       
Thu Aug 13 09:33:28 2020 -0700
+++ b/src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java       
Thu Aug 13 19:27:26 2020 +0200
@@ -110,9 +110,13 @@
          } else if (type == int.class) {
              return "I";
          }
-        StringBuilder sb = new StringBuilder();
-        unparseSig(type, sb);
-        return sb.toString();
+        Wrapper basicType = Wrapper.forBasicType(type);
+        char c = basicType.basicTypeChar();
+        if (c != 'L') {
+            return basicType.basicTypeString();
+        } else {
+            return type.descriptorString();
+        }
      }