From 73f4d50bcb33cb85b19f6a95ddf2b32556dc91ea Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 26 Apr 2022 13:22:37 +0200 Subject: [PATCH] Remove default values for heap options (#610) * Set default max heap size to null Let the JVM decide the max heap size instead of using hardcoded defaults to match the behaviour of vanilla Maven. * Add ITs for verifying max heap behaviour - By default no max heap should be set - If configured via jvm.config then max heap should be set but not mvnd.maxHeapSize - If configured via mvnd.maxHeapSize then max heap should be set * Remove defaults memory options * Add missing test project * Fix too small heap size * Fix tests Co-authored-by: Ashhar Hasan --- .../mvndaemon/mvnd/common/Environment.java | 6 +- .../mvndaemon/mvnd/it/JvmConfigNativeIT.java | 51 -------- .../org/mvndaemon/mvnd/it/MavenConfTest.java | 28 ----- .../mvndaemon/mvnd/it/MaxHeapNativeIT.java | 117 ++++++++++++++++++ .../org/mvndaemon/mvnd/it/MaxHeapTest.java | 37 ++++++ .../test/projects/jvm-config/.mvn/jvm.config | 1 - .../default-heap}/.mvn/maven.config | 0 .../projects/max-heap/default-heap/pom.xml | 27 ++++ .../max-heap/jvm-config/.mvn/jvm.config | 1 + .../max-heap/jvm-config/.mvn/maven.config | 3 + .../{ => max-heap}/jvm-config/pom.xml | 4 +- .../max-heap/mvnd-props/.mvn/maven.config | 3 + .../max-heap/mvnd-props/.mvn/mvnd.properties | 17 +++ .../test/projects/max-heap/mvnd-props/pom.xml | 27 ++++ 14 files changed, 237 insertions(+), 85 deletions(-) delete mode 100644 integration-tests/src/test/java/org/mvndaemon/mvnd/it/JvmConfigNativeIT.java create mode 100644 integration-tests/src/test/java/org/mvndaemon/mvnd/it/MaxHeapNativeIT.java create mode 100644 integration-tests/src/test/java/org/mvndaemon/mvnd/it/MaxHeapTest.java delete mode 100644 integration-tests/src/test/projects/jvm-config/.mvn/jvm.config rename integration-tests/src/test/projects/{jvm-config => max-heap/default-heap}/.mvn/maven.config (100%) create mode 100644 integration-tests/src/test/projects/max-heap/default-heap/pom.xml create mode 100644 integration-tests/src/test/projects/max-heap/jvm-config/.mvn/jvm.config create mode 100644 integration-tests/src/test/projects/max-heap/jvm-config/.mvn/maven.config rename integration-tests/src/test/projects/{ => max-heap}/jvm-config/pom.xml (90%) create mode 100644 integration-tests/src/test/projects/max-heap/mvnd-props/.mvn/maven.config create mode 100644 integration-tests/src/test/projects/max-heap/mvnd-props/.mvn/mvnd.properties create mode 100644 integration-tests/src/test/projects/max-heap/mvnd-props/pom.xml diff --git a/common/src/main/java/org/mvndaemon/mvnd/common/Environment.java b/common/src/main/java/org/mvndaemon/mvnd/common/Environment.java index 90c9d77c..472f04a0 100644 --- a/common/src/main/java/org/mvndaemon/mvnd/common/Environment.java +++ b/common/src/main/java/org/mvndaemon/mvnd/common/Environment.java @@ -204,17 +204,17 @@ public enum Environment { * The -Xms value to pass to the daemon. * This option takes precedence over options specified in {@link #MVND_JVM_ARGS}. */ - MVND_MIN_HEAP_SIZE("mvnd.minHeapSize", null, "128M", OptionType.MEMORY_SIZE, Flags.DISCRIMINATING), + MVND_MIN_HEAP_SIZE("mvnd.minHeapSize", null, null, OptionType.MEMORY_SIZE, Flags.DISCRIMINATING | Flags.OPTIONAL), /** * The -Xmx value to pass to the daemon. * This option takes precedence over options specified in {@link #MVND_JVM_ARGS}. */ - MVND_MAX_HEAP_SIZE("mvnd.maxHeapSize", null, "2G", OptionType.MEMORY_SIZE, Flags.DISCRIMINATING), + MVND_MAX_HEAP_SIZE("mvnd.maxHeapSize", null, null, OptionType.MEMORY_SIZE, Flags.DISCRIMINATING | Flags.OPTIONAL), /** * The -Xss value to pass to the daemon. * This option takes precedence over options specified in {@link #MVND_JVM_ARGS}. */ - MVND_THREAD_STACK_SIZE("mvnd.threadStackSize", null, "1M", OptionType.MEMORY_SIZE, Flags.DISCRIMINATING), + MVND_THREAD_STACK_SIZE("mvnd.threadStackSize", null, null, OptionType.MEMORY_SIZE, Flags.DISCRIMINATING | Flags.OPTIONAL), /** * Additional JVM args to pass to the daemon. * The content of the .mvn/jvm.config file will prepended (and thus with diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/JvmConfigNativeIT.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/JvmConfigNativeIT.java deleted file mode 100644 index 38e1327e..00000000 --- a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/JvmConfigNativeIT.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright 2019-2022 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.mvndaemon.mvnd.it; - -import java.io.IOException; -import java.util.stream.Collectors; -import javax.inject.Inject; -import org.junit.jupiter.api.Test; -import org.mvndaemon.mvnd.assertj.TestClientOutput; -import org.mvndaemon.mvnd.client.Client; -import org.mvndaemon.mvnd.client.DaemonParameters; -import org.mvndaemon.mvnd.junit.MvndNativeTest; - -import static org.junit.jupiter.api.Assertions.assertTrue; - -@MvndNativeTest(projectDir = "src/test/projects/jvm-config") -public class JvmConfigNativeIT { - - @Inject - Client client; - - @Inject - DaemonParameters parameters; - - @Test - void version() throws IOException, InterruptedException { - final TestClientOutput o = new TestClientOutput(); - client.execute(o, "org.codehaus.gmaven:groovy-maven-plugin:2.1.1:execute", - "-Dsource=System.out.println(java.lang.management.ManagementFactory.getRuntimeMXBean().getInputArguments())") - .assertSuccess(); - String xmx = "-Xmx512k"; - assertTrue(o.getMessages().stream() - .anyMatch(m -> m.toString().contains(xmx)), - "Output should contain " + xmx + " but is:\n" - + o.getMessages().stream().map(Object::toString).collect(Collectors.joining("\n"))); - } - -} diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MavenConfTest.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MavenConfTest.java index 768ace5a..8efe2c4c 100644 --- a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MavenConfTest.java +++ b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MavenConfTest.java @@ -15,37 +15,9 @@ */ package org.mvndaemon.mvnd.it; -import java.io.IOException; -import javax.inject.Inject; -import org.junit.jupiter.api.Test; -import org.mvndaemon.mvnd.assertj.TestClientOutput; -import org.mvndaemon.mvnd.client.Client; -import org.mvndaemon.mvnd.client.DaemonParameters; import org.mvndaemon.mvnd.junit.MvndTest; -import org.mvndaemon.mvnd.junit.TestRegistry; - -import static org.junit.jupiter.api.Assertions.assertTrue; @MvndTest(projectDir = "src/test/projects/maven-conf") public class MavenConfTest extends MavenConfNativeIT { - @Inject - Client client; - - @Inject - DaemonParameters parameters; - - @Inject - TestRegistry registry; - - @Test - void version() throws IOException, InterruptedException { - final TestClientOutput o = new TestClientOutput(); - client.execute(o, "org.apache.maven.plugins:maven-help-plugin:3.2.0:evaluate", - "-Dexpression=maven.conf", "-q", "-DforceStdout", "--raw-streams").assertSuccess(); - String conf = parameters.mvndHome().resolve("mvn/conf").toString(); - assertTrue(o.getMessages().stream() - .anyMatch(m -> m.toString().contains(conf)), "Output should contain " + conf); - } - } diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MaxHeapNativeIT.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MaxHeapNativeIT.java new file mode 100644 index 00000000..116550a1 --- /dev/null +++ b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MaxHeapNativeIT.java @@ -0,0 +1,117 @@ +/* + * Copyright 2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mvndaemon.mvnd.it; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import javax.inject.Inject; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mvndaemon.mvnd.assertj.TestClientOutput; +import org.mvndaemon.mvnd.client.Client; +import org.mvndaemon.mvnd.junit.MvndNativeTest; +import org.slf4j.LoggerFactory; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class MaxHeapNativeIT { + + static class BaseTest { + + @Inject + Client client; + + static ListAppender appender = new ListAppender<>(); + + @BeforeAll + static void setup() { + Logger logger = (Logger) LoggerFactory.getLogger("org.mvndaemon.mvnd.client.DaemonConnector"); + logger.setLevel(Level.DEBUG); + logger.addAppender(appender); + appender.start(); + } + + @AfterAll + static void tearDown() { + Logger logger = (Logger) LoggerFactory.getLogger("org.mvndaemon.mvnd.client.DaemonConnector"); + logger.detachAppender(appender); + } + + static String getDaemonArgs() { + return appender.list.stream() + .filter(e -> e.getMessage().contains("Starting daemon process")) + .map(e -> e.getArgumentArray()[2].toString()) + .findAny().orElseThrow(); + } + + @BeforeEach + void unitSetup() { + appender.list.clear(); + } + + } + + @MvndNativeTest(projectDir = "src/test/projects/max-heap/default-heap") + static class DefaultConfig extends BaseTest { + + @Test + void noXmxPassedByDefault() throws InterruptedException { + final TestClientOutput output = new TestClientOutput(); + client.execute(output, "-Dmvnd.log.level=DEBUG", "org.codehaus.gmaven:groovy-maven-plugin:2.1.1:execute", + "-Dsource=System.out.println(java.lang.management.ManagementFactory.getRuntimeMXBean().getInputArguments())") + .assertSuccess(); + String daemonArgs = getDaemonArgs(); + assertTrue(!daemonArgs.contains("-Xmx") && !daemonArgs.contains("mvnd.maxHeapSize"), + "Args must not contain -Xmx or mvnd.maxHeapSize but is:\n" + daemonArgs); + } + } + + @MvndNativeTest(projectDir = "src/test/projects/max-heap/jvm-heap") + static class JvmConfig extends BaseTest { + + @Test + void xmxFromJvmConfig() throws InterruptedException { + final TestClientOutput output = new TestClientOutput(); + client.execute(output, "-Dmvnd.log.level=DEBUG", "org.codehaus.gmaven:groovy-maven-plugin:2.1.1:execute", + "-Dsource=System.out.println(java.lang.management.ManagementFactory.getRuntimeMXBean().getInputArguments())") + .assertSuccess(); + String daemonArgs = getDaemonArgs(); + assertTrue(!daemonArgs.contains("-Xmx") && !daemonArgs.contains("mvnd.maxHeapSize"), + "Args must not contain -Xmx or mvnd.maxHeapSize but is:\n" + daemonArgs); + } + } + + @MvndNativeTest(projectDir = "src/test/projects/max-heap/mvnd-props") + static class MvndProps extends BaseTest { + + @Test + void xmxFromMvndProperties() throws InterruptedException { + final TestClientOutput output = new TestClientOutput(); + client.execute(output, "-Dmvnd.log.level=DEBUG", "org.codehaus.gmaven:groovy-maven-plugin:2.1.1:execute", + "-Dsource=System.out.println(java.lang.management.ManagementFactory.getRuntimeMXBean().getInputArguments())") + .assertSuccess(); + String daemonArgs = getDaemonArgs(); + assertTrue(daemonArgs.contains("-Xmx130M") && daemonArgs.contains("mvnd.maxHeapSize=130M"), + "Args must contain -Xmx130M or mvnd.maxHeapSize=130M but is:\n" + daemonArgs); + } + + } + +} diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MaxHeapTest.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MaxHeapTest.java new file mode 100644 index 00000000..2b974d5e --- /dev/null +++ b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/MaxHeapTest.java @@ -0,0 +1,37 @@ +/* + * Copyright 2019-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mvndaemon.mvnd.it; + +import org.mvndaemon.mvnd.junit.MvndTest; + +public class MaxHeapTest extends MaxHeapNativeIT { + + @MvndTest(projectDir = "src/test/projects/max-heap/default-heap") + static class DefaultConfig extends MaxHeapNativeIT.DefaultConfig { + + } + + @MvndTest(projectDir = "src/test/projects/max-heap/jvm-config") + static class JvmConfig extends MaxHeapNativeIT.JvmConfig { + + } + + @MvndTest(projectDir = "src/test/projects/max-heap/mvnd-props") + static class MvndProps extends MaxHeapNativeIT.MvndProps { + + } + +} diff --git a/integration-tests/src/test/projects/jvm-config/.mvn/jvm.config b/integration-tests/src/test/projects/jvm-config/.mvn/jvm.config deleted file mode 100644 index b25be441..00000000 --- a/integration-tests/src/test/projects/jvm-config/.mvn/jvm.config +++ /dev/null @@ -1 +0,0 @@ --Xmx512k \ No newline at end of file diff --git a/integration-tests/src/test/projects/jvm-config/.mvn/maven.config b/integration-tests/src/test/projects/max-heap/default-heap/.mvn/maven.config similarity index 100% rename from integration-tests/src/test/projects/jvm-config/.mvn/maven.config rename to integration-tests/src/test/projects/max-heap/default-heap/.mvn/maven.config diff --git a/integration-tests/src/test/projects/max-heap/default-heap/pom.xml b/integration-tests/src/test/projects/max-heap/default-heap/pom.xml new file mode 100644 index 00000000..efa03df2 --- /dev/null +++ b/integration-tests/src/test/projects/max-heap/default-heap/pom.xml @@ -0,0 +1,27 @@ + + + + 4.0.0 + org.mvndaemon.mvnd.test.max-heap + max-heap-default-heap + 0.0.1-SNAPSHOT + pom + + \ No newline at end of file diff --git a/integration-tests/src/test/projects/max-heap/jvm-config/.mvn/jvm.config b/integration-tests/src/test/projects/max-heap/jvm-config/.mvn/jvm.config new file mode 100644 index 00000000..47195e23 --- /dev/null +++ b/integration-tests/src/test/projects/max-heap/jvm-config/.mvn/jvm.config @@ -0,0 +1 @@ +-Xmx140M \ No newline at end of file diff --git a/integration-tests/src/test/projects/max-heap/jvm-config/.mvn/maven.config b/integration-tests/src/test/projects/max-heap/jvm-config/.mvn/maven.config new file mode 100644 index 00000000..0ea53cd5 --- /dev/null +++ b/integration-tests/src/test/projects/max-heap/jvm-config/.mvn/maven.config @@ -0,0 +1,3 @@ +-Dmaven.wagon.httpconnectionManager.ttlSeconds=120 +-Dmaven.wagon.http.retryHandler.requestSentEnabled=true +-Dmaven.wagon.http.retryHandler.count=10 \ No newline at end of file diff --git a/integration-tests/src/test/projects/jvm-config/pom.xml b/integration-tests/src/test/projects/max-heap/jvm-config/pom.xml similarity index 90% rename from integration-tests/src/test/projects/jvm-config/pom.xml rename to integration-tests/src/test/projects/max-heap/jvm-config/pom.xml index 657d03b6..c7fdedd0 100644 --- a/integration-tests/src/test/projects/jvm-config/pom.xml +++ b/integration-tests/src/test/projects/max-heap/jvm-config/pom.xml @@ -19,8 +19,8 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> 4.0.0 - org.mvndaemon.mvnd.test.jvm-config - jvm-config + org.mvndaemon.mvnd.test.max-heap + max-heap-jvm-config 0.0.1-SNAPSHOT pom diff --git a/integration-tests/src/test/projects/max-heap/mvnd-props/.mvn/maven.config b/integration-tests/src/test/projects/max-heap/mvnd-props/.mvn/maven.config new file mode 100644 index 00000000..4230c241 --- /dev/null +++ b/integration-tests/src/test/projects/max-heap/mvnd-props/.mvn/maven.config @@ -0,0 +1,3 @@ +-Dmaven.wagon.httpconnectionManager.ttlSeconds=120 +-Dmaven.wagon.http.retryHandler.requestSentEnabled=true +-Dmaven.wagon.http.retryHandler.count=10 diff --git a/integration-tests/src/test/projects/max-heap/mvnd-props/.mvn/mvnd.properties b/integration-tests/src/test/projects/max-heap/mvnd-props/.mvn/mvnd.properties new file mode 100644 index 00000000..6e71ae5c --- /dev/null +++ b/integration-tests/src/test/projects/max-heap/mvnd-props/.mvn/mvnd.properties @@ -0,0 +1,17 @@ +# +# Copyright 2019 the original author or authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +mvnd.maxHeapSize=130M \ No newline at end of file diff --git a/integration-tests/src/test/projects/max-heap/mvnd-props/pom.xml b/integration-tests/src/test/projects/max-heap/mvnd-props/pom.xml new file mode 100644 index 00000000..c7fdedd0 --- /dev/null +++ b/integration-tests/src/test/projects/max-heap/mvnd-props/pom.xml @@ -0,0 +1,27 @@ + + + + 4.0.0 + org.mvndaemon.mvnd.test.max-heap + max-heap-jvm-config + 0.0.1-SNAPSHOT + pom + + \ No newline at end of file