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

Jaikiran Pai jpai at openjdk.org
Thu Nov 16 05:29:36 UTC 2023


On Wed, 15 Nov 2023 19:26:06 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

>> 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.
>> - By writing sparse 'holes' until the last CEN entry, we can reduce required disk space.
>> 
>> These speedups reduced the runtime from 4 min 17 sec to 3 seconds on my Macbook Pro. The produced ZIP size was reduced from 5.7 GB to ~4K. Memory consumption is down from 8GB to something like 12MB.
>
> Eirik Bjorsnos has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix spelling of LocalDateTime
>  - Add comment to the SparseOutputStream class noting that instances must be passed directly to the ZipOutputStream constructor, without buffering.

Hello Eirik, the code comment updates look good to me. I went ahead and ran another run our CI with the latest mainline and it too passed fine. I'll go ahead and sponsor this now.

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

PR Comment: https://git.openjdk.org/jdk/pull/12991#issuecomment-1813811258


More information about the core-libs-dev mailing list