OpenJDK / jdk / jdk
changeset 59244:e8cebbd8dfac
8193066: Avoid use of capturing lambdas in JarFile
Reviewed-by: lancea, alanb
| author | redestad |
|---|---|
| date | Mon, 11 May 2020 21:43:57 +0200 |
| parents | 34a299b47cd0 |
| children | 78fa29ad0b02 |
| files | src/java.base/share/classes/java/util/jar/JarFile.java src/java.base/share/classes/java/util/jar/JavaUtilJarAccessImpl.java src/java.base/share/classes/java/util/zip/ZipFile.java src/java.base/share/classes/jdk/internal/access/JavaUtilJarAccess.java src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java test/micro/org/openjdk/bench/java/util/jar/JarFileGetEntry.java |
| diffstat | 6 files changed, 191 insertions(+), 66 deletions(-) [+] |
line wrap: on
line diff
--- a/src/java.base/share/classes/java/util/jar/JarFile.java Mon May 11 21:42:23 2020 +0200 +++ b/src/java.base/share/classes/java/util/jar/JarFile.java Mon May 11 21:43:57 2020 +0200 @@ -503,11 +503,11 @@ 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 @@ * 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 @@ * @since 1.8 */ public Stream<JarEntry> stream() { - return JUZFA.stream(this, JarFileEntry::new); + return JUZFA.stream(this); } /** @@ -563,19 +563,11 @@ return stream(); } - /* - * Invokes {@ZipFile}'s getEntry to Return a {@code JarFileEntry} for the - * given entry name or {@code null} if not found. + /** + * Creates a ZipEntry suitable for the given ZipFile. */ - private JarFileEntry getEntry0(String name) { - // Not using a lambda/method reference here to optimize startup time - Function<String, JarEntry> newJarFileEntryFn = new Function<>() { - @Override - public JarEntry apply(String name) { - return new JarFileEntry(name); - } - }; - return (JarFileEntry)JUZFA.getEntry(this, name, newJarFileEntryFn); + JarEntry entryFor(String name) { + return new JarFileEntry(name); } private String getBasename(String name) { @@ -613,7 +605,8 @@ 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 @@ // 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 @@ Enumeration<JarEntry> entries2() { ensureInitialization(); if (jv != null) { - return jv.entries2(this, JUZFA.entries(JarFile.this, - JarFileEntry::new)); + return jv.entries2(this, JUZFA.entries(JarFile.this)); } // 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<>() {
--- a/src/java.base/share/classes/java/util/jar/JavaUtilJarAccessImpl.java Mon May 11 21:42:23 2020 +0200 +++ b/src/java.base/share/classes/java/util/jar/JavaUtilJarAccessImpl.java Mon May 11 21:43:57 2020 +0200 @@ -30,6 +30,9 @@ 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 @@ public boolean isInitializing() { return JarFile.isInitializing(); } + + public JarEntry entryFor(JarFile jar, String name) { + return jar.entryFor(name); + } }
--- a/src/java.base/share/classes/java/util/zip/ZipFile.java Mon May 11 21:42:23 2020 +0200 +++ b/src/java.base/share/classes/java/util/zip/ZipFile.java Mon May 11 21:43:57 2020 +0200 @@ -55,13 +55,13 @@ 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 @@ * @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 @@ 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 @@ 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 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 @@ 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 @@ * 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 @@ private int lastEntryPos; /* Check ensureOpen() before invoking this method */ - private ZipEntry getZipEntry(String name, int pos, - Function<String, ? extends ZipEntry> func) { + private ZipEntry getZipEntry(String name, int pos) { byte[] cen = res.zsrc.cen; int nlen = CENNAM(cen, pos); int elen = CENEXT(cen, pos); @@ -663,7 +645,12 @@ // 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 @@ return ((ZipFile)jar).getMetaInfVersions(); } @Override - public JarEntry getEntry(ZipFile zip, String name, - Function<String, JarEntry> func) { - return (JarEntry)zip.getEntry(name, func); + public Enumeration<JarEntry> entries(ZipFile zip) { + return zip.jarEntries(); } @Override - public Enumeration<JarEntry> entries(ZipFile zip, - Function<String, JarEntry> func) { - return zip.entries(func); - } - @Override - public Stream<JarEntry> stream(ZipFile zip, - Function<String, JarEntry> func) { - return zip.stream(func); + public Stream<JarEntry> stream(ZipFile zip) { + return zip.jarStream(); } @Override public Stream<String> entryNameStream(ZipFile zip) { @@ -1118,6 +1098,9 @@ } 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];
--- a/src/java.base/share/classes/jdk/internal/access/JavaUtilJarAccess.java Mon May 11 21:42:23 2020 +0200 +++ b/src/java.base/share/classes/jdk/internal/access/JavaUtilJarAccess.java Mon May 11 21:43:57 2020 +0200 @@ -46,4 +46,5 @@ public Attributes getTrustedAttributes(Manifest man, String name); public void ensureInitialization(JarFile jar); public boolean isInitializing(); + public JarEntry entryFor(JarFile file, String name); }
--- a/src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java Mon May 11 21:42:23 2020 +0200 +++ b/src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java Mon May 11 21:43:57 2020 +0200 @@ -27,7 +27,6 @@ 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 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, Function<String, JarEntry> func); - public Stream<JarEntry> stream(ZipFile zip, Function<String, JarEntry> func); + public Enumeration<JarEntry> entries(ZipFile zip); + public Stream<JarEntry> stream(ZipFile zip); public Stream<String> entryNameStream(ZipFile zip); }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/micro/org/openjdk/bench/java/util/jar/JarFileGetEntry.java Mon May 11 21:43:57 2020 +0200 @@ -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()); + } +}