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