On Mon, 4 Dec 2023 07:27:14 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
>> 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. 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16487#discussion_r1416585796
