RFR: 8253821: Improve ByteBuffer performance with GCM

[Xue-Lei Andrew Fan]

On Tue, 29 Sep 2020 20:22:55 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

> 8253821: Improve ByteBuffer performance with GCM

Impressive update and performance improvement!  I have no major concerns, all comments are just about trivial details
like indents.

src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java line 660:

> 658:     }
> 659:
> 660:         /**

There is one extra indent..

src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java line 664:

> 662:      * engineUpdate() and engineDoFinal().
> 663:      */
> 664:     private int bufferCrypt(ByteBuffer input, ByteBuffer output,

It looks like this method is copied from the CipherSpi.  For maintenance, it would be nice to add an extra comment  to
state the copy and update.  For example, "this method and implementation is copied from javax.crypto.CipherSpi, with an
improvement for GCM mode."

src/java.base/share/classes/com/sun/crypto/provider/CipherCore.java line 947:

> 945:             // create temporary output buffer if the estimated size is larger
> 946:             // than the user-provided buffer.
> 947:             if (output.length - outputOffset < estOutSize) {

"outputCapacity" could be used to replace "output.length - outputOffset", and join the clause with the if-clause above.

src/java.base/share/classes/com/sun/crypto/provider/CipherCore.java line 928:

> 926:         int outputCapacity = checkOutputCapacity(output, outputOffset,
> 927:                 estOutSize);
> 928:         int offset = outputOffset; // 0 for decrypting

the line comment, "// 0 for decrypting", could be remove as the expression get changed.

src/java.base/share/classes/com/sun/crypto/provider/CounterMode.java line 28:

> 26: package com.sun.crypto.provider;
> 27:
> 28: import java.nio.ByteBuffer;

There is no more update except this line.  The new import ByteBuffer may not be used.

src/java.base/share/classes/com/sun/crypto/provider/FeedbackCipher.java line 250:

> 248:      * ByteBuffer methods should not be accessed as CipherCore and AESCipher
> 249:      * copy the data to byte arrays.  These methods are to satisfy the compiler.
> 250:      *

there is an extra blank comment line.

src/java.base/share/classes/com/sun/crypto/provider/GHASH.java line 203:

> 201:
> 202:     // Maximum buffer size rotating ByteBuffer->byte[] intrinsic copy
> 203:     static final int MAX_LEN = 1024;

This filed could be private.  I would like to declare class fields in the beginning of a class, for easy
eyes-searching.  Maybe, it is nice to use a multiple of the block size (for example 64 * AES_BLOCK_SIZE), just in case
someone else update it to a weird value later.

src/java.base/share/classes/com/sun/crypto/provider/GCTR.java line 144:

> 142:
> 143:     // Maximum buffer size rotating ByteBuffer->byte[] intrinsic copy
> 144:     final int MAX_LEN = 1024;

This filed could be private. I would like to declare class fields in the beginning of a class, for easy eyes-searching.
Maybe, it is nice to use a multiple of the block size (for example 64 * AES_BLOCK_SIZE), just in case someone else
update it to a weird value later.

-------------

Marked as reviewed by xuelei (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/411