Adjust JVM arguments priority (#1898)

* Adjust JVM arguments priority

* #1889: Use the default JVM arguments consistent with the official launcher

* Follow -Xmx settings in modpack info

* Do not force generation of -Xmx

HMCL/src/main/java/org/jackhuang/hmcl/game/HMCLGameRepository.java

@@ -337,7 +337,7 @@ public class HMCLGameRepository extends DefaultGameRepository {
                 .setProfileName(Metadata.TITLE)
                 .setGameArguments(StringUtils.tokenize(vs.getMinecraftArgs()))
                 .setJavaArguments(StringUtils.tokenize(vs.getJavaArgs()))
-                .setMaxMemory((int)(getAllocatedMemory(
+                .setMaxMemory(vs.isNoJVMArgs() && vs.isAutoMemory() ? null : (int)(getAllocatedMemory(
                         vs.getMaxMemory() * 1024L * 1024L,
                         OperatingSystem.getPhysicalMemoryStatus().orElse(OperatingSystem.PhysicalMemoryStatus.INVALID).getAvailable(),
                         vs.isAutoMemory()
@@ -380,6 +380,9 @@ public class HMCLGameRepository extends DefaultGameRepository {
             }
         }
 
+        if (vs.isAutoMemory() && builder.getJavaArguments().stream().anyMatch(it -> it.startsWith("-Xmx")))
+            builder.setMaxMemory(null);
+
         return builder.create();
     }
 

HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java

@@ -105,7 +105,47 @@ public class DefaultLauncher extends Launcher {
 
         res.add(options.getJava().getBinary().toString());
 
-        res.addAllWithoutParsing(options.getJavaArguments());
+        // Fix RCE vulnerability of log4j2
+        res.addDefault("-Djava.rmi.server.useCodebaseOnly=", "true");
+        res.addDefault("-Dcom.sun.jndi.rmi.object.trustURLCodebase=", "false");
+        res.addDefault("-Dcom.sun.jndi.cosnaming.object.trustURLCodebase=", "false");
+
+        String formatMsgNoLookups = res.addDefault("-Dlog4j2.formatMsgNoLookups=", "true");
+        if (!"-Dlog4j2.formatMsgNoLookups=false".equals(formatMsgNoLookups) && isUsingLog4j()) {
+            res.addDefault("-Dlog4j.configurationFile=", getLog4jConfigurationFile().getAbsolutePath());
+        }
+
+        Proxy proxy = options.getProxy();
+        if (proxy != null && StringUtils.isBlank(options.getProxyUser()) && StringUtils.isBlank(options.getProxyPass())) {
+            InetSocketAddress address = (InetSocketAddress) options.getProxy().address();
+            if (address != null) {
+                String host = address.getHostString();
+                int port = address.getPort();
+                if (proxy.type() == Proxy.Type.HTTP) {
+                    res.addDefault("-Dhttp.proxyHost=", host);
+                    res.addDefault("-Dhttp.proxyPort=", String.valueOf(port));
+                    res.addDefault("-Dhttps.proxyHost=", host);
+                    res.addDefault("-Dhttps.proxyPort=", String.valueOf(port));
+                } else if (proxy.type() == Proxy.Type.SOCKS) {
+                    res.addDefault("-DsocksProxyHost=", host);
+                    res.addDefault("-DsocksProxyPort=", String.valueOf(port));
+                }
+            }
+        }
+
+        if (options.getMaxMemory() != null && options.getMaxMemory() > 0)
+            res.addDefault("-Xmx", options.getMaxMemory() + "m");
+
+        if (options.getMinMemory() != null && options.getMinMemory() > 0)
+            res.addDefault("-Xms", options.getMinMemory() + "m");
+
+        if (options.getMetaspace() != null && options.getMetaspace() > 0)
+            if (options.getJava().getParsedVersion() < JavaVersion.JAVA_8)
+                res.addDefault("-XX:PermSize=", options.getMetaspace() + "m");
+            else
+                res.addDefault("-XX:MetaspaceSize=", options.getMetaspace() + "m");
+
+        res.addAllDefaultWithoutParsing(options.getJavaArguments());
 
         Charset encoding = OperatingSystem.NATIVE_CHARSET;
         String fileEncoding = res.addDefault("-Dfile.encoding=", encoding.name());
@@ -119,7 +159,7 @@ public class DefaultLauncher extends Launcher {
         res.addDefault("-Dsun.stdout.encoding=", encoding.name());
         res.addDefault("-Dsun.stderr.encoding=", encoding.name());
 
-        // JVM Args
+        // Default JVM Args
         if (!options.isNoGeneratedJVMArgs()) {
             appendJvmArgs(res);
 
@@ -151,20 +191,13 @@ public class DefaultLauncher extends Launcher {
                     res.addUnstableDefault("G1NewSizePercent", "20");
                     res.addUnstableDefault("G1ReservePercent", "20");
                     res.addUnstableDefault("MaxGCPauseMillis", "50");
-                    res.addUnstableDefault("G1HeapRegionSize", "16m");
+                    res.addUnstableDefault("G1HeapRegionSize", "32m");
                 }
             }
 
-            if (options.getMetaspace() != null && options.getMetaspace() > 0)
-                if (options.getJava().getParsedVersion() < JavaVersion.JAVA_8)
-                    res.addDefault("-XX:PermSize=", options.getMetaspace() + "m");
-                else
-                    res.addDefault("-XX:MetaspaceSize=", options.getMetaspace() + "m");
-
             res.addUnstableDefault("UseAdaptiveSizePolicy", false);
             res.addUnstableDefault("OmitStackTraceInFastThrow", false);
             res.addUnstableDefault("DontCompileHugeMethods", false);
-            res.addDefault("-Xmn", "128m");
 
             // As 32-bit JVM allocate 320KB for stack by default rather than 64-bit version allocating 1MB,
             // causing Minecraft 1.13 crashed accounting for java.lang.StackOverflowError.
@@ -172,12 +205,6 @@ public class DefaultLauncher extends Launcher {
                 res.addDefault("-Xss", "1m");
             }
 
-            if (options.getMaxMemory() != null && options.getMaxMemory() > 0)
-                res.addDefault("-Xmx", options.getMaxMemory() + "m");
-
-            if (options.getMinMemory() != null && options.getMinMemory() > 0)
-                res.addDefault("-Xms", options.getMinMemory() + "m");
-
             if (options.getJava().getParsedVersion() == JavaVersion.JAVA_16)
                 res.addDefault("--illegal-access=", "permit");
 
@@ -185,34 +212,6 @@ public class DefaultLauncher extends Launcher {
             res.addDefault("-Dfml.ignorePatchDiscrepancies=", "true");
         }
 
-        // Fix RCE vulnerability of log4j2
-        res.addDefault("-Djava.rmi.server.useCodebaseOnly=", "true");
-        res.addDefault("-Dcom.sun.jndi.rmi.object.trustURLCodebase=", "false");
-        res.addDefault("-Dcom.sun.jndi.cosnaming.object.trustURLCodebase=", "false");
-
-        String formatMsgNoLookups = res.addDefault("-Dlog4j2.formatMsgNoLookups=", "true");
-        if (!"-Dlog4j2.formatMsgNoLookups=false".equals(formatMsgNoLookups) && isUsingLog4j()) {
-            res.addDefault("-Dlog4j.configurationFile=", getLog4jConfigurationFile().getAbsolutePath());
-        }
-
-        Proxy proxy = options.getProxy();
-        if (proxy != null && StringUtils.isBlank(options.getProxyUser()) && StringUtils.isBlank(options.getProxyPass())) {
-            InetSocketAddress address = (InetSocketAddress) options.getProxy().address();
-            if (address != null) {
-                String host = address.getHostString();
-                int port = address.getPort();
-                if (proxy.type() == Proxy.Type.HTTP) {
-                    res.addDefault("-Dhttp.proxyHost=", host);
-                    res.addDefault("-Dhttp.proxyPort=", String.valueOf(port));
-                    res.addDefault("-Dhttps.proxyHost=", host);
-                    res.addDefault("-Dhttps.proxyPort=", String.valueOf(port));
-                } else if (proxy.type() == Proxy.Type.SOCKS) {
-                    res.addDefault("-DsocksProxyHost=", host);
-                    res.addDefault("-DsocksProxyPort=", String.valueOf(port));
-                }
-            }
-        }
-
         List<String> classpath = repository.getClasspath(version);
 
         File jar = repository.getVersionJar(version);

HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/CommandBuilder.java

@@ -83,28 +83,97 @@ public final class CommandBuilder {
         return this;
     }
 
-    public String addDefault(String opt) {
+    public void addAllDefault(Collection<String> args) {
-        for (Item item : raw) {
+        addAllDefault(args, true);
-            if (item.arg.equals(opt)) {
+    }
-                return item.arg;
+
+    public void addAllDefaultWithoutParsing(Collection<String> args) {
+        addAllDefault(args, false);
+    }
+
+    private void addAllDefault(Collection<String> args, boolean parse) {
+        loop:
+        for (String arg : args) {
+            if (arg.startsWith("-D")) {
+                int idx = arg.indexOf('=');
+                if (idx >= 0) {
+                    addDefault(arg.substring(0, idx + 1), arg.substring(idx + 1), parse);
+                } else {
+                    String opt = arg + "=";
+                    for (Item item : raw) {
+                        if (item.arg.startsWith(opt)) {
+                            LOG.info("Default option '" + arg + "' is suppressed by '" + item.arg + "'");
+                            continue loop;
+                        } else if (item.arg.equals(arg)) {
+                            continue loop;
+                        }
+                    }
+                    raw.add(new Item(arg, parse));
+                }
+                continue;
             }
+
+            if (arg.startsWith("-XX:")) {
+                Matcher matcher = UNSTABLE_OPTION_PATTERN.matcher(arg);
+                if (matcher.matches()) {
+                    addUnstableDefault(matcher.group("key"), matcher.group("value"), parse);
+                    continue;
+                }
+
+                matcher = UNSTABLE_BOOLEAN_OPTION_PATTERN.matcher(arg);
+                if (matcher.matches()) {
+                    addUnstableDefault(matcher.group("key"), "+".equals(matcher.group("value")), parse);
+                    continue;
+                }
+            }
+
+            if (arg.startsWith("-X")) {
+                String opt = null;
+                String value = null;
+
+                for (String prefix : new String[]{"-Xmx", "-Xms", "-Xmn", "-Xss"}) {
+                    if (arg.startsWith(prefix)) {
+                        opt = prefix;
+                        value = arg.substring(prefix.length());
+                        break;
+                    }
+                }
+
+                if (opt != null) {
+                    addDefault(opt, value, parse);
+                    continue;
+                }
+            }
+
+            for (Item item : raw) {
+                if (item.arg.equals(arg)) {
+                    continue loop;
+                }
+            }
+            raw.add(new Item(arg, parse));
         }
-        raw.add(new Item(opt, true));
-        return null;
     }
 
     public String addDefault(String opt, String value) {
+        return addDefault(opt, value, true);
+    }
+
+    private String addDefault(String opt, String value, boolean parse) {
         for (Item item : raw) {
             if (item.arg.startsWith(opt)) {
                 LOG.info("Default option '" + opt + value + "' is suppressed by '" + item.arg + "'");
                 return item.arg;
             }
         }
-        raw.add(new Item(opt + value, true));
+        raw.add(new Item(opt + value, parse));
         return null;
     }
 
     public String addUnstableDefault(String opt, boolean value) {
+        return addUnstableDefault(opt, value, true);
+    }
+
+    private String addUnstableDefault(String opt, boolean value, boolean parse) {
         for (Item item : raw) {
             final Matcher matcher = UNSTABLE_BOOLEAN_OPTION_PATTERN.matcher(item.arg);
             if (matcher.matches()) {
@@ -115,14 +184,18 @@ public final class CommandBuilder {
         }
 
         if (value) {
-            raw.add(new Item("-XX:+" + opt, true));
+            raw.add(new Item("-XX:+" + opt, parse));
         } else {
-            raw.add(new Item("-XX:-" + opt, true));
+            raw.add(new Item("-XX:-" + opt, parse));
         }
         return null;
     }
 
     public String addUnstableDefault(String opt, String value) {
+        return addUnstableDefault(opt, value, true);
+    }
+
+    private String addUnstableDefault(String opt, String value, boolean parse) {
         for (Item item : raw) {
             final Matcher matcher = UNSTABLE_OPTION_PATTERN.matcher(item.arg);
             if (matcher.matches()) {
@@ -132,7 +205,7 @@ public final class CommandBuilder {
             }
         }
 
-        raw.add(new Item("-XX:" + opt + "=" + value, true));
+        raw.add(new Item("-XX:" + opt + "=" + value, parse));
         return null;
     }