RFR: 8331744: java.lang.classfile.TypeKind improvements

[Chen Liang]

On Mon, 6 May 2024 22:00:24 GMT, Claes Redestad <redestad 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.
>
> Leaning on `TypeDescriptor.OfField::isPrimitive` presents an opportunity: override `isPrimitive` in `PrimitiveClassDescImpl` and `ReferenceClassDescImpl` to return `true` and `false` respectively: 
> 
> 
> Name                            (type) Cnt    Base    Error     Test    Error  Unit  Change
> TypeKindBench.fromClassDescs PRIMITIVE   6 199,765 ±  3,370  205,531 ±  2,632 ns/op   0,97x (p = 0,000*)
> TypeKindBench.fromClassDescs REFERENCE   6  75,018 ±  1,113   25,925 ±  1,145 ns/op   2,89x (p = 0,000*)
> TypeKindBench.fromClasses    PRIMITIVE   6 344,477 ± 46,310  366,135 ± 54,955 ns/op   0,94x (p = 0,066 )
> TypeKindBench.fromClasses    REFERENCE   6  23,338 ±  0,467   23,183 ±  1,357 ns/op   1,01x (p = 0,484 )
>   * = significant
> 
> 
> Interestingly this has a tiny regression for the primitive case - in this micro. Probably an effect of the default `descriptor.length() == 1` implementation acting as a sort of prefetch the value we'll switch on (`descriptor.charAt(0)`) down this path. Only overriding for `ReferenceClassDescImpl` is neutral, but maybe that's overfitting:
> 
> 
> Name                            (type) Cnt    Base    Error     Test    Error  Unit  Change
> TypeKindBench.fromClassDescs PRIMITIVE   6 199,765 ±  3,370  196,203 ±  2,469 ns/op   1,02x (p = 0,000*)
> TypeKindBench.fromClassDescs REFERENCE   6  75,018 ±  1,113   25,311 ±  0,138 ns/op   2,96x (p = 0,000*)
> TypeKindBench.fromClasses    PRIMITIVE   6 344,477 ± 46,310  325,336 ± 12,203 ns/op   1,06x (p = 0,035 )
> TypeKindBench.fromClasses    REFERENCE   6  23,338 ±  0,467   23,462 ±  3,239 ns/op   0,99x (p = 0,805 )
>   * = significant

@cl4es
I think the benchmark results might be biased due to JIT optimizing for all-primitive or all-reference scenarios, while in practice these are more likely to be mixed (still, I assume a user usually just pass all CD or Class into from, no polymorphism there). The new MIXED type provides more interesting insights and is likely closer to production usages.

I specialized `isPrimitive` https://gist.github.com/c16f30a657648531edbf98fcbedb41e7
And the results are quite distinct from yours, as in my scenario, all ClassDescs see a consistent performance increase: (The Class versions see a negligible decrease, which I believe is just noise)

Baseline:

Benchmark                        (type)  Mode  Cnt    Score   Error  Units
TypeKindBench.fromClassDescs  PRIMITIVE  avgt    6  121.749 ± 6.362  ns/op
TypeKindBench.fromClassDescs  REFERENCE  avgt    6   56.299 ± 2.032  ns/op
TypeKindBench.fromClassDescs      MIXED  avgt    6  138.690 ± 3.376  ns/op
TypeKindBench.fromClasses     PRIMITIVE  avgt    6  224.747 ± 3.721  ns/op
TypeKindBench.fromClasses     REFERENCE  avgt    6   15.850 ± 0.610  ns/op
TypeKindBench.fromClasses         MIXED  avgt    6  134.017 ± 1.014  ns/op


Override:

Benchmark                        (type)  Mode  Cnt    Score   Error  Units
TypeKindBench.fromClassDescs  PRIMITIVE  avgt    6  109.005 ± 2.947  ns/op
TypeKindBench.fromClassDescs  REFERENCE  avgt    6   16.917 ± 0.242  ns/op
TypeKindBench.fromClassDescs      MIXED  avgt    6   97.376 ± 2.235  ns/op
TypeKindBench.fromClasses     PRIMITIVE  avgt    6  227.644 ± 4.851  ns/op
TypeKindBench.fromClasses     REFERENCE  avgt    6   16.375 ± 0.233  ns/op
TypeKindBench.fromClasses         MIXED  avgt    6  146.652 ± 4.549  ns/op

-------------

PR Comment: https://git.openjdk.org/jdk/pull/19109#issuecomment-2097271752