RFR: 8317678: Fix up hashCode() for ZipFile.Source.Key [v5]

[Sean Coffey]

On Tue, 17 Oct 2023 08:50:15 GMT, Sean Coffey <coffeys at 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