RFR: 8304020: Speed up test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java and clarify its purpose [v3]
Martin Buchholz
martin at openjdk.org
Sun Mar 12 17:01:13 UTC 2023
On Sun, 12 Mar 2023 10:00:50 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.
>> - 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 1 min 54 sec on my Macbook Pro. The produced ZIP size was reduced from 5.7 GB to 3.5 GB.
>
> Eirik Bjorsnos has updated the pull request incrementally with one additional commit since the last revision:
>
> Use Integer.toString instead of Long.toString
Marked as reviewed by martin (Reviewer).
test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java line 67:
> 65: // Add entries until MAX_CEN_SIZE is reached
> 66: for (int i = 0; cenSize < MAX_CEN_SIZE; i++) {
> 67: String name = Integer.toString(i);
Consider making fewer but larger entries by generating large metadata.
Most obviously by making names in length close to the 64k metadata limit.
or by adding some fixed large extra field to each entry.
test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java line 70:
> 68: ZipEntry entry = new ZipEntry(name);
> 69: // Use STORED for faster processing
> 70: entry.setMethod(ZipEntry.STORED);
it's a little surprising that STORED vs DEFLATED here makes much of a difference because there's no file contents to be compressed.
test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java line 86:
> 84: */
> 85: @Test
> 86: public void test() {
I've learned to always give test methods long meaningful names, e.g. centralDirectoryTooLargeToFitInByteArray
-------------
PR: https://git.openjdk.org/jdk/pull/12991
More information about the core-libs-dev
mailing list