On Mon, 6 May 2024 22:00:24 GMT, Claes Redestad <redes...@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
