8265448: (zipfs): Reduce read contention in ZipFileSystem

Reviewed-by: alanb, lancea

Author: Jason Zaugg

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java

@@ -1227,8 +1227,12 @@ final long readFullyAt(byte[] buf, int off, long len, long pos)
     }
 
     private long readFullyAt(ByteBuffer bb, long pos) throws IOException {
-        synchronized(ch) {
-            return ch.position(pos).read(bb);
+        if (ch instanceof FileChannel fch) {
+            return fch.read(bb, pos);
+        } else {
+            synchronized(ch) {
+                return ch.position(pos).read(bb);
+            }
         }
     }
 
@@ -2116,7 +2120,7 @@ else if (e.file != null)
             // streams.add(eis);
             return eis;
         } else {  // untouched CEN or COPY
-            eis = new EntryInputStream(e, ch);
+            eis = new EntryInputStream(e);
         }
         if (e.method == METHOD_DEFLATED) {
             // MORE: Compute good size for inflater stream:
@@ -2173,15 +2177,12 @@ public int available() {
     // Inner class implementing the input stream used to read
     // a (possibly compressed) zip file entry.
     private class EntryInputStream extends InputStream {
-        private final SeekableByteChannel zfch; // local ref to zipfs's "ch". zipfs.ch might
-                                                // point to a new channel after sync()
         private long pos;                       // current position within entry data
         private long rem;                       // number of remaining bytes within entry
 
-        EntryInputStream(Entry e, SeekableByteChannel zfch)
+        EntryInputStream(Entry e)
             throws IOException
         {
-            this.zfch = zfch;
             rem = e.csize;
             pos = e.locoff;
             if (pos == -1) {
@@ -2206,14 +2207,10 @@ public int read(byte[] b, int off, int len) throws IOException {
             if (len > rem) {
                 len = (int) rem;
             }
-            // readFullyAt()
-            long n;
             ByteBuffer bb = ByteBuffer.wrap(b);
             bb.position(off);
             bb.limit(off + len);
-            synchronized(zfch) {
-                n = zfch.position(pos).read(bb);
-            }
+            long n = readFullyAt(bb, pos);
             if (n > 0) {
                 pos += n;
                 rem -= n;

test/jdk/jdk/nio/zipfs/ZipFSTester.java

@@ -499,10 +499,12 @@ private static void checkRead(Path path, byte[] expected) throws IOException {
                 }
                 // System.out.printf("  --> %d, %d%n", pos, len);
                 bb.position(0).limit(len);    // bb.flip().limit(len);
+                int expectedReadResult = sbc.size() == 0 ? -1 : len;
                 if (sbc.position(pos).position() != pos ||
-                    sbc.read(bb) != len ||
+                    sbc.read(bb) != expectedReadResult ||
                     !Arrays.equals(buf, 0, bb.position(), expected, pos, pos + len)) {
                     System.out.printf("read()/position() failed%n");
+                    throw new RuntimeException("CHECK FAILED!");
                 }
             }
         } catch (IOException x) {

test/micro/org/openjdk/bench/jdk/nio/zipfs/ZipFileSystemBenchmark.java

@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.bench.jdk.nio.zipfs;
+
+import org.openjdk.jmh.annotations.*;
+import org.openjdk.jmh.infra.Blackhole;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.FileSystem;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.spi.FileSystemProvider;
+import java.util.Map;
+import java.util.Random;
+import java.util.concurrent.TimeUnit;
+
+@BenchmarkMode(Mode.AverageTime)
+@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
+@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
+@State(Scope.Benchmark)
+@Threads(Threads.MAX)
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+@Fork(value = 3)
+public class ZipFileSystemBenchmark {
+    private static final String FILE_NAME = "filename";
+    private FileSystemProvider jarFsProvider;
+    private Path readPath;
+    private FileSystem fileSystem;
+    private Path zip;
+
+    @Setup(Level.Trial) public void setup() throws IOException {
+        jarFsProvider = FileSystemProvider.installedProviders().stream().filter(x -> x.getScheme().equals("jar")).findFirst().get();
+        zip = Files.createTempFile("zipfs-benchmark", ".jar");
+        createTestZip();
+        fileSystem = jarFsProvider.newFileSystem(zip, Map.of());
+        Path rootRead = fileSystem.getRootDirectories().iterator().next();
+        readPath = rootRead.resolve(FILE_NAME);
+    }
+
+    private void createTestZip() throws IOException {
+        Files.delete(zip);
+        FileSystem writableFileSystem = jarFsProvider.newFileSystem(zip, Map.of("create", "true"));
+        byte[] data = new byte[16 * 1024 * 1024];
+        new Random(31).nextBytes(data);
+        Path root = writableFileSystem.getRootDirectories().iterator().next();
+        Files.write(root.resolve(FILE_NAME), data);
+        writableFileSystem.close();
+    }
+
+    @TearDown public void tearDown() throws IOException {
+        if (fileSystem != null) {
+            fileSystem.close();
+        }
+        Files.deleteIfExists(zip);
+    }
+
+    // Performance should remain constant when varying the number of threads up to the
+    // number of physical cores if the NIO implementation on the platform supports
+    // concurrent reads to a single FileChannel instance. At the time of writing, NIO on Windows
+    // serializes access.
+    @Benchmark public void read(Blackhole bh) throws IOException {
+        InputStream inputStream = Files.newInputStream(readPath);
+        byte[] buffer = new byte[8192];
+        while (inputStream.read(buffer) != -1) {
+            bh.consume(buffer);
+        }
+    }
+}