JBR-7392: Use NIO FS in ZipFile

[knisht]

See https://youtrack.jetbrains.com/issue/JBR-7392/Allow-using-NIO-in-ZipFile for discussion.

Tests against upstream JDK: https://github.com/knisht/jdk/pull/1 Tests with enabled property: https://github.com/knisht/jdk/pull/2. It contains an additional failure of RedefineSharedClass, which is fixable.

[mkartashev]

It contains an additional failure of RedefineSharedClass, which is fixable.

Please, provide a detailed explanation of the failure and how it can be fixed.

[mkartashev]

Since ZipFile is used in VM startup, it means that the VM loses the ability to work without allocation of direct buffers

Can you make the "new" ZipFile parametrized by the VM bootup stage so that it can be successfully used both during the JVM startup (in the RandomAccessFile mode) and later (in the NIO mode)?

[mkartashev]

What are performance implications outside of WSL? "Regular" Windows, macOS, Linux? How does this patch affect startup time?

[knisht]

Please, provide a detailed explanation of the failure and how it can be fixed.

RedefineSharedClass fails on the following line:

        // When run without arguments this class acts as the test. First redefining
        // a class that we expect to be in the archive if used and the triggering a
        // System.gc() to clean up.
        RedefineClassHelper.redefineClass(java.io.RandomAccessFile.class, getClassBytes(java.io.RandomAccessFile.class));

Since java.io.RandomAccessFile is not loaded in ZipFile anymore, we cannot expect that it appears in the archive. If we replace java.io.RandomAccessFile with java.util.zip.ZipFile, the test passes, and it checks what it used to check.

There is also another failure in TestZipError, and it is Windows-related.

        // Open the ZipFile.  This will read the zip file's central
        // directory into in-memory data structures.
        ZipFile zf = new ZipFile(fileName);

        // Delete the file; of course this does not change the in-memory data
        // structures that represent the central directory!
        f.delete();

        // Re-create zip file, with different entries than earlier.  However,
        // recall that we have in-memory information about the central
        // directory of the file at its previous state.
        zos = new ZipOutputStream(new FileOutputStream(f));

The test fails here with FileNotFoundException. The reason is that RandomAccessFile opens a file without FILE_SHARE_DELETE (https://github.com/openjdk/jdk/blob/7e11fb702696df733ca89d325200f2e9414402d9/src/java.base/windows/native/libjava/io_util_md.c#L220), while FileChannel has this share mode enabled by default (https://github.com/openjdk/jdk/blob/7e11fb702696df733ca89d325200f2e9414402d9/src/java.base/windows/classes/sun/nio/fs/WindowsChannelFactory.java#L121). In the case of RandomAccessFile, the delete fails (so the test's behavior is not really meaningful), and in the case of FileChannel we get a well-deserved exception. This can be fixed simply by removing f.delete().

[knisht]

Can you make the "new" ZipFile parametrized by the VM bootup stage so that it can be successfully used both during the JVM startup (in the RandomAccessFile mode) and later (in the NIO mode)?

What are we trying to achieve here? Whenever the VM accesses a jar file (for lazy classloading, as an example), it would allocate a memory-mapped file. I don't think it is possible to have FileChannel-based ZipFile without the use of direct buffers in VM's internal operations.

[mkartashev]

Can you make the "new" ZipFile parametrized by the VM bootup stage so that it can be successfully used both during the JVM startup (in the RandomAccessFile mode) and later (in the NIO mode)?

What are we trying to achieve here?

Never mind, I thought this is what was causing the test to fail.

[mkartashev]

Thanks for explaining the failures. Could you, please, fix those two tests so that we don't have to remember to exclude them forever?

[mkartashev]

BTW, those tests need to also be run with -Djava.util.zip.use.nio.for.zip.file.access=true

[knisht]

Benchmark description

I made a small performance test which loads all classes from the provided class path.

1. The test itself uses an inheritor of ZipFile.
2. The test loads classes, and classloader is a client of ZipFile.

   
   import java.io.File;
   import java.io.IOException;
   import java.util.jar.JarFile;

public class Main {

public static void main(String[] args) throws IOException { ClassLoader cl = Thread.currentThread().getContextClassLoader();

String cp = System.getProperty("java.class.path");
String[] jars = cp.split(File.pathSeparator);
for (String jar : jars) {
  try (JarFile jarFile = new JarFile(jar)) {
    jarFile.stream().forEach(entry -> {
      if (entry.getName().endsWith(".class")) {
        String className = entry.getName().replace('/', '.').substring(0, entry.getName().length() - 6);
        try {
          Class.forName(className, true, cl);
        }
        catch (Throwable e) {
        }
      }
    });
  }
}
System.exit(0);

} }

I applied the test to a classpath consisting of several fat jars. For example, the archive with IDE classes takes around 120 MB of space, so I consider it a reasonable object of measurement. The test loaded 70935 classes in total.

Full command line looks like this: 
```sh
./java -Djava.util.zip.use.nio.for.zip.file.access=<true/false> -cp app-client.jar:kotlin-stdlib-2.0.0.jar:util-8.jar <benchmark file.java>

Performance

I wrote a script which measures time of execution. The script invokes the command above 10 times, gathering some statistics about the execution time.

MacOS (aarch64)

Without NIO in ZipFile:

Mean Execution Time: 8.703340 seconds
Standard Deviation of Execution Time: 0.594840 seconds

With NIO in ZipFile

Mean Execution Time: 8.694077 seconds
Standard Deviation of Execution Time: 0.389752 seconds

Windows (x64)

Without NIO:

Mean Execution Time: 11.088352 seconds
Standard Deviation of Execution Time: 0.645189 seconds

With NIO:

Mean Execution Time: 10.799217 seconds
Standard Deviation of Execution Time: 0.309649 seconds

Linux (x64, inside WSL)

Without NIO:

Mean Execution Time: 9.980494 seconds
Standard Deviation of Execution Time: 0.254731 seconds

With NIO:

Mean Execution Time: 10.369276 seconds
Standard Deviation of Execution Time: 0.521237 seconds

[mkartashev]

Thank you, sounds reasonable. There doesn't seem to be any impact on startup time, so I will go ahead and port this change to jbr21.