Revert "Make SourceRemapper threaded (#446)"

This reverts commit ef2816b0d0.

src/main/java/net/fabricmc/loom/configuration/LoomDependencyManager.java

@@ -25,7 +25,6 @@
 package net.fabricmc.loom.configuration;
 
 import java.io.File;
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -170,15 +169,7 @@ public class LoomDependencyManager {
 
 		ModCompileRemapper.remapDependencies(project, mappingsIdentifier, extension, sourceRemapper);
 
-		long start = System.currentTimeMillis();
+		sourceRemapper.remapAll();
-
-		try {
-			sourceRemapper.remapAll();
-		} catch (IOException exception) {
-			throw new RuntimeException("Failed to remap mod sources", exception);
-		}
-
-		project.getLogger().info("Source remapping took: %dms".formatted(System.currentTimeMillis() - start));
 
 		for (Runnable runnable : afterTasks) {
 			runnable.run();

src/main/java/net/fabricmc/loom/configuration/RemapConfiguration.java

@@ -108,6 +108,7 @@ public class RemapConfiguration {
 
 				rootProject.getTasks().register(remapAllSourcesTaskName, RemapAllSourcesTask.class, task -> {
 					task.sourceRemapper = sourceRemapper;
+					task.doLast(t -> sourceRemapper.remapAll());
 				});
 
 				parentTask = rootProject.getTasks().getByName(remapAllSourcesTaskName);

src/main/java/net/fabricmc/loom/task/RemapAllSourcesTask.java

@@ -24,10 +24,7 @@
 
 package net.fabricmc.loom.task;
 
-import java.io.IOException;
-
 import org.gradle.api.tasks.Internal;
-import org.gradle.api.tasks.TaskAction;
 
 import net.fabricmc.loom.util.SourceRemapper;
 
@@ -38,13 +35,4 @@ public class RemapAllSourcesTask extends AbstractLoomTask {
 	public SourceRemapper getSourceRemapper() {
 		return sourceRemapper;
 	}
-
-	@TaskAction
-	public void remap() {
-		try {
-			sourceRemapper.remapAll();
-		} catch (IOException exception) {
-			throw new RuntimeException("Failed to remap mod", exception);
-		}
-	}
 }

src/main/java/net/fabricmc/loom/util/SourceRemapper.java

@@ -29,27 +29,20 @@ import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Objects;
 import java.util.Set;
-import java.util.concurrent.CompletableFuture;
+import java.util.function.Consumer;
-import java.util.concurrent.CompletionException;
-import java.util.concurrent.ConcurrentLinkedDeque;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 
 import org.cadixdev.lorenz.MappingSet;
 import org.cadixdev.mercury.Mercury;
-import org.cadixdev.mercury.SourceProcessor;
 import org.cadixdev.mercury.remapper.MercuryRemapper;
 import org.gradle.api.Project;
-import org.gradle.api.plugins.JavaPlugin;
 import org.zeroturnaround.zip.ZipUtil;
 
 import net.fabricmc.loom.LoomGradleExtension;
 import net.fabricmc.loom.configuration.RemappedConfigurationEntry;
 import net.fabricmc.loom.configuration.providers.mappings.MappingsProviderImpl;
+import net.fabricmc.loom.util.gradle.ProgressLogger;
 import net.fabricmc.lorenztiny.TinyMappingsReader;
 import net.fabricmc.mapping.tree.TinyTree;
 import net.fabricmc.stitch.util.StitchUtil;
@@ -57,171 +50,115 @@ import net.fabricmc.stitch.util.StitchUtil;
 public class SourceRemapper {
 	private final Project project;
 	private final boolean toNamed;
-	private final List<RemapTask> remapTasks = new ArrayList<>();
+	private final List<Consumer<ProgressLogger>> remapTasks = new ArrayList<>();
-	private boolean hasStartedRemap = false;
+
+	private Mercury mercury;
 
 	public SourceRemapper(Project project, boolean toNamed) {
 		this.project = project;
 		this.toNamed = toNamed;
 	}
 
-	public static void remapSources(Project project, File input, File output, boolean named, boolean reproducibleFileOrder, boolean preserveFileTimestamps) throws IOException {
+	public static void remapSources(Project project, File input, File output, boolean named, boolean reproducibleFileOrder, boolean preserveFileTimestamps) {
 		SourceRemapper sourceRemapper = new SourceRemapper(project, named);
 		sourceRemapper.scheduleRemapSources(input, output, reproducibleFileOrder, preserveFileTimestamps);
 		sourceRemapper.remapAll();
 	}
 
 	public void scheduleRemapSources(File source, File destination, boolean reproducibleFileOrder, boolean preserveFileTimestamps) {
-		this.scheduleRemapSources(new RemapTask(source, destination, reproducibleFileOrder, preserveFileTimestamps));
+		remapTasks.add((logger) -> {
+			try {
+				logger.progress("remapping sources - " + source.getName());
+				remapSourcesInner(source, destination);
+				ZipReprocessorUtil.reprocessZip(destination, reproducibleFileOrder, preserveFileTimestamps);
+
+				// Set the remapped sources creation date to match the sources if we're likely succeeded in making it
+				destination.setLastModified(source.lastModified());
+			} catch (Exception e) {
+				// Failed to remap, lets clean up to ensure we try again next time
+				destination.delete();
+				throw new RuntimeException("Failed to remap sources for " + source, e);
+			}
+		});
 	}
 
-	public synchronized void scheduleRemapSources(RemapTask data) {
+	public void remapAll() {
-		if (hasStartedRemap) {
-			throw new UnsupportedOperationException("Cannot add data after remapping has started");
-		}
-
-		this.remapTasks.add(data);
-	}
-
-	public void remapAll() throws IOException {
-		hasStartedRemap = true;
-
 		if (remapTasks.isEmpty()) {
 			return;
 		}
 
-		int threads = Math.min(remapTasks.size(), Math.max(2, Runtime.getRuntime().availableProcessors() / 2));
+		project.getLogger().lifecycle(":remapping sources");
-		ExecutorService ioExecutor = Executors.newFixedThreadPool(2);
-		ExecutorService remapExecutor = Executors.newFixedThreadPool(threads);
 
-		List<CompletableFuture<Void>> completableFutures = new ArrayList<>();
+		ProgressLogger progressLogger = ProgressLogger.getProgressFactory(project, SourceRemapper.class.getName());
+		progressLogger.start("Remapping dependency sources", "sources");
 
-		{
+		remapTasks.forEach(consumer -> consumer.accept(progressLogger));
-			// We have to build the Mercury instances on the main thread as createMercuryRemapper resolves gradle stuff
-			// TODO refactor this a bit to not do that.
-			var mercuryQueue = new ConcurrentLinkedDeque<Mercury>();
 
-			final var remapper = createMercuryRemapper();
+		progressLogger.completed();
-			final var inputClasspath = getInputClasspath(project);
-
-			for (int i = 0; i < threads; i++) {
-				Mercury mercury = createMercuryWithClassPath(project, toNamed);
-				mercury.getProcessors().add(remapper);
-				mercury.getClassPath().addAll(inputClasspath);
-
-				mercuryQueue.add(mercury);
-			}
-
-			ThreadLocal<Mercury> mercuryThreadLocal = ThreadLocal.withInitial(() -> Objects.requireNonNull(mercuryQueue.pop(), "No Mercury instances left"));
-
-			for (RemapTask remapTask : this.remapTasks) {
-				completableFutures.add(
-						CompletableFuture.supplyAsync(() ->
-							prepareForRemap(remapTask), ioExecutor)
-							.thenApplyAsync(remapData -> doRemap(mercuryThreadLocal.get(), remapData), remapExecutor)
-							.thenAcceptAsync(remapData -> completeRemap(remapData, remapTask), ioExecutor)
-				);
-			}
-		}
-
-		for (CompletableFuture<Void> future : completableFutures) {
-			try {
-				future.join();
-			} catch (CompletionException e) {
-				try {
-					throw e.getCause();
-				} catch (IOException ioe) {
-					throw ioe;
-				} catch (Throwable unknown) {
-					throw new RuntimeException("An unknown exception occured when remapping", unknown);
-				}
-			}
-		}
-
-		ioExecutor.shutdown();
-		remapExecutor.shutdown();
 
 		// TODO: FIXME - WORKAROUND https://github.com/FabricMC/fabric-loom/issues/45
 		System.gc();
 	}
 
-	private RemapData prepareForRemap(RemapTask remapTask) {
+	private void remapSourcesInner(File source, File destination) throws Exception {
-		try {
+		project.getLogger().info(":remapping source jar");
-			File source = remapTask.source();
+		Mercury mercury = getMercuryInstance();
-			final File destination = remapTask.destination();
 
-			if (source.equals(destination)) {
+		if (source.equals(destination)) {
-				if (source.isDirectory()) {
+			if (source.isDirectory()) {
-					throw new RuntimeException("Directories must differ!");
+				throw new RuntimeException("Directories must differ!");
-				}
-
-				source = new File(destination.getAbsolutePath().substring(0, destination.getAbsolutePath().lastIndexOf('.')) + "-dev.jar");
-
-				try {
-					com.google.common.io.Files.move(destination, source);
-				} catch (IOException e) {
-					throw new RuntimeException("Could not rename " + destination.getName() + "!", e);
-				}
 			}
 
-			Path srcPath = source.toPath();
+			source = new File(destination.getAbsolutePath().substring(0, destination.getAbsolutePath().lastIndexOf('.')) + "-dev.jar");
-			boolean isSrcTmp = false;
 
-			if (!source.isDirectory()) {
+			try {
-				// create tmp directory
+				com.google.common.io.Files.move(destination, source);
-				isSrcTmp = true;
+			} catch (IOException e) {
-				srcPath = Files.createTempDirectory("fabric-loom-src");
+				throw new RuntimeException("Could not rename " + destination.getName() + "!", e);
-				ZipUtil.unpack(source, srcPath.toFile());
 			}
-
-			if (!destination.isDirectory() && destination.exists()) {
-				if (!destination.delete()) {
-					throw new RuntimeException("Could not delete " + destination.getName() + "!");
-				}
-			}
-
-			StitchUtil.FileSystemDelegate dstFs = remapTask.destination().isDirectory() ? null : StitchUtil.getJarFileSystem(remapTask.destination(), true);
-			Path dstPath = dstFs != null ? dstFs.get().getPath("/") : remapTask.destination().toPath();
-
-			return new RemapData(Objects.requireNonNull(srcPath, "srcPath"), Objects.requireNonNull(dstPath, "dstPath"), dstFs, isSrcTmp);
-		} catch (IOException e) {
-			throw new CompletionException("Failed to prepare for remap", e);
 		}
-	}
 
-	private RemapData doRemap(Mercury mercury, RemapData remapData) {
+		Path srcPath = source.toPath();
+		boolean isSrcTmp = false;
+
+		if (!source.isDirectory()) {
+			// create tmp directory
+			isSrcTmp = true;
+			srcPath = Files.createTempDirectory("fabric-loom-src");
+			ZipUtil.unpack(source, srcPath.toFile());
+		}
+
+		if (!destination.isDirectory() && destination.exists()) {
+			if (!destination.delete()) {
+				throw new RuntimeException("Could not delete " + destination.getName() + "!");
+			}
+		}
+
+		StitchUtil.FileSystemDelegate dstFs = destination.isDirectory() ? null : StitchUtil.getJarFileSystem(destination, true);
+		Path dstPath = dstFs != null ? dstFs.get().getPath("/") : destination.toPath();
+
 		try {
-			mercury.rewrite(remapData.source(), remapData.destination());
+			mercury.rewrite(srcPath, dstPath);
 		} catch (Exception e) {
-			project.getLogger().warn("Could not remap " + remapData.source().toString() + " fully!", e);
+			project.getLogger().warn("Could not remap " + source.getName() + " fully!", e);
-			// TODO do something more with this error? delete the dst file in complete remap?
 		}
 
-		return remapData;
+		copyNonJavaFiles(srcPath, dstPath, project, source);
-	}
 
-	private void completeRemap(RemapData remapData, RemapTask remapTask) {
+		if (dstFs != null) {
-		try {
+			dstFs.close();
-			copyNonJavaFiles(remapData.source(), remapData.destination(), project, remapTask.source());
+		}
 
-			if (remapData.dstFs() != null) {
+		if (isSrcTmp) {
-				remapData.dstFs().close();
+			Files.walkFileTree(srcPath, new DeletingFileVisitor());
-			}
-
-			if (remapData.isSrcTmp()) {
-				Files.walkFileTree(remapData.source(), new DeletingFileVisitor());
-			}
-
-			ZipReprocessorUtil.reprocessZip(remapTask.destination(), remapTask.reproducibleFileOrder(), remapTask.preserveFileTimestamps());
-
-			// Set the remapped sources creation date to match the sources if we're likely succeeded in making it
-			remapTask.destination().setLastModified(remapTask.source().lastModified());
-		} catch (IOException e) {
-			throw new CompletionException("Failed to complete remap", e);
 		}
 	}
 
-	private SourceProcessor createMercuryRemapper() {
+	private Mercury getMercuryInstance() {
+		if (this.mercury != null) {
+			return this.mercury;
+		}
+
 		LoomGradleExtension extension = LoomGradleExtension.get(project);
 		MappingsProviderImpl mappingsProvider = extension.getMappingsProvider();
 
@@ -235,7 +172,35 @@ public class SourceRemapper {
 			}
 		});
 
-		return MercuryRemapper.create(mappings);
+		Mercury mercury = extension.getOrCreateSrcMercuryCache(toNamed ? 1 : 0, () -> {
+			Mercury m = createMercuryWithClassPath(project, toNamed);
+
+			for (File file : extension.getUnmappedModCollection()) {
+				Path path = file.toPath();
+
+				if (Files.isRegularFile(path)) {
+					m.getClassPath().add(path);
+				}
+			}
+
+			m.getClassPath().add(extension.getMinecraftMappedProvider().getMappedJar().toPath());
+			m.getClassPath().add(extension.getMinecraftMappedProvider().getIntermediaryJar().toPath());
+
+			Set<File> files = project.getConfigurations()
+					.detachedConfiguration(project.getDependencies().create(Constants.Dependencies.JETBRAINS_ANNOTATIONS + Constants.Dependencies.Versions.JETBRAINS_ANNOTATIONS))
+					.resolve();
+
+			for (File file : files) {
+				m.getClassPath().add(file.toPath());
+			}
+
+			m.getProcessors().add(MercuryRemapper.create(mappings));
+
+			return m;
+		});
+
+		this.mercury = mercury;
+		return mercury;
 	}
 
 	private static void copyNonJavaFiles(Path from, Path to, Project project, File source) throws IOException {
@@ -253,62 +218,26 @@ public class SourceRemapper {
 	}
 
 	public static Mercury createMercuryWithClassPath(Project project, boolean toNamed) {
-		var mercury = new Mercury();
+		Mercury m = new Mercury();
-		mercury.setGracefulClasspathChecks(true);
+		m.setGracefulClasspathChecks(true);
-
-		var classpath = mercury.getClassPath();
-		classpath.addAll(getCompileClasspath(project, toNamed));
-
-		return mercury;
-	}
-
-	private static Set<Path> getCompileClasspath(Project project, boolean toNamed) {
-		var classpath = new HashSet<Path>();
 
 		for (File file : project.getConfigurations().getByName(Constants.Configurations.LOADER_DEPENDENCIES).getFiles()) {
-			classpath.add(file.toPath());
+			m.getClassPath().add(file.toPath());
 		}
 
 		if (!toNamed) {
-			for (File file : project.getConfigurations().getByName(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME).getFiles()) {
+			for (File file : project.getConfigurations().getByName("compileClasspath").getFiles()) {
-				classpath.add(file.toPath());
+				m.getClassPath().add(file.toPath());
 			}
 		} else {
 			for (RemappedConfigurationEntry entry : Constants.MOD_COMPILE_ENTRIES) {
 				for (File inputFile : project.getConfigurations().getByName(entry.sourceConfiguration()).getFiles()) {
-					classpath.add(inputFile.toPath());
+					m.getClassPath().add(inputFile.toPath());
 				}
 			}
 		}
 
-		return classpath;
+		return m;
-	}
-
-	private static Set<Path> getInputClasspath(Project project) {
-		LoomGradleExtension extension = LoomGradleExtension.get(project);
-
-		var classpath = new HashSet<Path>();
-
-		for (File file : extension.getUnmappedModCollection()) {
-			Path path = file.toPath();
-
-			if (Files.isRegularFile(path)) {
-				classpath.add(path);
-			}
-		}
-
-		classpath.add(extension.getMinecraftMappedProvider().getMappedJar().toPath());
-		classpath.add(extension.getMinecraftMappedProvider().getIntermediaryJar().toPath());
-
-		Set<File> files = project.getConfigurations()
-				.detachedConfiguration(project.getDependencies().create(Constants.Dependencies.JETBRAINS_ANNOTATIONS + Constants.Dependencies.Versions.JETBRAINS_ANNOTATIONS))
-				.resolve();
-
-		for (File file : files) {
-			classpath.add(file.toPath());
-		}
-
-		return classpath;
 	}
 
 	private static boolean isJavaFile(Path path) {
@@ -316,10 +245,4 @@ public class SourceRemapper {
 		// ".java" is not a valid java file
 		return name.endsWith(".java") && name.length() != 5;
 	}
-
-	public static record RemapTask(File source, File destination, boolean reproducibleFileOrder, boolean preserveFileTimestamps) {
-	}
-
-	public static record RemapData(Path source, Path destination, StitchUtil.FileSystemDelegate dstFs, boolean isSrcTmp) {
-	}
 }