RFR: 8331744: java.lang.classfile.TypeKind improvements [v3]

[Claes Redestad]

On Tue, 7 May 2024 01:49:27 GMT, Chen Liang <liach at 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