On Wed, 6 Dec 2023 03:07:54 GMT, Anthony Scarpino <[email protected]> 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
