On Tue, 7 May 2024 10:46:41 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add a mixed scenario > > Nice improvement to the micro. It might be preferable to use random > generation with a hardcoded or parameterized seed to reduce run-to-run > variation, eg. `new Random(1)`. > > Agree that call sites are unlikely to mix `CD` and `Class`, but we're likely > to see the bimorphicness from passing Primitive and ReferenceCDs. This is > typically a very small and constant call overhead. > > The primitives path seem to be the weak link, and maybe we could take a cue > from `sun.util.invoke.Wrapper` and set up a similar perfect hash table > instead of a switch. This seem somewhat profitable: > > Name (type) Cnt Base Error Test > Error Unit Change > TypeKindBench.fromClassDescs PRIMITIVE 6 196,203 ± 2,469 147,898 ± 8,786 > ns/op 1,33x (p = 0,000*) > TypeKindBench.fromClasses PRIMITIVE 6 325,336 ± 12,203 206,084 ± 2,180 > ns/op 1,58x (p = 0,000*) > > > The `fromClasses PRIMITIVE` case is still a bit behind since getting the > descriptorString takes a few extra (non-allocating) steps. > > Patch: > > diff --git a/src/java.base/share/classes/java/lang/classfile/TypeKind.java > b/src/java.base/share/classes/java/lang/classfile/TypeKind.java > index 5ba566b3d06..dd0a06c63ea 100644 > --- a/src/java.base/share/classes/java/lang/classfile/TypeKind.java > +++ b/src/java.base/share/classes/java/lang/classfile/TypeKind.java > @@ -58,6 +58,7 @@ public enum TypeKind { > > private final String name; > private final String descriptor; > + private final char basicTypeChar; > private final int newarrayCode; > > /** {@return the human-readable name corresponding to this type} */ > @@ -100,6 +101,7 @@ public TypeKind asLoadable() { > TypeKind(String name, String descriptor, int newarrayCode) { > this.name = name; > this.descriptor = descriptor; > + this.basicTypeChar = descriptor.charAt(0); > this.newarrayCode = newarrayCode; > } > > @@ -154,7 +156,29 @@ public static TypeKind fromDescriptor(CharSequence s) { > */ > public static TypeKind from(TypeDescriptor.OfField<?> descriptor) { > return descriptor.isPrimitive() // implicit null check > - ? fromDescriptor(descriptor.descriptorString()) > + ? ofPrimitive(descriptor.descriptorString()) > : TypeKind.ReferenceType; > } > + > + // Perfect hashing for basic types, borrowed from sun.invoke.util.Wrapper > + private static final TypeKind[] FROM_CHAR = new TypeKind[16]; > + > + static { > + for (TypeKind w : values()) { > + if... @cl4es I have tried 3 scenarios with the fixed seed: baseline before your comment, hash table over only primitives (your proposal), hash table over all chars (my uploaded patch) All of these are without the specialization of `isPrimitive()`. I observe that hash for all chars boosts the mixed scenario; cannot explain why exactly, but I go for hashing all chars because it should benefit the general-purpose `fromDescriptor`, used by ClassFile API's implementations too. What do you think? No hash table: Benchmark (type) Mode Cnt Score Error Units TypeKindBench.fromClassDescs PRIMITIVE avgt 6 121.863 ± 3.968 ns/op TypeKindBench.fromClassDescs REFERENCE avgt 6 56.158 ± 1.469 ns/op TypeKindBench.fromClassDescs MIXED avgt 6 141.508 ± 5.412 ns/op TypeKindBench.fromClasses PRIMITIVE avgt 6 232.201 ± 3.988 ns/op TypeKindBench.fromClasses REFERENCE avgt 6 15.663 ± 0.359 ns/op TypeKindBench.fromClasses MIXED avgt 6 155.033 ± 1.927 ns/op Primitive-only hash: Benchmark (type) Mode Cnt Score Error Units TypeKindBench.fromClassDescs PRIMITIVE avgt 6 112.731 ± 1.985 ns/op TypeKindBench.fromClassDescs REFERENCE avgt 6 57.339 ± 1.119 ns/op TypeKindBench.fromClassDescs MIXED avgt 6 126.926 ± 3.432 ns/op TypeKindBench.fromClasses PRIMITIVE avgt 6 154.779 ± 4.008 ns/op TypeKindBench.fromClasses REFERENCE avgt 6 15.619 ± 0.343 ns/op TypeKindBench.fromClasses MIXED avgt 6 132.254 ± 2.133 ns/op Current hash: Benchmark (type) Mode Cnt Score Error Units TypeKindBench.fromClassDescs PRIMITIVE avgt 6 116.446 ± 6.085 ns/op TypeKindBench.fromClassDescs REFERENCE avgt 6 56.399 ± 1.145 ns/op TypeKindBench.fromClassDescs MIXED avgt 6 121.087 ± 2.410 ns/op TypeKindBench.fromClasses PRIMITIVE avgt 6 154.842 ± 4.600 ns/op TypeKindBench.fromClasses REFERENCE avgt 6 15.897 ± 0.454 ns/op TypeKindBench.fromClasses MIXED avgt 6 118.444 ± 3.616 ns/op ------------- PR Comment: https://git.openjdk.org/jdk/pull/19109#issuecomment-2098712190