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

Eirik Bjorsnos duke at openjdk.org
Sun Mar 12 17:01:15 UTC 2023


On Sun, 12 Mar 2023 16:41:33 GMT, Martin Buchholz <martin at openjdk.org> wrote:

>> Eirik Bjorsnos has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Use Integer.toString instead of Long.toString
>
> 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.

Oh no, you reviewed just seconds before I pushed an update which does just that. I use a maximally sized extra field which makes the test run in 4 seconds using 12MB (measured using Runtime.freeMemory) of memory in the main loop.

Could I bother you to review the update version?

> 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.

Well, there is! ZipOutputStream will output an 'empty final' DEFLATE block, followed by a 16-byte data descriptor. Take a look at PR #12899  which suggests adding an apiNote recommending the use of STORED for this reason.

> 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

Thanks, I did actually not notice that myself, good idea!

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

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


More information about the core-libs-dev mailing list