On Wed, 25 May 2022 14:13:52 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> The bespoke caching scheme in `jl.invoke.LambdaFormEditor.TransformKey`
>> allows keys to be compacted when all byte values of the key fit in 4 bits,
>> otherwise a byte array is allocated and used. This means that all transforms
>> with a kind value above 15 will be forced to allocate and use array
>> comparisons.
>>
>> Removing unused and folding some transforms to ensure all existing kinds can
>> fit snugly within the 0-15 value range realize a minor improvement to
>> footprint, speed and allocation pressure of affected transforms, e.g.
>> ~300bytes/op reduction in the `StringConcatFactoryBootstraps` microbenchmark:
>>
>> Baseline:
>>
>> Benchmark Mode Cnt
>> Score Error Units
>> SCFB.makeConcatWithConstants avgt 15
>> 2048.475 ? 69.887 ns/op
>> SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt 15
>> 3487.311 ? 80.385 B/op
>>
>>
>> Patched:
>>
>> Benchmark Mode Cnt
>> Score Error Units
>> SCFB.makeConcatWithConstants avgt 15
>> 1961.985 ? 101.519 ns/op
>> SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt 15
>> 3156.478 ? 183.600 B/op
>
> Claes Redestad has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Missed correctly taking b1 into account in of(byte, int, int...)
> (java/lang/String/concat/ImplicitStringConcatShapes.java test failure)
LGTM with a couple of suggestions.
src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java line 219:
> 217: return new TransformKey(fullBytes);
> 218: }
> 219: static TransformKey of(byte kind, int b1, int b2, int[] b3456) {
Nit: I can't figure out if `b3456` intends to be a different convention than
the others `b123` and `b234`. I would expect something like this:
Suggestion:
static TransformKey of(byte kind, int b1, int b2, int... b345) {
src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java line 252:
> 250: if (!inRange(bitset))
> 251: return 0;
> 252: pb = pb | b2 << (2 * PACKED_BYTE_SIZE) | b1 <<
> PACKED_BYTE_SIZE | b0;
Suggestion:
pb = pb | packagedBytes(b0, b1, b2);
src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java line 267:
> 265: if (!inRange(bitset))
> 266: return 0;
> 267: pb = pb | b1 << PACKED_BYTE_SIZE | b0;
Suggestion:
pb = pb | packagedBytes(b0, b1);
-------------
Marked as reviewed by mchung (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8881
