From 7a3e34885ecbf712ec0b2e02e45795669ddc123c Mon Sep 17 00:00:00 2001 From: Glavo Date: Mon, 10 Apr 2023 16:42:46 +0800 Subject: [PATCH 1/6] Optimize the nameMap of ZipFile --- .../compress/archivers/zip/ZipFile.java | 57 +++++++++++++------ 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java index 19f08f16000..e33d1d66b65 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java @@ -30,6 +30,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Comparator; @@ -371,9 +372,14 @@ public static void closeQuietly(final ZipFile zipFile) { private final List entries = new LinkedList<>(); /** - * Maps String to list of ZipArchiveEntrys, name -> actual entries. + * Maps a string to the first entry named it. */ - private final Map> nameMap = new HashMap<>(HASH_SIZE); + private final Map nameMap = new HashMap<>(HASH_SIZE); + + /** + * If multiple entries have the same name, maps the name to entries named it. + */ + private Map> duplicateNameMap = null; /** * The encoding to use for file names and the file comment. @@ -792,11 +798,24 @@ private BoundedArchiveInputStream createBoundedInputStream(final long start, fin private void fillNameMap() { entries.forEach(ze -> { - // entries is filled in populateFromCentralDirectory and + // entries are filled in populateFromCentralDirectory and // never modified final String name = ze.getName(); - final LinkedList entriesOfThatName = nameMap.computeIfAbsent(name, k -> new LinkedList<>()); - entriesOfThatName.addLast(ze); + ZipArchiveEntry firstEntry = nameMap.putIfAbsent(name, ze); + + if (firstEntry != null) { + if (duplicateNameMap == null) { + duplicateNameMap = new HashMap<>(); + } + + List entriesOfThatName = duplicateNameMap.computeIfAbsent(name, k -> { + ArrayList list = new ArrayList<>(2); + list.add(firstEntry); + return list; + }); + + entriesOfThatName.add(ze); + } }); } @@ -868,9 +887,12 @@ public Enumeration getEntries() { * @since 1.6 */ public Iterable getEntries(final String name) { - final List entriesOfThatName = nameMap.get(name); - return entriesOfThatName != null ? entriesOfThatName - : Collections.emptyList(); + List entriesOfThatName = duplicateNameMap == null ? null : duplicateNameMap.get(name); + if (entriesOfThatName == null) { + ZipArchiveEntry entry = nameMap.get(name); + entriesOfThatName = entry == null ? Collections.emptyList() : Collections.singletonList(entry); + } + return entriesOfThatName; } /** @@ -899,13 +921,17 @@ public Enumeration getEntriesInPhysicalOrder() { * @since 1.6 */ public Iterable getEntriesInPhysicalOrder(final String name) { - ZipArchiveEntry[] entriesOfThatName = ZipArchiveEntry.EMPTY_ARRAY; - final LinkedList linkedList = nameMap.get(name); - if (linkedList != null) { - entriesOfThatName = linkedList.toArray(entriesOfThatName); - Arrays.sort(entriesOfThatName, offsetComparator); + if (duplicateNameMap != null) { + List list = duplicateNameMap.get(name); + if (list != null) { + ZipArchiveEntry[] entriesOfThatName = list.toArray(ZipArchiveEntry.EMPTY_ARRAY); + Arrays.sort(entriesOfThatName, offsetComparator); + return Arrays.asList(entriesOfThatName); + } } - return Arrays.asList(entriesOfThatName); + + final ZipArchiveEntry entry = nameMap.get(name); + return entry == null ? Collections.emptyList() : Collections.singletonList(entry); } /** @@ -921,8 +947,7 @@ public Iterable getEntriesInPhysicalOrder(final String name) { * {@code null} if not present. */ public ZipArchiveEntry getEntry(final String name) { - final LinkedList entriesOfThatName = nameMap.get(name); - return entriesOfThatName != null ? entriesOfThatName.getFirst() : null; + return nameMap.get(name); } /** From 4019b26a13b99d1fc16a4342218c872c8e0a11f9 Mon Sep 17 00:00:00 2001 From: Glavo Date: Wed, 10 May 2023 01:44:06 +0800 Subject: [PATCH 2/6] Add final modifiers --- .../compress/archivers/zip/ZipFile.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java index e33d1d66b65..ab1260e9dbf 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java @@ -379,7 +379,7 @@ public static void closeQuietly(final ZipFile zipFile) { /** * If multiple entries have the same name, maps the name to entries named it. */ - private Map> duplicateNameMap = null; + private Map> duplicateNameMap; /** * The encoding to use for file names and the file comment. @@ -776,7 +776,7 @@ public void copyRawEntries(final ZipArchiveOutputStream target, final ZipArchive final Enumeration src = getEntriesInPhysicalOrder(); while (src.hasMoreElements()) { final ZipArchiveEntry entry = src.nextElement(); - if (predicate.test( entry)) { + if (predicate.test(entry)) { target.addRawArchiveEntry(entry, getRawInputStream(entry)); } } @@ -801,7 +801,7 @@ private void fillNameMap() { // entries are filled in populateFromCentralDirectory and // never modified final String name = ze.getName(); - ZipArchiveEntry firstEntry = nameMap.putIfAbsent(name, ze); + final ZipArchiveEntry firstEntry = nameMap.putIfAbsent(name, ze); if (firstEntry != null) { if (duplicateNameMap == null) { @@ -809,7 +809,7 @@ private void fillNameMap() { } List entriesOfThatName = duplicateNameMap.computeIfAbsent(name, k -> { - ArrayList list = new ArrayList<>(2); + final ArrayList list = new ArrayList<>(2); list.add(firstEntry); return list; }); @@ -887,12 +887,13 @@ public Enumeration getEntries() { * @since 1.6 */ public Iterable getEntries(final String name) { - List entriesOfThatName = duplicateNameMap == null ? null : duplicateNameMap.get(name); - if (entriesOfThatName == null) { - ZipArchiveEntry entry = nameMap.get(name); - entriesOfThatName = entry == null ? Collections.emptyList() : Collections.singletonList(entry); + final List entriesOfThatName = duplicateNameMap == null ? null : duplicateNameMap.get(name); + if (entriesOfThatName != null) { + return Collections.unmodifiableList(entriesOfThatName); + } else { + final ZipArchiveEntry entry = nameMap.get(name); + return entry == null ? Collections.emptyList() : Collections.singletonList(entry); } - return entriesOfThatName; } /** @@ -922,9 +923,9 @@ public Enumeration getEntriesInPhysicalOrder() { */ public Iterable getEntriesInPhysicalOrder(final String name) { if (duplicateNameMap != null) { - List list = duplicateNameMap.get(name); + final List list = duplicateNameMap.get(name); if (list != null) { - ZipArchiveEntry[] entriesOfThatName = list.toArray(ZipArchiveEntry.EMPTY_ARRAY); + final ZipArchiveEntry[] entriesOfThatName = list.toArray(ZipArchiveEntry.EMPTY_ARRAY); Arrays.sort(entriesOfThatName, offsetComparator); return Arrays.asList(entriesOfThatName); } From 77585b49b93653b80d238f453ca8c14f3f8f3268 Mon Sep 17 00:00:00 2001 From: Glavo Date: Wed, 10 May 2023 01:49:29 +0800 Subject: [PATCH 3/6] Add final modifiers --- .../java/org/apache/commons/compress/archivers/zip/ZipFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java index ab1260e9dbf..ac486cea648 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java @@ -808,7 +808,7 @@ private void fillNameMap() { duplicateNameMap = new HashMap<>(); } - List entriesOfThatName = duplicateNameMap.computeIfAbsent(name, k -> { + final List entriesOfThatName = duplicateNameMap.computeIfAbsent(name, k -> { final ArrayList list = new ArrayList<>(2); list.add(firstEntry); return list; From c07b7dfe241aeca32ce6c71903ae540fac8e3d31 Mon Sep 17 00:00:00 2001 From: Glavo Date: Wed, 10 May 2023 02:47:28 +0800 Subject: [PATCH 4/6] flip branches --- .../apache/commons/compress/archivers/zip/ZipFile.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java index ac486cea648..55f1f3941a9 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java @@ -888,11 +888,11 @@ public Enumeration getEntries() { */ public Iterable getEntries(final String name) { final List entriesOfThatName = duplicateNameMap == null ? null : duplicateNameMap.get(name); - if (entriesOfThatName != null) { - return Collections.unmodifiableList(entriesOfThatName); - } else { + if (entriesOfThatName == null) { final ZipArchiveEntry entry = nameMap.get(name); - return entry == null ? Collections.emptyList() : Collections.singletonList(entry); + return entry != null ? Collections.singletonList(entry) : Collections.emptyList(); + } else { + return Collections.unmodifiableList(entriesOfThatName); } } @@ -932,7 +932,7 @@ public Iterable getEntriesInPhysicalOrder(final String name) { } final ZipArchiveEntry entry = nameMap.get(name); - return entry == null ? Collections.emptyList() : Collections.singletonList(entry); + return entry != null ? Collections.singletonList(entry) : Collections.emptyList(); } /** From 71c3641cce2ff37cca497198214c4bc32b3aeb16 Mon Sep 17 00:00:00 2001 From: Glavo Date: Tue, 18 Jul 2023 10:48:49 +0800 Subject: [PATCH 5/6] Delete EMPTY_LINKED_LIST --- .../apache/commons/compress/archivers/zip/ZipArchiveEntry.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java index 62f93b9aaae..bf4fe6fc6a4 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java @@ -26,7 +26,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Date; -import java.util.LinkedList; import java.util.List; import java.util.NoSuchElementException; import java.util.Objects; @@ -229,7 +228,6 @@ public enum NameSource { } static final ZipArchiveEntry[] EMPTY_ARRAY = {}; - static LinkedList EMPTY_LINKED_LIST = new LinkedList<>(); public static final int PLATFORM_UNIX = 3; public static final int PLATFORM_FAT = 0; From b7cb0f02e9ddd660e2158b87638c8a3d55a2b071 Mon Sep 17 00:00:00 2001 From: Glavo Date: Sat, 9 Dec 2023 05:29:02 +0800 Subject: [PATCH 6/6] Update comments --- .../org/apache/commons/compress/archivers/zip/ZipFile.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java index 97d624e3d17..3a5f309b183 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java @@ -369,12 +369,12 @@ public static void closeQuietly(final ZipFile zipFile) { private final List entries = new LinkedList<>(); /** - * Maps a string to the first entry named it. + * Maps a string to the first entry by that name. */ private final Map nameMap = new HashMap<>(HASH_SIZE); /** - * If multiple entries have the same name, maps the name to entries named it. + * If multiple entries have the same name, maps the name to entries by that name. */ private Map> duplicateNameMap; @@ -763,6 +763,7 @@ private void fillNameMap() { } final List entriesOfThatName = duplicateNameMap.computeIfAbsent(name, k -> { + // Create a list when there are two entries with the same name final ArrayList list = new ArrayList<>(2); list.add(firstEntry); return list;