On Wed, 6 Dec 2023 03:07:54 GMT, Anthony Scarpino <ascarp...@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