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.

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

Commit messages:
 - Use compressed size instead of uncompressed size when determining the 
ZipFileInflaterInputStream input buffer size

Changes: https://git.openjdk.org/jdk/pull/21379/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21379&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8341597
  Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/21379.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/21379/head:pull/21379

PR: https://git.openjdk.org/jdk/pull/21379

Reply via email to