On Mon, 26 Oct 2026 20:11:43 GMT, Vladimir Kozlov <[email protected]> wrote:
>> Hi @vnkozlov, thank you for the review comment. I understand the concern
>> about complexity.
>>
>>> I am not sure about this optimization. On one hand, the performance
>>> improvement of the **micro** benchmark is impressive. On the other hand,
>>> your changes complicated the code which became harder to understand and
>>> maintain.
>>
>> To clarify, the change does not introduce a new algorithm: it makes the
>> non-AVX-512 reduction follow the same L_128_done + L_barrett sequence
>> already used by kernel_crc32_avx512(), with the same K_128_64/K_64_32
>> constants and masks. The adaptations for kernel_crc32() are mechanical only:
>> the constants come from _crc_by128_masks, and the masks are built in a
>> register instead of being loaded from the AVX-512 mask table.
>>
>> Would you consider this acceptable, or would you prefer a different way to
>> structure/comment the reduction? I'll revise accordingly.
>
> @LangruiZhou please re-run your performance testing with your latest changes.
@vnkozlov hi, I've removed the unused `fold_8bit_crc32()` and re-ran the micro
test `test/micro/org/openjdk/bench/java/util/TestCRC32.java` on the same
config, As expected, the benchmark results are essentially identical to the
previous revision:
- env: Intel Xeon 6980P (Granite Rapids), single socket, 128 cores / 256
threads (SMT2), 3 NUMA nodes, 755 GiB RAM, Ubuntu 22.04.4 LTS, glibc 2.35
- JMH: test/micro/org/openjdk/bench/java/util/TestCRC32.java
- jvmArgs: "-Xms4g -Xmx4g -XX:UseAVX=2"
- results (ops/ms)
size | baseline | patched | delta
-- | -- | -- | --
16 | 23588.54 | 23592.13 | +0.0%
32 | 14587.51 | 19580.11 | +34.2%
64 | 13844.55 | 18284.63 | +32.1%
128 | 12564.18 | 16084.47 | +28.0%
256 | 12075.04 | 15214.56 | +26.0%
512 | 11152.53 | 13801.76 | +23.8%
1024 | 9622.52 | 11596.26 | +20.5%
2048 | 7371.23 | 8552.20 | +16.0%
4096 | 4641.69 | 5057.77 | +9.0%
16384 | 1433.73 | 1473.04 | +2.7%
65536 | 374.96 | 378.64 | +1.0%
262144 | 96.44 | 96.68 | +0.2%
-------------
PR Comment: https://git.openjdk.org/jdk/pull/33342#issuecomment-6434677638
