From 6452509ec5571ab9f4170e282ed5e48bea08d904 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Wed, 20 Dec 2023 16:58:42 +0000 Subject: [PATCH] Optimise IncludedJarFactory & ZipReprocessorUtil No longer processes the jar multiple times, caches the jar in place. ZipReprocessorUtil now directly writes the file to disk, instead of first copying it to memory. --- .../build/nesting/IncludedJarFactory.java | 11 ++--- .../loom/task/AbstractRemapJarTask.java | 2 +- .../fabricmc/loom/util/SourceRemapper.java | 2 +- .../loom/util/ZipReprocessorUtil.java | 45 ++++++++++--------- .../loom/test/unit/ZipUtilsTest.groovy | 4 +- 5 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/build/nesting/IncludedJarFactory.java b/src/main/java/net/fabricmc/loom/build/nesting/IncludedJarFactory.java index 925f5a7c..3cf13e03 100644 --- a/src/main/java/net/fabricmc/loom/build/nesting/IncludedJarFactory.java +++ b/src/main/java/net/fabricmc/loom/build/nesting/IncludedJarFactory.java @@ -94,7 +94,7 @@ public final class IncludedJarFactory { artifact.getClassifier() ); - files.from(project.provider(() -> getNestableJar(artifact.getFile(), metadata))); + files.from(getNestableJar(artifact.getFile(), metadata)); } } @@ -149,7 +149,8 @@ public final class IncludedJarFactory { } LoomGradleExtension extension = LoomGradleExtension.get(project); - File tempDir = new File(extension.getFiles().getProjectBuildCache(), "temp/modprocessing"); + String childName = "temp/modprocessing/%s/%s/%s/%s".formatted(metadata.group().replace(".", "/"), metadata.name(), metadata.version(), input.getName()); + File tempDir = new File(extension.getFiles().getProjectBuildCache(), childName); if (!tempDir.exists()) { tempDir.mkdirs(); @@ -157,13 +158,13 @@ public final class IncludedJarFactory { File tempFile = new File(tempDir, input.getName()); - if (tempFile.exists()) { - tempFile.delete(); + if (tempFile.exists() && FabricModJsonFactory.isModJar(tempFile)) { + return tempFile; } try { FileUtils.copyFile(input, tempFile); - ZipReprocessorUtil.appendZipEntry(tempFile, "fabric.mod.json", generateModForDependency(metadata).getBytes(StandardCharsets.UTF_8)); + ZipReprocessorUtil.appendZipEntry(tempFile.toPath(), "fabric.mod.json", generateModForDependency(metadata).getBytes(StandardCharsets.UTF_8)); } catch (IOException e) { throw new UncheckedIOException("Failed to add dummy mod while including %s".formatted(input), e); } diff --git a/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java b/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java index 7430e0e1..e0f2abf1 100644 --- a/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java @@ -225,7 +225,7 @@ public abstract class AbstractRemapJarTask extends Jar { final ZipEntryCompression compression = getParameters().getEntryCompression().get(); if (isReproducibleFileOrder || !isPreserveFileTimestamps || compression != ZipEntryCompression.DEFLATED) { - ZipReprocessorUtil.reprocessZip(outputFile.toFile(), isReproducibleFileOrder, isPreserveFileTimestamps, compression); + ZipReprocessorUtil.reprocessZip(outputFile, isReproducibleFileOrder, isPreserveFileTimestamps, compression); } } } diff --git a/src/main/java/net/fabricmc/loom/util/SourceRemapper.java b/src/main/java/net/fabricmc/loom/util/SourceRemapper.java index 1f4fca35..f008293d 100644 --- a/src/main/java/net/fabricmc/loom/util/SourceRemapper.java +++ b/src/main/java/net/fabricmc/loom/util/SourceRemapper.java @@ -72,7 +72,7 @@ public class SourceRemapper { try { logger.progress("remapping sources - " + source.getName()); remapSourcesInner(source, destination); - ZipReprocessorUtil.reprocessZip(destination, reproducibleFileOrder, preserveFileTimestamps); + ZipReprocessorUtil.reprocessZip(destination.toPath(), reproducibleFileOrder, preserveFileTimestamps); // Set the remapped sources creation date to match the sources if we're likely succeeded in making it destination.setLastModified(source.lastModified()); diff --git a/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java b/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java index ab8f92a3..e8bbf075 100644 --- a/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java +++ b/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java @@ -24,12 +24,11 @@ package net.fabricmc.loom.util; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.Calendar; import java.util.Comparator; import java.util.GregorianCalendar; @@ -87,16 +86,19 @@ public class ZipReprocessorUtil { return name1.compareTo(name2); } - public static void reprocessZip(File file, boolean reproducibleFileOrder, boolean preserveFileTimestamps) throws IOException { + public static void reprocessZip(Path file, boolean reproducibleFileOrder, boolean preserveFileTimestamps) throws IOException { reprocessZip(file, reproducibleFileOrder, preserveFileTimestamps, ZipEntryCompression.DEFLATED); } - public static void reprocessZip(File file, boolean reproducibleFileOrder, boolean preserveFileTimestamps, ZipEntryCompression zipEntryCompression) throws IOException { + public static void reprocessZip(Path file, boolean reproducibleFileOrder, boolean preserveFileTimestamps, ZipEntryCompression zipEntryCompression) throws IOException { if (!reproducibleFileOrder && preserveFileTimestamps) { return; } - try (var zipFile = new ZipFile(file)) { + final Path tempFile = file.resolveSibling(file.getFileName() + ".tmp"); + + try (var zipFile = new ZipFile(file.toFile()); + var fileOutputStream = Files.newOutputStream(tempFile)) { ZipEntry[] entries; if (reproducibleFileOrder) { @@ -108,9 +110,7 @@ public class ZipReprocessorUtil { .toArray(ZipEntry[]::new); } - final var outZip = new ByteArrayOutputStream(entries.length); - - try (var zipOutputStream = new ZipOutputStream(outZip)) { + try (var zipOutputStream = new ZipOutputStream(fileOutputStream)) { zipOutputStream.setMethod(zipOutputStreamCompressionMethod(zipEntryCompression)); for (ZipEntry entry : entries) { @@ -125,11 +125,9 @@ public class ZipReprocessorUtil { copyZipEntry(zipOutputStream, newEntry, zipFile.getInputStream(entry)); } } - - try (var fileOutputStream = new FileOutputStream(file)) { - outZip.writeTo(fileOutputStream); - } } + + Files.move(tempFile, file, StandardCopyOption.REPLACE_EXISTING); } /** @@ -137,15 +135,20 @@ public class ZipReprocessorUtil { * The new entry is added with a constant time stamp to ensure reproducibility. * This method should only be used when a reproducible output is required, use {@link ZipUtils#add(Path, String, byte[])} normally. */ - public static void appendZipEntry(File file, String path, byte[] data) throws IOException { - try (var zipFile = new ZipFile(file)) { + public static void appendZipEntry(Path file, String path, byte[] data) throws IOException { + final Path tempFile = file.resolveSibling(file.getFileName() + ".tmp"); + + try (var zipFile = new ZipFile(file.toFile()); + var fileOutputStream = Files.newOutputStream(tempFile)) { ZipEntry[] entries = zipFile.stream().toArray(ZipEntry[]::new); - final var outZip = new ByteArrayOutputStream(entries.length); - - try (var zipOutputStream = new ZipOutputStream(outZip)) { + try (var zipOutputStream = new ZipOutputStream(fileOutputStream)) { // Copy existing entries for (ZipEntry entry : entries) { + if (entry.getName().equals(path)) { + throw new IllegalArgumentException("Zip file (%s) already contains entry (%s)".formatted(file.getFileName().toString(), path)); + } + copyZipEntry(zipOutputStream, entry, zipFile.getInputStream(entry)); } @@ -156,11 +159,9 @@ public class ZipReprocessorUtil { zipOutputStream.write(data, 0, data.length); zipOutputStream.closeEntry(); } - - try (var fileOutputStream = new FileOutputStream(file)) { - outZip.writeTo(fileOutputStream); - } } + + Files.move(tempFile, file, StandardCopyOption.REPLACE_EXISTING); } private static void copyZipEntry(ZipOutputStream zipOutputStream, ZipEntry entry, InputStream inputStream) throws IOException { diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy index 1b40e3f3..e967638e 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy @@ -165,11 +165,11 @@ class ZipUtilsTest extends Specification { def fileInside = dir.resolve("text.txt") Files.writeString(fileInside, "hello world") ZipUtils.pack(dir, zip) - ZipReprocessorUtil.reprocessZip(zip.toFile(), true, false) + ZipReprocessorUtil.reprocessZip(zip, true, false) when: // Add an entry to it - ZipReprocessorUtil.appendZipEntry(zip.toFile(), "fabric.mod.json", "Some text".getBytes(StandardCharsets.UTF_8)) + ZipReprocessorUtil.appendZipEntry(zip, "fabric.mod.json", "Some text".getBytes(StandardCharsets.UTF_8)) // Reset the timezone back TimeZone.setDefault(currentTimezone)