RFR: 8304020: Speed up test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java and clarify its purpose [v7]

Eirik Bjorsnos duke at openjdk.org
Mon Oct 30 11:57:09 UTC 2023


> Please review this PR which speeds up TestTooManyEntries and clarifies its purpose:
> 
> - The name 'TestTooManyEntries' does not clearly convey the purpose of the test. What is tested is the validation that the total CEN size fits in a Java byte array. Suggested rename: CenSizeTooLarge
> - The test creates DEFLATED entries which incurs zlib costs and File Data / Data Descriptors for no additional benefit. We can use STORED instead.
> - By creating a single LocalDateTime and setting it with `ZipEntry.setTimeLocal`, we can avoid repeated time zone calculations. 
> - The name of entries is generated by calling UUID.randomUUID, we could use simple counter instead.
> - The produced file is unnecessarily large. We know how large a CEN entry is, let's take advantage of that to create a file with the minimal size.
> - By adding a maximally large extra field to the CEN entries, we get away with fewer CEN records and save memory
> - The summary and comments of the test can be improved to help explain the purpose of the test and how we reach the limit being tested.
> 
> These speedups reduced the runtime from 4 min 17 sec to 4 seconds on my Macbook Pro. The produced ZIP size was reduced from 5.7 GB to 2 GB. Memory consumption is down from 8GB to something like 12MB.

Eirik Bjorsnos has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 14 additional commits since the last revision:

 - Run CenSizeTooLarge automatically, since it no longer requires excessive memory or disk space
 - Use a SparseOutputStream to reduce required diskspace from 2GB to 4K. SparseOutputStream writes 'holes' instead of actual bytes until the final CEN is written.
 - TestTooManyEntries is renamed to CenSizeTooLarge
 - Merge branch 'master' into cen-size-too-large
 - MAX_EXTRA_FIELD_SIZE can be better expressed as 0xFFFF
 - Bring back '@requires sun.arch.data.model == 64' for now
 - Spell 'specified' correctly
 - Give test method a long, meaningful name
 - Remove blank line
 - Make CEN headers maximally large such that we need fewer of them and save memory
 - ... and 4 more: https://git.openjdk.org/jdk/compare/33a18950...9629b8d2

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/12991/files
  - new: https://git.openjdk.org/jdk/pull/12991/files/a1a77192..9629b8d2

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=12991&range=06
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12991&range=05-06

  Stats: 1001917 lines in 12941 files changed: 671061 ins; 209198 del; 121658 mod
  Patch: https://git.openjdk.org/jdk/pull/12991.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/12991/head:pull/12991

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


More information about the core-libs-dev mailing list