Strip memory arguments from Decompiler task, trusting java to allocate enough.

Author: LexManos

src/main/java/net/minecraftforge/mcmaven/impl/Mavenizer.java

@@ -74,18 +74,61 @@ public static boolean checkCache(File output, HashStore cache) {
     public static void setDecompileMemory(String value) {
         decompileMemory = value;
     }
-    public static List<String> fillDecompileJvmArgs(List<String> args) {
-        return fillJvmArgs(decompileMemory, args);
+
+    // Default java argumetns are built build by the Image, and env variables
+    // https://github.com/openjdk/jdk/blob/08c8520b39083ec6354dc5df2f18c1f4c3588053/src/hotspot/share/runtime/arguments.cpp#L3628
+    private static final String[] DEFAULT_ARG_ENV = { "JAVA_OPTIONS", "_JAVA_OPTIONS", "JAVA_TOOL_OPIONS"};
+    private static final String[] MEMORY_FLAGS = {"-Xmx", "-XX:MaxHeapSize", "-Xms"};
+    private static void warnAboutMemory() {
+        var found = false;
+        for (var env : DEFAULT_ARG_ENV) {
+            var value = System.getenv(env);
+            if (value == null)
+                continue;
+            for (var flag : MEMORY_FLAGS) {
+                if (value.contains(flag)) {
+                    LOGGER.warn("Detected Heap Size Argument(" + flag + ") in " + env + " environment variable.");
+                    found = true;
+                }
+            }
+        }
+        if (found)
+            LOGGER.warn("Please remove it if you run into memory related issues");
+    }
+
+    public static List<String> fillDecompileJvmArgs(List<String> args, boolean firstRun) {
+        if (!firstRun)
+            return args; // Use the unmodifed args from MCPConfig
+
+        var ret = stripMemoryJvmArgs(args);
+        // If we have an explicit memory size use it
+        if (decompileMemory != null) {
+            ret.add("-Xmx" + decompileMemory);
+        } else {
+            // Don't use any memory arguments and hope java picks the correct values
+            // By default it is 1/4th physical memory on modern JVMs
+            //     https://docs.oracle.com/en/java/javase/21/gctuning/ergonomics.html
+            // There are old JVMs that limit it to 1GB but there is no good way to detect if we're using one so just hope.
+            //     https://docs.oracle.com/javase/8/docs/technotes/guides/vm/gctuning/ergonomics.html
+            // Best we can do is warn about memory argumetns if we see them.
+            warnAboutMemory();
+        }
+        return ret;
     }
-    private static List<String> fillJvmArgs(@Nullable String mx, List<String> args) {
-        if (mx == null)
-            return args;
-        var ret = new ArrayList<String>();
-        ret.add("-Xmx" + mx);
+
+    private static List<String> stripMemoryJvmArgs(List<String> args) {
+        var ret = new ArrayList<String>(args.size());
+
         for (var arg : args) {
-            if (arg.startsWith("-Xmx"))
-                continue;
-            ret.add(arg);
+            var skip = false;
+            for (var flag : MEMORY_FLAGS) {
+                if (arg.startsWith(flag)) {
+                    skip = true;
+                    break;
+                }
+            }
+            if (!skip)
+                ret.add(arg);
         }
         return ret;
     }

src/main/java/net/minecraftforge/mcmaven/impl/repo/mcpconfig/MCPTaskFactory.java

@@ -20,6 +20,7 @@
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.function.BiPredicate;
+import java.util.function.ToIntFunction;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.jar.JarInputStream;
@@ -827,10 +828,23 @@ private File execute(String name, List<TaskOrArg> jvmArgs, List<TaskOrArg> runAr
             throw new IllegalStateException("Failed to find JDK for version " + java_version, e);
         }
 
-        if (isDecompile)
-            jvm = Mavenizer.fillDecompileJvmArgs(jvm);
+        ToIntFunction<String> logHandler = null;
+        if (isDecompile) {
+            jvm = Mavenizer.fillDecompileJvmArgs(jvm, true);
+            logHandler = MCPTaskFactory::parseDecompileLog;
+        }
 
-        var ret = ProcessUtils.runJar(jdk, log.getParentFile(), log, tool, jvm, run, isDecompile ? MCPTaskFactory::parseDecompileLog : null);
+        var ret = ProcessUtils.runJar(jdk, log.getParentFile(), log, tool, jvm, run, logHandler);
+        if (ret.exitCode == OUT_OF_MEMORY && isDecompile) {
+            var newJvm = Mavenizer.fillDecompileJvmArgs(resolveArgs(cache, tasks, jvmArgs), false);
+            if (!newJvm.equals(jvm)) {
+                LOGGER.error("First decompile failed with OutOfMemory using JVM Args: " + jvm);
+                LOGGER.error("Attempting again with: " + newJvm);
+                ret = ProcessUtils.runJar(jdk, log.getParentFile(), log, tool, newJvm, run, logHandler);
+                if (ret.exitCode == OUT_OF_MEMORY)
+                    LOGGER.error("Ran out of memory again, you can specify more manually using the --decompile-memory Mavenizer argument");
+            }
+        }
         if (ret.exitCode != 0)
             throw new IllegalStateException("Failed to run MCP Step (exit code " + ret.exitCode + "), See log: " + log.getAbsolutePath());
 
@@ -892,6 +906,8 @@ private List<String> resolveArgs(HashStore cache, Map<Task, String> tasks, List<
     private static int parseDecompileLog(String line) {
         if (line.startsWith("java.lang.OutOfMemoryError:"))
             return OUT_OF_MEMORY;
+        if (line.startsWith("Exception in thread") && line.contains("java.lang.OutOfMemoryError"))
+            return OUT_OF_MEMORY;
         if (line.contains("ERROR:")) {
             // String message = "Method " + mt.getName() + " " + mt.getDescriptor() + " in class " + cl.qualifiedName + " couldn't be written.";
             // String message = "Method " + mt.getName() + " " + mt.getDescriptor() + " in class " + classWrapper.getClassStruct().qualifiedName + " couldn't be written.";