On Wed, 6 Dec 2023 02:43:06 GMT, Anthony Scarpino <ascarp...@openjdk.org> wrote:

>> 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.
>
> Ok.. I updated the code to handle it more like it did.  With one update(), 
> the memory usage is still how I intended.  So keeping the CipherOutputStream 
> case, where it's multiple update() calls, it's a problem.

After my change, that perf test showed a 30% for 16k and 15% for 1M perf 
increase

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

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

Reply via email to