From d49f21043b84ebcc8b9176de3a84621ca7bca8fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eirik=20Bj=C3=B8rsn=C3=B8s?= Date: Mon, 28 Oct 2024 18:21:18 +0000 Subject: [PATCH] 8342040: Further improve entry lookup performance for multi-release JARs Co-authored-by: Claes Redestad Reviewed-by: redestad --- .../share/classes/java/util/jar/JarFile.java | 29 +++----- .../share/classes/java/util/zip/ZipFile.java | 72 +++++++++---------- .../access/JavaUtilZipFileAccess.java | 3 +- .../bench/java/util/jar/JarFileGetEntry.java | 23 +++++- 4 files changed, 64 insertions(+), 63 deletions(-) diff --git a/src/java.base/share/classes/java/util/jar/JarFile.java b/src/java.base/share/classes/java/util/jar/JarFile.java index 3dc3a49dc77..3a1f412e9c7 100644 --- a/src/java.base/share/classes/java/util/jar/JarFile.java +++ b/src/java.base/share/classes/java/util/jar/JarFile.java @@ -40,6 +40,7 @@ import java.io.InputStream; import java.lang.ref.SoftReference; import java.security.CodeSigner; import java.security.cert.Certificate; +import java.util.BitSet; import java.util.Enumeration; import java.util.List; import java.util.Objects; @@ -597,26 +598,16 @@ public class JarFile extends ZipFile { } private JarEntry getVersionedEntry(String name, JarEntry defaultEntry) { - if (!name.startsWith(META_INF)) { - int[] versions = JUZFA.getMetaInfVersions(this); - if (BASE_VERSION_FEATURE < versionFeature && versions.length > 0) { - // search for versioned entry - for (int i = versions.length - 1; i >= 0; i--) { - int version = versions[i]; - // skip versions above versionFeature - if (version > versionFeature) { - continue; - } - // skip versions below base version - if (version < BASE_VERSION_FEATURE) { - break; - } - JarFileEntry vje = (JarFileEntry)super.getEntry( - META_INF_VERSIONS + version + "/" + name); - if (vje != null) { - return vje.withBasename(name); - } + if (BASE_VERSION_FEATURE < versionFeature && !name.startsWith(META_INF)) { + BitSet versions = JUZFA.getMetaInfVersions(this, name); + int version = versions.previousSetBit(versionFeature); + while (version >= BASE_VERSION_FEATURE) { + JarFileEntry vje = (JarFileEntry)super.getEntry( + META_INF_VERSIONS + version + "/" + name); + if (vje != null) { + return vje.withBasename(name); } + version = versions.previousSetBit(version - 1); } } return defaultEntry; diff --git a/src/java.base/share/classes/java/util/zip/ZipFile.java b/src/java.base/share/classes/java/util/zip/ZipFile.java index 21b9593c017..792b317a006 100644 --- a/src/java.base/share/classes/java/util/zip/ZipFile.java +++ b/src/java.base/share/classes/java/util/zip/ZipFile.java @@ -37,23 +37,7 @@ import java.nio.charset.Charset; import java.nio.file.InvalidPathException; import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.Files; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Deque; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Locale; -import java.util.Objects; -import java.util.NoSuchElementException; -import java.util.Set; -import java.util.Spliterator; -import java.util.Spliterators; -import java.util.TreeSet; -import java.util.WeakHashMap; +import java.util.*; import java.util.function.Consumer; import java.util.function.IntFunction; import java.util.jar.JarEntry; @@ -64,6 +48,7 @@ import jdk.internal.access.JavaUtilZipFileAccess; import jdk.internal.access.JavaUtilJarAccess; import jdk.internal.access.SharedSecrets; import jdk.internal.util.ArraysSupport; +import jdk.internal.util.DecimalDigits; import jdk.internal.util.OperatingSystem; import jdk.internal.perf.PerfCounter; import jdk.internal.ref.CleanerFactory; @@ -1090,19 +1075,23 @@ public class ZipFile implements ZipConstants, Closeable { } /** - * Returns the versions for which there exists a non-directory - * entry that begin with "META-INF/versions/" (case ignored). + * Returns a BitSet where the set bits represents versions found for + * the given entry name. For performance reasons, the name is looked + * up only by hashcode, meaning the result is an over-approximation. * This method is used in JarFile, via SharedSecrets, as an * optimization when looking up potentially versioned entries. - * Returns an empty array if no versioned entries exist. + * Returns an empty BitSet if no versioned entries exist for this + * name. */ - private int[] getMetaInfVersions() { + private BitSet getMetaInfVersions(String name) { synchronized (this) { ensureOpen(); - return res.zsrc.metaVersions; + return res.zsrc.metaVersions.getOrDefault(ZipCoder.hash(name), EMPTY_VERSIONS); } } + private static final BitSet EMPTY_VERSIONS = new BitSet(); + /** * Returns the value of the System property which indicates whether the * Extra ZIP64 validation should be disabled. @@ -1139,8 +1128,8 @@ public class ZipFile implements ZipConstants, Closeable { return ((ZipFile)jar).getManifestName(onlyIfHasSignatureRelatedFiles); } @Override - public int[] getMetaInfVersions(JarFile jar) { - return ((ZipFile)jar).getMetaInfVersions(); + public BitSet getMetaInfVersions(JarFile jar, String name) { + return ((ZipFile)jar).getMetaInfVersions(name); } @Override public Enumeration entries(ZipFile zip) { @@ -1175,7 +1164,8 @@ public class ZipFile implements ZipConstants, Closeable { private static final JavaUtilJarAccess JUJA = SharedSecrets.javaUtilJarAccess(); // "META-INF/".length() private static final int META_INF_LEN = 9; - private static final int[] EMPTY_META_VERSIONS = new int[0]; + // "META-INF/versions//".length() + private static final int META_INF_VERSIONS_LEN = 19; // CEN size is limited to the maximum array size in the JDK private static final int MAX_CEN_SIZE = ArraysSupport.SOFT_MAX_ARRAY_LENGTH; @@ -1192,7 +1182,7 @@ public class ZipFile implements ZipConstants, Closeable { private int manifestPos = -1; // position of the META-INF/MANIFEST.MF, if exists private int manifestNum = 0; // number of META-INF/MANIFEST.MF, case insensitive private int[] signatureMetaNames; // positions of signature related entries, if such exist - private int[] metaVersions; // list of unique versions found in META-INF/versions/ + private Map metaVersions; // Versions found in META-INF/versions/, by entry name hash private final boolean startsWithLoc; // true, if ZIP file starts with LOCSIG (usually true) // A Hashmap for all entries. @@ -1574,7 +1564,7 @@ public class ZipFile implements ZipConstants, Closeable { manifestPos = -1; manifestNum = 0; signatureMetaNames = null; - metaVersions = EMPTY_META_VERSIONS; + metaVersions = null; } private static final int BUF_SIZE = 8192; @@ -1759,8 +1749,6 @@ public class ZipFile implements ZipConstants, Closeable { // list for all meta entries ArrayList signatureNames = null; - // Set of all version numbers seen in META-INF/versions/ - Set metaVersionsSet = null; // Iterate through the entries in the central directory int idx = 0; // Index into the entries array @@ -1799,9 +1787,19 @@ public class ZipFile implements ZipConstants, Closeable { // performance in multi-release jar files int version = getMetaVersion(entryPos + META_INF_LEN, nlen - META_INF_LEN); if (version > 0) { - if (metaVersionsSet == null) - metaVersionsSet = new TreeSet<>(); - metaVersionsSet.add(version); + try { + // Compute hash code of name from "META-INF/versions/{version)/{name} + int prefixLen = META_INF_VERSIONS_LEN + DecimalDigits.stringSize(version); + int hashCode = zipCoderForPos(pos).checkedHash(cen, + entryPos + prefixLen, + nlen - prefixLen); + // Register version for this hash code + if (metaVersions == null) + metaVersions = new HashMap<>(); + metaVersions.computeIfAbsent(hashCode, _ -> new BitSet()).set(version); + } catch (Exception e) { + throw new IllegalArgumentException(e); + } } } } @@ -1819,14 +1817,8 @@ public class ZipFile implements ZipConstants, Closeable { signatureMetaNames[j] = signatureNames.get(j); } } - if (metaVersionsSet != null) { - metaVersions = new int[metaVersionsSet.size()]; - int c = 0; - for (Integer version : metaVersionsSet) { - metaVersions[c++] = version; - } - } else { - metaVersions = EMPTY_META_VERSIONS; + if (metaVersions == null) { + metaVersions = Map.of(); } if (pos != cen.length) { zerror("invalid CEN header (bad header size)"); diff --git a/src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java b/src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java index 2c9d904005d..9ff0f2e8b92 100644 --- a/src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java +++ b/src/java.base/share/classes/jdk/internal/access/JavaUtilZipFileAccess.java @@ -25,6 +25,7 @@ package jdk.internal.access; +import java.util.BitSet; import java.util.Enumeration; import java.util.List; import java.util.jar.JarEntry; @@ -38,7 +39,7 @@ public interface JavaUtilZipFileAccess { public List getManifestAndSignatureRelatedFiles(JarFile zip); public String getManifestName(JarFile zip, boolean onlyIfSignatureRelatedFiles); public int getManifestNum(JarFile zip); - public int[] getMetaInfVersions(JarFile zip); + public BitSet getMetaInfVersions(JarFile zip, String name); public Enumeration entries(ZipFile zip); public Stream stream(ZipFile zip); public Stream entryNameStream(ZipFile zip); diff --git a/test/micro/org/openjdk/bench/java/util/jar/JarFileGetEntry.java b/test/micro/org/openjdk/bench/java/util/jar/JarFileGetEntry.java index bc2182e00b4..a0c3938e8eb 100644 --- a/test/micro/org/openjdk/bench/java/util/jar/JarFileGetEntry.java +++ b/test/micro/org/openjdk/bench/java/util/jar/JarFileGetEntry.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2024, 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 @@ -31,9 +31,12 @@ import java.io.IOException; import java.nio.file.Files; import java.util.Random; import java.util.concurrent.TimeUnit; +import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; /** * Simple benchmark measuring cost of looking up entries in a jar file. @@ -71,6 +74,9 @@ public class JarFileGetEntry { @Param({"512", "1024"}) private int size; + @Param({"false", "true"}) + private boolean mr; + public JarFile jarFile; public String[] entryNames; public String[] missingEntryNames; @@ -91,9 +97,20 @@ public class JarFileGetEntry { entryNames = new String[size]; missingEntryNames = new String[size]; + Manifest man = new Manifest(); + man.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); + if (mr) { + man.getMainAttributes().put(Attributes.Name.MULTI_RELEASE, "true"); + } try (FileOutputStream fos = new FileOutputStream(tempFile); - JarOutputStream jos = new JarOutputStream(fos)) { + JarOutputStream jos = new JarOutputStream(fos, man)) { + if (mr) { + // Add a few versioned entries + jos.putNextEntry(new ZipEntry("META-INF/versions/9/module-info.class")); + jos.putNextEntry(new ZipEntry("META-INF/versions/17/foo/library/Library.class")); + jos.putNextEntry(new ZipEntry("META-INF/versions/21/foo/library/Library.class")); + } Random random = new Random(4711); for (int i = 0; i < size; i++) { String ename = "entry-" + (random.nextInt(90000) + 10000) + "-" + i; @@ -107,7 +124,7 @@ public class JarFileGetEntry { } } - jarFile = new JarFile(tempFile); + jarFile = new JarFile(tempFile, true, ZipFile.OPEN_READ, mr ? JarFile.runtimeVersion() : JarFile.baseVersion()); } @Benchmark