On Sun, 6 Oct 2024 18:02:58 GMT, Eirik Bjørsnøs <[email protected]> wrote:
> Please review this PR which proposes to change the input buffer size of
> `ZipFileInflaterInputStream` to be based on the _compressed_ size of a ZIP
> entry instead of the _uncompressed_ size. This saves allocation since buffers
> will no longer be oversized:
>
> * The `size` parameter passed to the `ZipFileInflaterInputStream` constructor
> is passed on to the superclass `InflaterInputStream` where it determines the
> size of the input buffer. This buffer is used to read compressed data and
> pass it on to the `Inflater`.
> * `ZipFile:getInputStream` currently looks at the _uncompressed_ size when
> determining the input buffer size. It should instead use the _compressed_
> size, since this buffer is used for compressed, not uncompressed data.
> * The current implementation somewhat mysteriously adds 2 to the uncompressed
> size. My guess is that this is to allow fitting a two-byte DEFLATE header for
> empty files (where the uncompressed size is 0 and the compressed size is 2).
> * There is a check for `size <= 0`. This condition is unreachable in the
> current code and in the PR as well, since the compressed size will always be
> `>= 2`. I propose we remove this check.
>
> Performance: A benchmark which measures the cost of opening and closing input
> streams using `ZipFile::getInputStream` shows a modest improvement of ~5%,
> consistent with less allocation of unused buffer space.
>
> Testing: No tests are added in this PR. The `noreg-cleanup` label is added in
> JBS. GHA testing is currently pending.
src/java.base/share/classes/java/util/zip/ZipFile.java line 417:
> 415: if (size > 65536) {
> 416: size = 8192;
> 417: }
Not sure if this clamping makes sense? We clamp the size at 8192, but only when
size is larger than 65536?
Wondering if we should simply clamp to 8192 instead:
Suggestion:
int size = Math.clamp(CENSIZ(zsrc.cen, pos), 2, 8192);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21379#discussion_r1789215300
