On Tue, 7 May 2024 01:49:27 GMT, Chen Liang <li...@openjdk.org> wrote:
>> A peek into TypeKind during the research for #19105 reveals that TypeKind >> has a few issues: >> 1. Name mismatch for `newarraycode` and `fromNewArrayCode`: Renamed both to >> use "newarray code" >> 2. `fromDescriptor` can throw IOOBE if the input string is empty: changed to >> throw IAE and added tests. >> 3. `from(Class)` can be slow due to descriptor computation: added benchmark, >> will share result in next comment (as it may change with code changes). >> >> The first 2 changes involves API changes, and a CSR has been created. >> Requesting @asotona for a review. > > 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 (w == ReferenceType) { + continue; + } + char c = w.descriptor.charAt(0); + FROM_CHAR[(c + (c >> 1)) & 0xf] = w; + } + } + + private static TypeKind ofPrimitive(String descriptor) { + char basicTypeChar = descriptor.charAt(0); + TypeKind tk = FROM_CHAR[(basicTypeChar + (basicTypeChar >> 1)) & 0xf]; + if (tk == null || tk.basicTypeChar != basicTypeChar) { + throw new IllegalArgumentException("bad descriptor: " + descriptor); + } + return tk; + } } ``` ------------- PR Comment: https://git.openjdk.org/jdk/pull/19109#issuecomment-2098060815