On Tue, 28 Nov 2023 00:49:49 GMT, Anthony Scarpino <[email protected]>
wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/AEADBufferedStream.java
>> line 94:
>>
>>> 92: } else {
>>> 93: if (buf.length < (count + len)) {
>>> 94: buf = Arrays.copyOf(buf, count + len);
>>
>> this will copy a lot if the user performs many small writes, for example
>> when decrypting with CipherInputStream; see `AESGCMCipherOutputStream`
>> benchmark.
>
> As I understand the `ByteArrayOutputStream` code, the
> `ArraysSupport.newLength()` will double the allocation each time. So if the
> buffer is 1k size and it wants to add one more byte, the method will allocate
> 2k.
> I agree that in my change, if you send one byte at a time, it will be doing a
> lot of allocating. But I don't believe that is the general case. If you
> have examples of apps doing small allocations, let me know and I can come up
> with a different scheme. But at this point I thought it was a bitter risk
> more allocations in the less-likely situation, than unused allocation in what
> I see as a more common case.
Well, as stated above, any application using CipherInputStream will do O(N)
reallocations here, bringing back
[JDK-8298249](https://bugs.openjdk.org/browse/JDK-8298249); you might want to
check with the reporter if this actually affects any real applications.
For reference, the results with this patch:
Benchmark (dataSize) (keyLength) (provider) Mode
Cnt Score Error Units
AESGCMCipherOutputStream.decrypt 16384 128 thrpt
40 30325.669 ? 1616.428 ops/s
AESGCMCipherOutputStream.decrypt 1048576 128 thrpt
40 7.034 ? 0.479 ops/s
Baseline:
Benchmark (dataSize) (keyLength) (provider) Mode
Cnt Score Error Units
AESGCMCipherOutputStream.decrypt 16384 128 thrpt
40 52171.497 ? 6229.841 ops/s
AESGCMCipherOutputStream.decrypt 1048576 128 thrpt
40 470.844 ? 179.817 ops/s
i.e. with the patch, decryption is 40% slower for 16KB stream, 98% (or 50x)
slower for 1MB stream.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16487#discussion_r1413461835
