On Tue, 17 Oct 2023 08:50:15 GMT, Sean Coffey <coff...@openjdk.org> wrote:
>> Fix up java.util.zip.ZipFile$Source hashCode() impl so that duplicate Source
>> objects aren't created for the same zip file.
>
> Sean Coffey 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 six additional
> commits since the last revision:
>
> - update pae exception handling, correct test case typo and improve comments
> - Merge branch 'master' into 8317678-Zip-Source
> - minor test edits and comment updates
> - insert back lastmodified check. enhance testcase. more review comments
> - incorporate review comments
> - Initial changes
thanks for the reviews and comments to date. I've pushed new changes which
should address them.
Good catch on the ZIPENTRY_NAME typo @jaikiran
I've run some tests also and it looks like the fileKey() returns the same
object/reference - even if symbolic links are used.
you raise an interesting point about `getCanonicalFile()` performance though.
I've updated an existing benchmark[1] to check and performance for fileystems
that don't support fileKey() (windows) has regressed[2]. Performance is much
better on systems that support fileKey() (unix) -- I'll look at it further but
perhaps I'll only target the optimizations for the filesystems that support
filekey() for now.
[1]
diff --git a/test/micro/org/openjdk/bench/java/util/zip/ZipFileOpen.java
b/test/micro/org/openjdk/bench/java/util/zip/ZipFileOpen.java
index ffbbc3d245f..34449c14e32 100644
--- a/test/micro/org/openjdk/bench/java/util/zip/ZipFileOpen.java
+++ b/test/micro/org/openjdk/bench/java/util/zip/ZipFileOpen.java
@@ -29,6 +29,7 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.concurrent.TimeUnit;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
@@ -50,6 +51,7 @@ public class ZipFileOpen {
private int size;
public File zipFile;
+ public File relativePathFile;
@Setup(Level.Trial)
public void beforeRun() throws IOException {
@@ -74,6 +76,8 @@ public void beforeRun() throws IOException {
}
}
zipFile = tempFile;
+ relativePathFile = Path.of(System.getProperty("user.dir"))
+ .relativize(zipFile.toPath()).toFile();
}
@Benchmark
@@ -90,4 +94,13 @@ public ZipFile openCloseZipFile() throws Exception {
zf.close();
return zf;
}
+
+ @Benchmark
+ public void openCloseZipFilex2() throws Exception {
+ // Ensure that we only create ZipFile.Source.Key entry per unique zip
file
+ ZipFile zf = new ZipFile(zipFile);
+ ZipFile zf2 = new ZipFile(relativePathFile);
+ zf.close();
+ zf2.close();
+ }
}
[2]
jdk-tip-no-patch: (Windows)
Benchmark (size) Mode Cnt Score Error
Units
ZipFileOpen.openCloseZipFile 512 avgt 15 319543.232 ± 46112.481
ns/op
ZipFileOpen.openCloseZipFile 1024 avgt 15 463719.381 ± 139409.316
ns/op
ZipFileOpen.openCloseZipFilex2 512 avgt 15 750838.029 ± 190726.572
ns/op
ZipFileOpen.openCloseZipFilex2 1024 avgt 15 875174.035 ± 140971.036
ns/op
jdk-with-new-patch
Benchmark (size) Mode Cnt Score Error
Units
ZipFileOpen.openCloseZipFile 512 avgt 15 603053.335 ± 50514.066
ns/op
ZipFileOpen.openCloseZipFile 1024 avgt 15 710960.542 ± 46343.033
ns/op
ZipFileOpen.openCloseZipFilex2 512 avgt 15 1258566.172 ± 216230.090
ns/op
ZipFileOpen.openCloseZipFilex2 1024 avgt 15 1260380.961 ± 164216.628
ns/op
[3]
without patch: (Linux)
Benchmark (size) Mode Cnt Score Error Units
ZipFileOpen.openCloseZipFile 512 avgt 15 75378.890 ? 642.370 ns/op
ZipFileOpen.openCloseZipFile 1024 avgt 15 150814.293 ? 29801.424 ns/op
ZipFileOpen.openCloseZipFilex2 512 avgt 15 168104.286 ? 41505.778 ns/op
ZipFileOpen.openCloseZipFilex2 1024 avgt 15 299282.488 ? 44496.777 ns/op
Finished running test 'micro:java.util.zip.ZipFileOpen'
with patch:
Benchmark (size) Mode Cnt Score Error Units
ZipFileOpen.openCloseZipFile 512 avgt 15 75400.408 ? 408.981 ns/op
ZipFileOpen.openCloseZipFile 1024 avgt 15 146213.650 ? 15509.167 ns/op
ZipFileOpen.openCloseZipFilex2 512 avgt 15 83194.552 ? 16361.844 ns/op
ZipFileOpen.openCloseZipFilex2 1024 avgt 15 153956.028 ? 30550.595 ns/op
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16115#issuecomment-1765969450
