8193066: Avoid use of capturing lambdas in JarFile

Reviewed-by: lancea, alanb

src/java.base/share/classes/java/util/jar/JarFile.java

@@ -503,11 +503,11 @@ public class JarFile extends ZipFile {
         if (isMultiRelease()) {
             JarEntry je = getVersionedEntry(name, null);
             if (je == null) {
-                je = getEntry0(name);
+                je = (JarEntry)super.getEntry(name);
             }
             return je;
         } else {
-            return getEntry0(name);
+            return super.getEntry(name);
         }
     }
 
@@ -519,7 +519,7 @@ public class JarFile extends ZipFile {
      *         may be thrown if the jar file has been closed
      */
     public Enumeration<JarEntry> entries() {
-        return JUZFA.entries(this, JarFileEntry::new);
+        return JUZFA.entries(this);
     }
 
     /**
@@ -532,7 +532,7 @@ public class JarFile extends ZipFile {
      * @since 1.8
      */
     public Stream<JarEntry> stream() {
-        return JUZFA.stream(this, JarFileEntry::new);
+        return JUZFA.stream(this);
     }
 
     /**
@@ -563,19 +563,11 @@ public class JarFile extends ZipFile {
         return stream();
     }
 
-    /*
+    /**
-     * Invokes {@ZipFile}'s getEntry to Return a {@code JarFileEntry} for the
+     * Creates a ZipEntry suitable for the given ZipFile.
-     * given entry name or {@code null} if not found.
      */
-    private JarFileEntry getEntry0(String name) {
+    JarEntry entryFor(String name) {
-        // Not using a lambda/method reference here to optimize startup time
+        return new JarFileEntry(name);
-        Function<String, JarEntry> newJarFileEntryFn = new Function<>() {
-            @Override
-            public JarEntry apply(String name) {
-                return new JarFileEntry(name);
-            }
-        };
-        return (JarFileEntry)JUZFA.getEntry(this, name, newJarFileEntryFn);
     }
 
     private String getBasename(String name) {
@@ -613,7 +605,8 @@ public class JarFile extends ZipFile {
                     if (version < BASE_VERSION_FEATURE) {
                         break;
                     }
-                    JarFileEntry vje = getEntry0(META_INF_VERSIONS + version + "/" + name);
+                    JarFileEntry vje = (JarFileEntry)super.getEntry(
+                            META_INF_VERSIONS + version + "/" + name);
                     if (vje != null) {
                         return vje.withBasename(name);
                     }
@@ -926,7 +919,7 @@ public class JarFile extends ZipFile {
             // initialization
             String name = JUZFA.getManifestName(this, false);
             if (name != null) {
-                this.manEntry = getEntry0(name);
+                this.manEntry = (JarEntry)super.getEntry(name);
             }
         }
         return manEntry;
@@ -1093,12 +1086,11 @@ public class JarFile extends ZipFile {
     Enumeration<JarEntry> entries2() {
         ensureInitialization();
         if (jv != null) {
-            return jv.entries2(this, JUZFA.entries(JarFile.this,
+            return jv.entries2(this, JUZFA.entries(JarFile.this));
-                                                   JarFileEntry::new));
         }
 
         // screen out entries which are never signed
-        final var unfilteredEntries = JUZFA.entries(JarFile.this, JarFileEntry::new);
+        final var unfilteredEntries = JUZFA.entries(JarFile.this);
 
         return new Enumeration<>() {
 

src/java.base/share/classes/java/util/jar/JavaUtilJarAccessImpl.java

@@ -30,6 +30,9 @@ import java.net.URL;
 import java.security.CodeSource;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+
 import jdk.internal.access.JavaUtilJarAccess;
 
 class JavaUtilJarAccessImpl implements JavaUtilJarAccess {
@@ -72,4 +75,8 @@ class JavaUtilJarAccessImpl implements JavaUtilJarAccess {
     public boolean isInitializing() {
         return JarFile.isInitializing();
     }
+
+    public JarEntry entryFor(JarFile jar, String name) {
+        return jar.entryFor(name);
+    }
 }

src/java.base/share/classes/java/util/zip/ZipFile.java

@@ -55,13 +55,13 @@ import java.util.Spliterators;
 import java.util.TreeSet;
 import java.util.WeakHashMap;
 import java.util.function.Consumer;
-import java.util.function.Function;
 import java.util.function.IntFunction;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 import jdk.internal.access.JavaUtilZipFileAccess;
+import jdk.internal.access.JavaUtilJarAccess;
 import jdk.internal.access.SharedSecrets;
 import jdk.internal.misc.VM;
 import jdk.internal.perf.PerfCounter;
@@ -321,27 +321,13 @@ public class ZipFile implements ZipConstants, Closeable {
      * @throws IllegalStateException if the zip file has been closed
      */
     public ZipEntry getEntry(String name) {
-        return getEntry(name, ZipEntry::new);
-    }
-
-    /*
-     * Returns the zip file entry for the specified name, or null
-     * if not found.
-     *
-     * @param name the name of the entry
-     * @param func the function that creates the returned entry
-     *
-     * @return the zip file entry, or null if not found
-     * @throws IllegalStateException if the zip file has been closed
-     */
-    private ZipEntry getEntry(String name, Function<String, ? extends ZipEntry> func) {
         Objects.requireNonNull(name, "name");
         ZipEntry entry = null;
         synchronized (this) {
             ensureOpen();
             int pos = res.zsrc.getEntryPos(name, true);
             if (pos != -1) {
-                entry = getZipEntry(name, pos, func);
+                entry = getZipEntry(name, pos);
             }
         }
         return entry;
@@ -487,11 +473,9 @@ public class ZipFile implements ZipConstants, Closeable {
 
         private int i = 0;
         private final int entryCount;
-        private final Function<String, T> gen;
 
-        public ZipEntryIterator(int entryCount, Function<String, T> gen) {
+        public ZipEntryIterator(int entryCount) {
             this.entryCount = entryCount;
-            this.gen = gen;
         }
 
         @Override
@@ -518,7 +502,7 @@ public class ZipFile implements ZipConstants, Closeable {
                     throw new NoSuchElementException();
                 }
                 // each "entry" has 3 ints in table entries
-                return (T)getZipEntry(null, res.zsrc.getEntryPos(i++ * 3), gen);
+                return (T)getZipEntry(null, res.zsrc.getEntryPos(i++ * 3));
             }
         }
 
@@ -536,14 +520,14 @@ public class ZipFile implements ZipConstants, Closeable {
     public Enumeration<? extends ZipEntry> entries() {
         synchronized (this) {
             ensureOpen();
-            return new ZipEntryIterator<ZipEntry>(res.zsrc.total, ZipEntry::new);
+            return new ZipEntryIterator<ZipEntry>(res.zsrc.total);
         }
     }
 
-    private Enumeration<JarEntry> entries(Function<String, JarEntry> func) {
+    private Enumeration<JarEntry> jarEntries() {
         synchronized (this) {
             ensureOpen();
-            return new ZipEntryIterator<JarEntry>(res.zsrc.total, func);
+            return new ZipEntryIterator<JarEntry>(res.zsrc.total);
         }
     }
 
@@ -590,7 +574,7 @@ public class ZipFile implements ZipConstants, Closeable {
         synchronized (this) {
             ensureOpen();
             return StreamSupport.stream(new EntrySpliterator<>(0, res.zsrc.total,
-                pos -> getZipEntry(null, pos, ZipEntry::new)), false);
+                pos -> getZipEntry(null, pos)), false);
        }
     }
 
@@ -625,16 +609,15 @@ public class ZipFile implements ZipConstants, Closeable {
      * Entries appear in the {@code Stream} in the order they appear in
      * the central directory of the jar file.
      *
-     * @param func the function that creates the returned entry
      * @return an ordered {@code Stream} of entries in this zip file
      * @throws IllegalStateException if the zip file has been closed
      * @since 10
      */
-    private Stream<JarEntry> stream(Function<String, JarEntry> func) {
+    private Stream<JarEntry> jarStream() {
         synchronized (this) {
             ensureOpen();
             return StreamSupport.stream(new EntrySpliterator<>(0, res.zsrc.total,
-                pos -> (JarEntry)getZipEntry(null, pos, func)), false);
+                pos -> (JarEntry)getZipEntry(null, pos)), false);
         }
     }
 
@@ -642,8 +625,7 @@ public class ZipFile implements ZipConstants, Closeable {
     private int lastEntryPos;
 
     /* Check ensureOpen() before invoking this method */
-    private ZipEntry getZipEntry(String name, int pos,
+    private ZipEntry getZipEntry(String name, int pos) {
-                                 Function<String, ? extends ZipEntry> func) {
         byte[] cen = res.zsrc.cen;
         int nlen = CENNAM(cen, pos);
         int elen = CENEXT(cen, pos);
@@ -663,7 +645,12 @@ public class ZipFile implements ZipConstants, Closeable {
             // invoked from iterator, use the entry name stored in cen
             name = zc.toString(cen, pos + CENHDR, nlen);
         }
-        ZipEntry e = func.apply(name);    //ZipEntry e = new ZipEntry(name);
+        ZipEntry e;
+        if (this instanceof JarFile) {
+            e = Source.JUJA.entryFor((JarFile)this, name);
+        } else {
+            e = new ZipEntry(name);
+        }
         e.flag = CENFLG(cen, pos);
         e.xdostime = CENTIM(cen, pos);
         e.crc = CENCRC(cen, pos);
@@ -1094,19 +1081,12 @@ public class ZipFile implements ZipConstants, Closeable {
                     return ((ZipFile)jar).getMetaInfVersions();
                 }
                 @Override
-                public JarEntry getEntry(ZipFile zip, String name,
+                public Enumeration<JarEntry> entries(ZipFile zip) {
-                    Function<String, JarEntry> func) {
+                    return zip.jarEntries();
-                    return (JarEntry)zip.getEntry(name, func);
                 }
                 @Override
-                public Enumeration<JarEntry> entries(ZipFile zip,
+                public Stream<JarEntry> stream(ZipFile zip) {
-                    Function<String, JarEntry> func) {
+                    return zip.jarStream();
-                    return zip.entries(func);
-                }
-                @Override
-                public Stream<JarEntry> stream(ZipFile zip,
-                    Function<String, JarEntry> func) {
-                    return zip.stream(func);
                 }
                 @Override
                 public Stream<String> entryNameStream(ZipFile zip) {
@@ -1118,6 +1098,9 @@ public class ZipFile implements ZipConstants, Closeable {
     }
 
     private static class Source {
+        // While this is only used from ZipFile, defining it there would cause
+        // a bootstrap cycle that would leave this initialized as null
+        private static final JavaUtilJarAccess JUJA = SharedSecrets.javaUtilJarAccess();
         // "META-INF/".length()
         private static final int META_INF_LENGTH = 9;
         private static final int[] EMPTY_META_VERSIONS = new int[0];

src/java.base/share/classes/jdk/internal/access/JavaUtilJarAccess.java

@@ -46,4 +46,5 @@ public interface JavaUtilJarAccess {
     public Attributes getTrustedAttributes(Manifest man, String name);
     public void ensureInitialization(JarFile jar);
     public boolean isInitializing();
+    public JarEntry entryFor(JarFile file, String name);
 }

src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java

@@ -27,7 +27,6 @@ package jdk.internal.access;
 
 import java.util.Enumeration;
 import java.util.List;
-import java.util.function.Function;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.stream.Stream;
@@ -38,9 +37,8 @@ public interface JavaUtilZipFileAccess {
     public List<String> getManifestAndSignatureRelatedFiles(JarFile zip);
     public String getManifestName(JarFile zip, boolean onlyIfSignatureRelatedFiles);
     public int[] getMetaInfVersions(JarFile zip);
-    public JarEntry getEntry(ZipFile zip, String name, Function<String, JarEntry> func);
+    public Enumeration<JarEntry> entries(ZipFile zip);
-    public Enumeration<JarEntry> entries(ZipFile zip, Function<String, JarEntry> func);
+    public Stream<JarEntry> stream(ZipFile zip);
-    public Stream<JarEntry> stream(ZipFile zip, Function<String, JarEntry> func);
     public Stream<String> entryNameStream(ZipFile zip);
 }
 

test/micro/org/openjdk/bench/java/util/jar/JarFileGetEntry.java

@@ -0,0 +1,144 @@
+/*
+ * 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.java.util.jar;
+
+import org.openjdk.jmh.annotations.*;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.Random;
+import java.util.concurrent.TimeUnit;
+import java.util.jar.JarFile;
+import java.util.jar.JarOutputStream;
+import java.util.zip.ZipEntry;
+
+/**
+ * Simple benchmark measuring cost of looking up entries in a jar file.
+ *
+ * Before JDK-8193066
+ * Benchmark                             (size)  Mode  Cnt   Score    Error   Units
+ * JarFileGetEntry.getEntryHit            1024  avgt   10  102.554    3.371   ns/op
+ *   gc.alloc.rate.norm                   1024  avgt   10  144.036    0.004    B/op
+ * JarFileGetEntry.getEntryHitUncached    1024  avgt   10  141.307    7.454   ns/op
+ *   gc.alloc.rate.norm                   1024  avgt   10  200.040    0.004    B/op
+ * JarFileGetEntry.getEntryMiss           1024  avgt   10   26.489    1.737   ns/op
+ *   gc.alloc.rate.norm                   1024  avgt   10   16.001    0.001    B/op
+ * JarFileGetEntry.getEntryMissUncached   1024  avgt   10   74.189    3.320   ns/op
+ *   gc.alloc.rate.norm                   1024  avgt   10   72.194    0.001    B/op
+ *
+ * After JDK-8193066
+ * Benchmark                            (size)  Mode  Cnt    Score    Error   Units
+ * JarFileGetEntry.getEntryHit            1024  avgt   10   98.075    3.718   ns/op
+ *   gc.alloc.rate.norm                   1024  avgt   10  128.034    0.007    B/op
+ * JarFileGetEntry.getEntryHitUncached    1024  avgt   10  132.998    5.937   ns/op
+ *   gc.alloc.rate.norm                   1024  avgt   10  184.039    0.009    B/op
+ * JarFileGetEntry.getEntryMiss           1024  avgt   10   24.043    0.930   ns/op
+ *   gc.alloc.rate.norm                   1024  avgt   10    0.001    0.001    B/op
+ * JarFileGetEntry.getEntryMissUncached   1024  avgt   10   65.840    3.296   ns/op
+ *   gc.alloc.rate.norm                   1024  avgt   10   56.192    0.003    B/op
+ */
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Thread)
+@Warmup(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
+@Measurement(iterations = 5, time = 1000, timeUnit = TimeUnit.MILLISECONDS)
+@Fork(3)
+public class JarFileGetEntry {
+
+    @Param({"512", "1024"})
+    private int size;
+
+    public JarFile jarFile;
+    public String[]         entryNames;
+    public String[]         missingEntryNames;
+    public StringBuilder[]  entryNameBuilders;
+    public StringBuilder[]  missingEntryNameBuilders;
+
+    public int index = 0;
+
+    @Setup(Level.Trial)
+    public void beforeRun() throws IOException {
+        // Create a test Zip file with the number of entries.
+        File tempFile = Files.createTempFile("jar-mr-micro", ".jar").toFile();
+        tempFile.deleteOnExit();
+
+        entryNameBuilders = new StringBuilder[size];
+        missingEntryNameBuilders = new StringBuilder[size];
+
+        entryNames = new String[size];
+        missingEntryNames = new String[size];
+
+        try (FileOutputStream fos = new FileOutputStream(tempFile);
+             JarOutputStream jos = new JarOutputStream(fos)) {
+
+            Random random = new Random(4711);
+            for (int i = 0; i < size; i++) {
+                String ename = "entry-" + (random.nextInt(90000) + 10000) + "-" + i;
+                jos.putNextEntry(new ZipEntry(ename));
+
+                entryNames[i] = ename;
+                entryNameBuilders[i] = new StringBuilder(ename);
+
+                missingEntryNames[i] = ename + "-";
+                missingEntryNameBuilders[i] = new StringBuilder(missingEntryNames[i]);
+            }
+        }
+
+        jarFile = new JarFile(tempFile);
+    }
+
+    @Benchmark
+    public void getEntryHit() {
+        if (index >= size) {
+            index = 0;
+        }
+        jarFile.getEntry(entryNames[index++]);
+    }
+
+    @Benchmark
+    public void getEntryMiss() {
+        if (index >= size) {
+            index = 0;
+        }
+        jarFile.getEntry(missingEntryNames[index++]);
+    }
+
+    @Benchmark
+    public void getEntryHitUncached() {
+        if (index >= size) {
+            index = 0;
+        }
+        jarFile.getEntry(entryNameBuilders[index++].toString());
+    }
+
+    @Benchmark
+    public void getEntryMissUncached() {
+        if (index >= size) {
+            index = 0;
+        }
+        jarFile.getEntry(missingEntryNameBuilders[index++].toString());
+    }
+}