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
