RFR: 8318756 Create better internal buffer for AEADs [v4]

Daniel Jeliński djelinski at openjdk.org
Wed Dec 6 07:45:38 UTC 2023


On Wed, 6 Dec 2023 03:07:54 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> Hi,
>> 
>> I need a review for a new internal buffer class called AEADBufferStream.  AEADBufferStream extends ByteArrayOutputStream, but eliminates some data checking and copying that are not necessary for what GaloisCounterMode.java and ChaCha20Cipher.java need.  
>> 
>> The changes greatest benefit is with decryption operations.  ChaCha20-Poly1305 had larger performance gains by adopting similar techniques that AES/GCM already uses. 
>> 
>> The new buffer shows up to 21% bytes/sec performance increase for decryption for ChaCha20-Poly1305 and 12% for AES/GCM.  16K data sizes saw a memory usage reduction of 46% with and 83% with ChaCha20-Poly1305.  These results come from the JMH tests updated in this request and memory usage using the JMH gc profile gc.alloc.rate.norm entry
>> 
>> thanks
>> 
>> Tony
>
> Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update

src/java.base/share/classes/com/sun/crypto/provider/AEADBufferedStream.java line 60:

> 58:      * returning byte[] maybe larger.
> 59:      *
> 60:      * @return internal or new byte array of non-blocksize data.

Please update the doc - the method always returns the internal array.

I would create a new method to retrieve the internal buffer instead of overriding `toByteArray`; it would be less surprising to the future code editors.

src/java.base/share/classes/com/sun/crypto/provider/AEADBufferedStream.java line 76:

> 74:         // Create a new larger buffer and append the new data
> 75:         if (blen < count + len) {
> 76:             buf = Arrays.copyOf(buf, ArraysSupport.newLength(blen, blen + len,

Suggestion:

            buf = Arrays.copyOf(buf, ArraysSupport.newLength(blen, count + len - blen,

the second parameter is minGrowth; `count+len-blen` would be more appropriate.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16487#discussion_r1416796431
PR Review Comment: https://git.openjdk.org/jdk/pull/16487#discussion_r1416785537



More information about the security-dev mailing list