On Thu, 8 Feb 2024 14:09:35 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:

>> Please review this PR which suggests we speed up the `Zip64SizeTest` using a 
>> small-sized ZIP64 ZIP file specifically created to reproduce the issue being 
>> tested.
>> 
>> The disk space requirement of this test is known to cause problems in some 
>> builds, see [JDK-8259866](https://bugs.openjdk.org/browse/JDK-8259866)
>> 
>> By using a sparse file, we reduce consumed disk space from 5GB to 266 bytes 
>> and also reduce the runtime from ~35 seconds to ~1 seconds on my Macbook Pro.
>> 
>> The PR also fixes the `@summary` tag, which seems to have been copied from 
>> an unrelated test.
>
> 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/adc5f068...41b2ba5e

Reopening this PR which was closed without review in May 2023.

Initially, this PR suggested to use a sparse file when creating the large ZIP 
file. After consideration, I have found it simpler to instead doctor a 
small-sized ZIP64-entry with the specific structure required to trigger the 
regression being tested.

I have verified that the test actually fails if `ZipFile` is updated to call 
`ZipEntry.setExtra0` with `isLoc: true`, meaning it still catches the 
regression.

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

PR Comment: https://git.openjdk.org/jdk/pull/12948#issuecomment-1934202567

Reply via email to