Optimize sun.invoke.util.BytecodeDescriptor.unparse

[Christoph Dreis]

HI,

I hate pinging, but I would really appreciate it if someone finds the time to take a look at my suggestion below from last week.
Or is this maybe the wrong mailing list - given that it is in the "sun" root package?

Cheers,
Christoph

===== ORIGINAL MAIL =====

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();
+        }
     }