On Tue, 28 Nov 2023 00:49:49 GMT, Anthony Scarpino <ascarp...@openjdk.org> 
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

Reply via email to