Use buffered IO streams in renamer (#4)

Also fix several unclosed file-handle/stream issues. --------- Co-authored-by: Sebastian Hartte <[email protected]>
neoforged · Mar 26, 2024 · 176a151 · 176a151
1 parent e308285
commit 176a151
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 162 deletions.
diff --git a/build.gradle b/build.gradle
@@ -6,8 +6,8 @@ plugins {
     id 'java-library'
     id 'maven-publish'
     id 'org.cadixdev.licenser' version '0.6.1'
-    id 'com.github.johnrengelman.shadow' version '7.0.0'
-    id 'net.neoforged.gradleutils' version '3.0.0-alpha.5'
+    id 'com.github.johnrengelman.shadow' version '8.1.1'
+    id 'net.neoforged.gradleutils' version '3.0.0-alpha.12'
 }
 apply plugin: 'net.neoforged.gradleutils'
 

diff --git a/src/main/java/net/minecraftforge/fart/Main.java b/src/main/java/net/minecraftforge/fart/Main.java
@@ -15,6 +15,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.function.Consumer;
+import java.util.stream.Stream;
 
 import joptsimple.OptionException;
 import joptsimple.OptionParser;
@@ -167,9 +168,13 @@ private static String[] expandArgs(String[] args) throws IOException {
                 if (x + 1 == args.length)
                     throw new IllegalArgumentException("No value specified for '--cfg'");
 
-                Files.lines(Paths.get(args[++x])).forEach(ret::add);
+                try (Stream<String> lines = Files.lines(Paths.get(args[++x]))) {
+                    lines.forEach(ret::add);
+                }
             } else if (args[x].startsWith("--cfg=")) {
-                Files.lines(Paths.get(args[x].substring(6))).forEach(ret::add);
+                try (Stream<String> lines = Files.lines(Paths.get(args[x].substring(6)))) {
+                    lines.forEach(ret::add);
+                }
             } else {
                 ret.add(args[x]);
             }

diff --git a/src/main/java/net/minecraftforge/fart/internal/ClassProviderImpl.java b/src/main/java/net/minecraftforge/fart/internal/ClassProviderImpl.java
@@ -67,17 +67,17 @@ public Optional<? extends IClassInfo> getClass(String name) {
     }
 
     private Optional<? extends IClassInfo> computeClassInfo(String name) {
-        if (this.classInfos.containsKey(name))
-            return this.classInfos.get(name);
+        Optional<? extends IClassInfo> knownClassInfo = this.classInfos.get(name);
+        if (knownClassInfo != null)
+            return knownClassInfo;
 
         Path source = this.sources.get(name);
 
         if (source == null)
             return Optional.empty();
 
         try {
-            byte[] data = Util.toByteArray(Files.newInputStream(source));
-            return Optional.of(new ClassInfo(data));
+            return Optional.of(new ClassInfo(Files.readAllBytes(source)));
         } catch (IOException e) {
             throw new RuntimeException("Could not get data to compute class info in file: " + source.toAbsolutePath(), e);
         }
@@ -125,11 +125,10 @@ static class ClassInfo implements IClassInfo {
         }
 
         ClassInfo(Class<?> node) {
-            this.name = Util.nameToBytecode(node);
+            this.name = nameToBytecode(node);
             this.access = new Access(node.getModifiers());
-            this.superName = Util.nameToBytecode(node.getSuperclass());
-            this.interfaces = Collections.unmodifiableList(Arrays.stream(node.getInterfaces())
-                .map(c -> Util.nameToBytecode(c)).collect(Collectors.toList()));
+            this.superName = nameToBytecode(node.getSuperclass());
+            this.interfaces = Arrays.stream(node.getInterfaces()).map(ClassInfo::nameToBytecode).collect(Collectors.toList());
 
             Map<String, MethodInfo> mtds = Stream.concat(
                 Arrays.stream(node.getConstructors()).map(MethodInfo::new),
@@ -139,13 +138,17 @@ static class ClassInfo implements IClassInfo {
             this.methods = mtds.isEmpty() ? null : Collections.unmodifiableMap(mtds);
 
             Field[] flds = node.getDeclaredFields();
-            if (flds != null && flds.length > 0) {
+            if (flds.length > 0) {
                 this.fields = Collections.unmodifiableMap(Arrays.asList(flds).stream().map(FieldInfo::new)
                     .collect(Collectors.toMap(FieldInfo::getName, Function.identity())));
             } else
                 this.fields = null;
         }
 
+        private static String nameToBytecode(Class<?> cls) {
+            return cls == null ? null : cls.getName().replace('.', '/');
+        }
+
         @Override
         public String getName() {
             return name;
@@ -289,8 +292,8 @@ public String toString() {
     }
 
     private static class Access {
-        private static int[] ACC = new int[23];
-        private static String[] NAME = new String[23];
+        private static final int[] ACC = new int[23];
+        private static final String[] NAME = new String[23];
         static {
             int idx = 0;
             put(idx++, ACC_PUBLIC,      "public");

diff --git a/src/main/java/net/minecraftforge/fart/internal/EnhancedRemapper.java b/src/main/java/net/minecraftforge/fart/internal/EnhancedRemapper.java
@@ -157,9 +157,9 @@ private class MClass {
         private final String mappedName;
         private final List<MClass> parents;
         private final Map<String, Optional<MField>> fields = new ConcurrentHashMap<>();
-        private Collection<Optional<MField>> fieldsView = Collections.unmodifiableCollection(fields.values());
+        private final Collection<Optional<MField>> fieldsView = Collections.unmodifiableCollection(fields.values());
         private final Map<String, Optional<MMethod>> methods = new ConcurrentHashMap<>();
-        private Collection<Optional<MMethod>> methodsView = Collections.unmodifiableCollection(methods.values());
+        private final Collection<Optional<MMethod>> methodsView = Collections.unmodifiableCollection(methods.values());
 
         MClass(IClassInfo icls, IMappingFile.IClass mcls) {
             if (icls == null && mcls == null)

diff --git a/src/main/java/net/minecraftforge/fart/internal/HashFunction.java b/src/main/java/net/minecraftforge/fart/internal/HashFunction.java
diff --git a/src/main/java/net/minecraftforge/fart/internal/RenamerImpl.java b/src/main/java/net/minecraftforge/fart/internal/RenamerImpl.java
@@ -5,9 +5,13 @@
 
 package net.minecraftforge.fart.internal;
 
+import java.io.BufferedOutputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -101,7 +105,10 @@ public void run(File input, File output) {
                 if (e.isDirectory())
                     continue;
                 String name = e.getName();
-                byte[] data = Util.toByteArray(in.getInputStream(e));
+                byte[] data;
+                try (InputStream entryInput = in.getInputStream(e)) {
+                    data = readAllBytes(entryInput, e.getSize());
+                }
 
                 if (name.endsWith(".class"))
                     oldEntries.add(ClassEntry.create(name, e.getTime(), data));
@@ -158,7 +165,7 @@ else if (name.equals("javadoctor.json"))
             List<Entry> newEntries = async.invokeAll(oldEntries, Entry::getName, this::processEntry);
 
             logger.accept("Adding extras");
-            transformers.stream().forEach(t -> newEntries.addAll(t.getExtras()));
+            transformers.forEach(t -> newEntries.addAll(t.getExtras()));
 
             Set<String> seen = new HashSet<>();
             String dupes = newEntries.stream().map(Entry::getName)
@@ -168,13 +175,6 @@ else if (name.equals("javadoctor.json"))
             if (!dupes.isEmpty())
                 throw new IllegalStateException("Duplicate entries detected: " + dupes);
 
-            /*
-            log("Collecting new hashes");
-            Map<String, String> newHashes = async.invokeAll(newEntries,
-                e -> new Pair<>(e.getName(), HashFunction.SHA256.hash(e.getData()))
-            ).stream().collect(Collectors.toMap(Pair::getLeft, Pair::getRight));
-            */
-
             // We care about stable output, so sort, and single thread write.
             logger.accept("Sorting");
             Collections.sort(newEntries, this::compare);
@@ -188,8 +188,8 @@ else if (name.equals("javadoctor.json"))
             PROGRESS.setStep("Writing output");
 
             logger.accept("Writing Output: " + output.getAbsolutePath());
-            try (FileOutputStream fos = new FileOutputStream(output);
-                ZipOutputStream zos = new ZipOutputStream(fos)) {
+            try (OutputStream fos = new BufferedOutputStream(Files.newOutputStream(output.toPath()));
+                 ZipOutputStream zos = new ZipOutputStream(fos)) {
 
                 int amount = 0;
                 for (Entry e : newEntries) {
@@ -219,6 +219,19 @@ else if (name.equals("javadoctor.json"))
         }
     }
 
+    private byte[] readAllBytes(InputStream in, long size) throws IOException {
+        // This program will crash if size exceeds MAX_INT anyway since arrays are limited to 32-bit indices
+        ByteArrayOutputStream tmp = new ByteArrayOutputStream(size >= 0 ? (int) size : 0);
+
+        byte[] buffer = new byte[8192];
+        int read;
+        while ((read = in.read(buffer)) != -1) {
+            tmp.write(buffer, 0, read);
+        }
+
+        return tmp.toByteArray();
+    }
+
     // Tho Directory entries are not strictly necessary, we add them because some bad implementations of Zip extractors
     // attempt to extract files without making sure the parents exist.
     private void addDirectory(ZipOutputStream zos, Set<String> seen, String path) throws IOException {

diff --git a/src/main/java/net/minecraftforge/fart/internal/Util.java b/src/main/java/net/minecraftforge/fart/internal/Util.java