On Tue, 3 Nov 2026 22:37:25 GMT, Volodymyr Paprotski <[email protected]>
wrote:
>> I added an AVX2 version of AES/CTR intrinsic. Rather then adding a third
>> version of that intrinsic (we already have an AVX512 and an AES128), I
>> refactored existing ones so I could parametrize just one intrinsic. AVX512
>> got better at small payloads and AVX128 got better at large payloads. And
>> ECore/AVX256 used to use fall back to 128, now it doesn't.
>>
>> | Benchmark | Data | Key | AVX128 | AVX256 | AVX512 |
>> |---|---|---|---|---|---|
>> | AESBench.encrypt | 16 | 128 | 0% | 5% | 92% |
>> | AESBench.encrypt | 32 | 128 | 0% | 1% | 37% |
>> | AESBench.encrypt | 64 | 128 | 23% | 33% | 30% |
>> | AESBench.encrypt | 128 | 128 | 6% | 1% | 35% |
>> | AESBench.encrypt | 256 | 128 | 26% | 16% | 4% |
>> | AESBench.encrypt | 1024 | 128 | 29% | 13% | 0% |
>> | AESBench.encrypt | 16384 | 128 | 24% | 13% | 7% |
>> | AESBench.encrypt | 16 | 192 | 3% | 5% | 69% |
>> | AESBench.encrypt | 32 | 192 | 1% | 4% | 39% |
>> | AESBench.encrypt | 64 | 192 | 30% | 42% | 30% |
>> | AESBench.encrypt | 128 | 192 | 12% | 13% | 30% |
>> | AESBench.encrypt | 256 | 192 | 32% | 19% | 6% |
>> | AESBench.encrypt | 1024 | 192 | 25% | 15% | 1% |
>> | AESBench.encrypt | 16384 | 192 | 20% | 12% | 8% |
>> | AESBench.encrypt | 16 | 256 | -1% | 4% | 62% |
>> | AESBench.encrypt | 32 | 256 | 2% | 5% | 31% |
>> | AESBench.encrypt | 64 | 256 | 31% | 67% | 18% |
>> | AESBench.encrypt | 128 | 256 | 13% | 16% | 29% |
>> | AESBench.encrypt | 256 | 256 | 33% | 18% | 4% |
>> | AESBench.encrypt | 1024 | 256 | 23% | 15% | -2% |
>> | AESBench.encrypt | 16384 | 256 | 19% | 13% | 9% |
>>
>> Just note, measuring exact performance with our current AESBench JMH, and
>> especially at small payloads, can be an exercise in frustration. The
>> benchmark constantly allocates the result and discards it, producing a lot
>> of GC. (While developing, I instrumented the code to call the intrinsic
>> directly, but that involved making many methods public.. so not something
>> that can be checked in.)
>>
>> This is the invocation that gave me the most stable/repeatable results
>> (still swings on reruns, but not as much.. a very long run however).
>>
>>
>> HEAP="-Xms4g -Xmx4g"
>> JMH="-f 3 -i 10 -r 3 -wi 5 -w 5"
>> NEW=./build/linux-x86_64-server-release
>> BASE=./build/base
>> BENCH=org.openjdk.bench.javax.crypto.full.AESBench
>> SIZES="16 32 64 128 256 1024 16384"
>> ARGS="-p algorithm=AES/CTR/NoPadding"
>> for size in $SIZES; do
>> $NEW/images/jdk/bin/java -jar $NEW/images/test/micro/benchmarks.jar
>> $JMH -jvmArgsAppend "$HEAP" $ARGS -p dataSize=$size $BENCH 2>&1 | te...
>
> Volodymyr Paprotski has updated the pull request incrementally with one
> additional commit since the last revision:
>
> comments from Sandhya
src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 425:
> 423: // incr rest of counters
> 424: // LOOP:
> 425: // full aesrounds(4 per key)
There is an assumption here that the maxParallel is 4.
src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 541:
> 539:
> 540: // Constants/parameters
> 541: int totalRegs = vector_len == Assembler::AVX_512bit ||
> VM_Version::supports_avx512vl() ? 32 : 16;
It might be easier to say supports_evex() instead of (Assembler::AVX_512bit ||
VM_Version::supports_avx512vl()).
src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 577:
> 575: }
> 576:
> 577: // if (len == 0) return len
// if (len <=0) return len
src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 581:
> 579: __ cmpl(len, 0);
> 580: __ jcc(Assembler::belowEqual, ExitLabel);
> 581:
We could move this earlier, say at line 568.
src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 620:
> 618: // storing nextCtr{Low,High} delayed, get to AES faster
> 619:
> 620: // First iteration Incremenent is aranged for the unpack to work
Typo: Incremenent
Typo: aranged
src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 693:
> 691:
> 692: // Increment is now a constant step, 2 Ctrs at a time
> 693: __ vpbroadcastq(Increment, 2*blocksPerReg, vector_len, tmp);
May be better to say parallel*blocksPerReg?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/33128#discussion_r4474055296
PR Review Comment: https://git.openjdk.org/jdk/pull/33128#discussion_r4471070906
PR Review Comment: https://git.openjdk.org/jdk/pull/33128#discussion_r4474218930
PR Review Comment: https://git.openjdk.org/jdk/pull/33128#discussion_r4471158168
PR Review Comment: https://git.openjdk.org/jdk/pull/33128#discussion_r4474418847
PR Review Comment: https://git.openjdk.org/jdk/pull/33128#discussion_r4474672887
