On Wed, 23 Nov 2022 23:33:32 GMT, Volodymyr Paprotski <d...@openjdk.org> wrote:
> Regarding mainline: > - I decided not to 'unroll' the top while loop (i.e. `engineUpdate(byte[] > input, int offset, int len)` is unrolled) > - It is debatable which version is easier to understand. If this version > is 'too complex', I can unroll the top while loop. > - I do think this version is incremental (i.e. easier to review?): > - Move `remaining -= bytesToWrite;` into each `if` branch > - Change first `if` case to process multiple blocks instead of one > > This `while` loop has a lot of cases to remember; Very roughly: > > 1. process from previous call > 2. process current data > 3. store overflow > > Interesting situations: > - `blockOffset` might be non-`0` > - `remaining+blockOffset` might not be enough to fill a single block. Or just > enough for one block and to leave an overflow again.. > - etc. > > Regarding testing > - Correctness of intrinsic was already tested in > https://github.com/openjdk/jdk/pull/10582 so not adding any tests there (i.e. > no KAT) > - In principle, fuzz test should also be sufficient to test bytebuffer (did > increase repetitions) Please add a JMH micro showcasing gains, with heap allocated and native byte buffer (once you support it). src/java.base/share/classes/com/sun/crypto/provider/Poly1305.java line 268: > 266: } else { > 267: while (blockMultipleLength > 0) { > 268: processBlock(buf, BLOCK_LENGTH); For native byte buffers, you can pass the buffer address and base. ------------- PR: https://git.openjdk.org/jdk/pull/11338
