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

[Chen Liang]

On Tue, 7 May 2024 10:46:41 GMT, Claes Redestad <redestad at 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