On Mon, 10 Jun 2024 18:43:49 GMT, Vladimir Kozlov <k...@openjdk.org> wrote:
>> This PR removes some unnecessary conversions between byte arrays and long
>> arrays during SHA3 digest computations.
>
> src/java.base/share/classes/sun/security/provider/SHA3.java line 100:
>
>> 98: b2lLittle(b, ofs, longBuf, 0, blockSize);
>> 99: for (int i = 0; i < blockSize / 8; i++) {
>> 100: state[i] ^= longBuf[i];
>
> Clever. So the intrinsic (C2 code) still generates code corresponding
> original loop with `byte b[]` array. This will be confusing. It will also
> slowdown execution in Interpreter so - additional array copy.
>
> New code also assumes that `buffer.length == blockSize` and `(buffer.length %
> 8) == 0`. I hope there is some assertions/checks in java code to verify that.
>
> Some one from core-libs have to review this.
Well, the intrinsic function treats the input and state as long arrays anyways,
and so it only works on little endian architectures, where the conversion is a
no-op. There is no additional array copy, this b2lLittle() call used to be in
the keccak() method (along with the conversion back to byte array), the point
of this whole change is that only one of these conversions should be done with
every keccak() call (an additional benefit is that the xor and the
corresponding loads+store is done on longs, not on bytes).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19632#discussion_r1633830998
