Skip to content

Commit

Permalink
Prevent memory copy of list of Jar/Aar file entries. (#404)
Browse files Browse the repository at this point in the history
See #403 discussion, credit for this optimization goes to @Stephan202.

Note the comment edit for the AAR case. ZipFile is just surprisingly
hostile to copy-free for-each iteration.
  • Loading branch information
lazaroclapp committed Jun 29, 2020
1 parent 7f9aacb commit 3e2233b
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package com.uber.nullaway.jarinfer;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import java.io.BufferedReader;
Expand All @@ -24,6 +23,7 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.jar.JarEntry;
Expand Down Expand Up @@ -280,7 +280,7 @@ public static void annotateBytecodeInJar(
// Reference: https://bugs.openjdk.java.net/browse/JDK-8215788
// Note: we can't just put the code below inside stream().forach(), because it can throw
// IOException.
for (JarEntry jarEntry : inputJar.stream().collect(ImmutableList.toImmutableList())) {
for (JarEntry jarEntry : (Iterable<JarEntry>) inputJar.stream()::iterator) {
InputStream is = inputJar.getInputStream(jarEntry);
copyAndAnnotateJarEntry(
jarEntry,
Expand Down Expand Up @@ -317,10 +317,13 @@ public static void annotateBytecodeInAar(
LOG(debug, "DEBUG", "nullableReturns: " + nullableReturns);
LOG(debug, "DEBUG", "nonnullParams: " + nonnullParams);
// Error Prone doesn't like usages of the old Java Enumerator APIs. ZipFile does not implement
// Iterable, and likely
// never will (see https://bugs.openjdk.java.net/browse/JDK-6581715). So this seems like the
// Iterable, and likely never will (see https://bugs.openjdk.java.net/browse/JDK-6581715).
// Additionally, inputZip.stream() returns a Stream<? extends ZipEntry>, and a for-each loop
// has trouble handling the corresponding ::iterator method reference. So this seems like the
// best remaining way:
for (ZipEntry zipEntry : inputZip.stream().collect(ImmutableList.toImmutableList())) {
Iterator<? extends ZipEntry> zipIterator = inputZip.stream().iterator();
while (zipIterator.hasNext()) {
ZipEntry zipEntry = zipIterator.next();
InputStream is = inputZip.getInputStream(zipEntry);
zipOS.putNextEntry(new ZipEntry(zipEntry.getName()));
if (zipEntry.getName().equals("classes.jar")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
package com.uber.nullaway.jarinfer;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.io.InputStream;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.jar.JarEntry;
Expand Down Expand Up @@ -54,7 +54,9 @@ public static boolean checkMethodAnnotationsInAar(
String aarFile, Map<String, String> expectedToActualAnnotations) throws IOException {
Preconditions.checkArgument(aarFile.endsWith(".aar"), "invalid aar file: " + aarFile);
ZipFile zip = new ZipFile(aarFile);
for (ZipEntry zipEntry : zip.stream().collect(ImmutableList.toImmutableList())) {
Iterator<? extends ZipEntry> zipIterator = zip.stream().iterator();
while (zipIterator.hasNext()) {
ZipEntry zipEntry = zipIterator.next();
if (zipEntry.getName().equals("classes.jar")) {
JarInputStream jarIS = new JarInputStream(zip.getInputStream(zipEntry));
JarEntry jarEntry = jarIS.getNextJarEntry();
Expand Down Expand Up @@ -87,7 +89,7 @@ public static boolean checkMethodAnnotationsInJar(
String jarFile, Map<String, String> expectedToActualAnnotations) throws IOException {
Preconditions.checkArgument(jarFile.endsWith(".jar"), "invalid jar file: " + jarFile);
JarFile jar = new JarFile(jarFile);
for (JarEntry entry : jar.stream().collect(ImmutableList.toImmutableList())) {
for (JarEntry entry : (Iterable<JarEntry>) jar.stream()::iterator) {
if (entry.getName().endsWith(".class")
&& !checkMethodAnnotationsInClass(
jar.getInputStream(entry), expectedToActualAnnotations)) {
Expand Down

0 comments on commit 3e2233b

Please sign in to comment.