On Sun, 12 Mar 2023 16:41:33 GMT, Martin Buchholz <mar...@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
