RFR: 8303891: Speed up Zip64SizeTest using a small ZIP64 file [v5]
Eirik Bjørsnøs
eirbjo at openjdk.org
Fri Feb 9 10:45:02 UTC 2024
On Fri, 9 Feb 2024 06:39:31 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> Eirik Bjørsnøs 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 13 additional commits since the last revision:
>>
>> - Use a small ZIP64 file to reproduce the issue. Convert test to JUnit
>> - Update copyright year for 2024
>> - Use ENTRY instead of FILE when refering to names and sizes of file entries in the ZIP file
>> - Merge branch 'master' into zip64-size-test-sparse
>> - Merge branch 'master' into zip64-size-test-sparse
>> - Sparse files must be created explicitly on NTFS
>> - Merge branch 'master' into zip64-size-test-sparse
>> - Merge branch 'master' into zip64-size-test-sparse
>> - Make test method public
>> - Add a missing "when" in Javadocs for SparseOutputStream
>> - ... and 3 more: https://git.openjdk.org/jdk/compare/1bff6cd8...41b2ba5e
>
> test/jdk/java/util/zip/ZipFile/Zip64SizeTest.java line 53:
>
>> 51: // ZIP file to create
>> 52: private static final Path ZIP_FILE = Path.of("Zip64SizeTest.zip");
>> 53: // ontents to write to ZIP entries
>
> Typo - should have been "contents"
Thanks, fixed.
> Hello Eirik, I couldn't understand why we first add a opaque extra field first and then update it to be a zip64 extra field. Why do we do this?
`ZipEntry.setExtra` processes the byte array argument, looking for Zip64 extended fields which it can extract the size fields from. To prevent this parsing from happening, we temporarily use the `unknown` tag.
In this particular case, `ZipExtra.setExtra` actually ends up skipping this processing (because `isLOC == true` and it has a guard for the block size being `>= 16`).
However, I prefer the test to not depend too much on the details of `setExtra` Zip64 processing. This trick is used in other tests as well and may be copied over to a test where the conditions are not the same.
I have refactored a bit and added some code comments to help explain the use of the 'unknown' tag.
Do you think this makes sense?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12948#discussion_r1484148941
PR Review Comment: https://git.openjdk.org/jdk/pull/12948#discussion_r1484148034
More information about the core-libs-dev
mailing list