From 89e0382224e6827e4d3991af3a7d7b6a23722cc4 Mon Sep 17 00:00:00 2001 From: ChrisTrenkamp Date: Sun, 19 Mar 2023 16:48:59 -0400 Subject: [PATCH 1/5] Fixed isJansiConsoleInstalled performance issue isJansiConsoleInstalled() was repeatedly called in a loop from the TextTable#toString method, which caused performance problems due to the constant calls to Class#forName. This change caches the result from isJansiConsoleInstalled so it doesn't repeatedly call Class#forName, which cannot change at runtime. --- src/main/java/picocli/CommandLine.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java index a7b37d389..bb3ec3a4e 100644 --- a/src/main/java/picocli/CommandLine.java +++ b/src/main/java/picocli/CommandLine.java @@ -17805,7 +17805,14 @@ static boolean ansiPossible() { if (!isTTY() && !isPseudoTTY()) { return false; } return hintEnabled() || !isWindows() || isXterm() || isCygwin() || hasOsType(); } + /** Cache the result for isJansiConsoleInstalled so it doesn't repeatedly + * call Class#forName, which can cause performance issues. */ + static Boolean jansiConsole; static boolean isJansiConsoleInstalled() { + if (jansiConsole == null) { jansiConsole = calcIsJansiConsoleInstalled(); } + return jansiConsole; + } + static boolean calcIsJansiConsoleInstalled() { try { // first check if JANSI was explicitly disabled _without loading any JANSI classes_: // see https://github.com/remkop/picocli/issues/1106 From 2400268db75411250b4643c5b9740b3d93c4249d Mon Sep 17 00:00:00 2001 From: ChrisTrenkamp Date: Mon, 20 Mar 2023 08:40:59 -0400 Subject: [PATCH 2/5] Fixed failing test for Ansi.enabled() Fixed unit test by resetting the cached isJansiConsoleInstalled result. --- src/test/java/picocli/HelpAnsiTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/java/picocli/HelpAnsiTest.java b/src/test/java/picocli/HelpAnsiTest.java index 2855dabca..1bbc47b8b 100644 --- a/src/test/java/picocli/HelpAnsiTest.java +++ b/src/test/java/picocli/HelpAnsiTest.java @@ -128,6 +128,10 @@ public void testAnsiEnabled() { if (isWindows && !Ansi.AUTO.enabled()) { AnsiConsole.systemInstall(); + + // The previous Ansi.enabled() call caches the result for whether or not jansi is enabled. Reset the cache value + // and force the Ansi.enabled() call to rescan the classpath for the jansi classes. + Ansi.jansiConsole = null; try { assertTrue(Ansi.AUTO.enabled()); } finally { From a28578ef61cece7ef7fb731ecc762d53a912cbc1 Mon Sep 17 00:00:00 2001 From: ChrisTrenkamp Date: Mon, 20 Mar 2023 09:02:53 -0400 Subject: [PATCH 3/5] Fixed unexpected Ansi result in ModelTransformerTest#testUsage --- src/test/java/picocli/ModelTransformerTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/java/picocli/ModelTransformerTest.java b/src/test/java/picocli/ModelTransformerTest.java index 6d5b0a747..bec8e1dc7 100644 --- a/src/test/java/picocli/ModelTransformerTest.java +++ b/src/test/java/picocli/ModelTransformerTest.java @@ -2,6 +2,7 @@ import org.junit.Test; import picocli.CommandLine.Command; +import picocli.CommandLine.Help.Ansi; import java.io.PrintWriter; import java.io.StringWriter; @@ -34,7 +35,9 @@ public CommandLine.Model.CommandSpec transform(CommandLine.Model.CommandSpec com @Test public void testUsage() { StringWriter sw = new StringWriter(); - new CommandLine(new MyCommand()).usage(new PrintWriter(sw)); + // Explicitly disable Ansi to make sure that the cached isJansiConsoleInstalled + // value doesn't inadvertently cause the usage help to be enabled. + new CommandLine(new MyCommand()).usage(new PrintWriter(sw), Ansi.OFF); String expected = String.format("" + "Usage: mycmd [-hV] [COMMAND]%n" + " -h, --help Show this help message and exit.%n" + From a8edda3e23bb121115b82071cd13474dfb707666 Mon Sep 17 00:00:00 2001 From: ChrisTrenkamp Date: Mon, 20 Mar 2023 09:03:49 -0400 Subject: [PATCH 4/5] Fixed comment. --- src/test/java/picocli/ModelTransformerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/picocli/ModelTransformerTest.java b/src/test/java/picocli/ModelTransformerTest.java index bec8e1dc7..f6a1ed7e4 100644 --- a/src/test/java/picocli/ModelTransformerTest.java +++ b/src/test/java/picocli/ModelTransformerTest.java @@ -36,7 +36,7 @@ public CommandLine.Model.CommandSpec transform(CommandLine.Model.CommandSpec com public void testUsage() { StringWriter sw = new StringWriter(); // Explicitly disable Ansi to make sure that the cached isJansiConsoleInstalled - // value doesn't inadvertently cause the usage help to be enabled. + // value doesn't inadvertently cause the usage help to enable ansi. new CommandLine(new MyCommand()).usage(new PrintWriter(sw), Ansi.OFF); String expected = String.format("" + "Usage: mycmd [-hV] [COMMAND]%n" + From 7ed559c2bc862e4d32a00415d454d2d63c1ce020 Mon Sep 17 00:00:00 2001 From: ChrisTrenkamp Date: Mon, 20 Mar 2023 10:11:28 -0400 Subject: [PATCH 5/5] Fixed the rest of the failing test cases. --- .../src/test/java/picocli/HelpAnsiHeuristicsTest.java | 5 +++++ .../test/java/picocli/AutoCompleteSystemExitTest.java | 11 +++++++++++ .../src/test/java/picocli/HelpAnsiHeuristicsTest.java | 8 ++++++++ ...1125_1538_OptionNameOrSubcommandAsOptionValue.java | 6 ++++++ 4 files changed, 30 insertions(+) diff --git a/picocli-tests-java567/src/test/java/picocli/HelpAnsiHeuristicsTest.java b/picocli-tests-java567/src/test/java/picocli/HelpAnsiHeuristicsTest.java index 4b4296638..583f804e2 100644 --- a/picocli-tests-java567/src/test/java/picocli/HelpAnsiHeuristicsTest.java +++ b/picocli-tests-java567/src/test/java/picocli/HelpAnsiHeuristicsTest.java @@ -360,6 +360,11 @@ public void testAnsiAutoJansiConsoleInstalledOverridesHintDisabled() { environmentVariables.clear(ANSI_ENVIRONMENT_VARIABLES); environmentVariables.set("CLICOLOR", "0"); // hint disabled System.setProperty("os.name", "Windows"); + // Clear the globally cached jansiConsole value that might + // have been set in a previous test to force the + // Ansi#isJansiConsoleInstalled method to recalculate + // the cached value. + Ansi.jansiConsole = null; assertTrue(Ansi.isWindows()); assertFalse(Ansi.isPseudoTTY()); assertFalse(Ansi.forceDisabled()); diff --git a/picocli-tests-java9plus/src/test/java/picocli/AutoCompleteSystemExitTest.java b/picocli-tests-java9plus/src/test/java/picocli/AutoCompleteSystemExitTest.java index 1c8e612b5..b248544bd 100644 --- a/picocli-tests-java9plus/src/test/java/picocli/AutoCompleteSystemExitTest.java +++ b/picocli-tests-java9plus/src/test/java/picocli/AutoCompleteSystemExitTest.java @@ -1,9 +1,11 @@ package picocli; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledForJreRange; import org.junit.jupiter.api.condition.JRE; +import picocli.CommandLine.Help.Ansi; import java.io.File; import java.io.FileInputStream; @@ -508,6 +510,15 @@ private String expectedCompletionScriptForNonDefault() { CommandLine.VERSION); } + @BeforeAll + static void disableAnsi() { + // Clear the globally cached jansiConsole value that might + // have been set in a previous test to force the + // Ansi#isJansiConsoleInstalled method to recalculate + // the cached value. + Ansi.jansiConsole = null; + } + @Test @DisabledForJreRange(min = JRE.JAVA_18, max = JRE.JAVA_21, disabledReason = "UnsupportedOperationException in SystemLambda.withSecurityManager") public void testAutoCompleteAppHelp() throws Exception { diff --git a/picocli-tests-java9plus/src/test/java/picocli/HelpAnsiHeuristicsTest.java b/picocli-tests-java9plus/src/test/java/picocli/HelpAnsiHeuristicsTest.java index 52b5b0de9..fdb8cec3b 100644 --- a/picocli-tests-java9plus/src/test/java/picocli/HelpAnsiHeuristicsTest.java +++ b/picocli-tests-java9plus/src/test/java/picocli/HelpAnsiHeuristicsTest.java @@ -612,6 +612,12 @@ public void testAnsiAutoJansiConsoleInstalledOverridesHintDisabled() throws Exce assertFalse(Ansi.hintEnabled()); assertFalse(Ansi.isJansiConsoleInstalled()); + + // Clear the globally cached jansiConsole value that might + // have been set in a previous test to force the + // Ansi#isJansiConsoleInstalled method to recalculate + // the cached value. + Ansi.jansiConsole = null; AnsiConsole.systemInstall(); try { assertTrue(Ansi.isJansiConsoleInstalled()); @@ -628,6 +634,7 @@ public void testAnsiAutoHintDisabledOverridesHintEnabled() throws Exception { restoreSystemProperties(() -> { System.setProperty("os.name", "Windows"); + Ansi.jansiConsole = null; withEnvironmentVariable(ANSI_ENVIRONMENT_VARIABLES[0], null) .and(ANSI_ENVIRONMENT_VARIABLES[1], null) .and(ANSI_ENVIRONMENT_VARIABLES[2], null) @@ -688,6 +695,7 @@ public void testAnsiAutoDisabledIfNoTty() throws Exception { restoreSystemProperties(() -> { System.setProperty("os.name", "Windows"); + Ansi.jansiConsole = null; assertTrue(Ansi.isWindows()); assertFalse(Ansi.isPseudoTTY()); assertFalse(Ansi.isJansiConsoleInstalled()); diff --git a/src/test/java/picocli/Issue1125_1538_OptionNameOrSubcommandAsOptionValue.java b/src/test/java/picocli/Issue1125_1538_OptionNameOrSubcommandAsOptionValue.java index 2b01e2f62..bf23bc79d 100644 --- a/src/test/java/picocli/Issue1125_1538_OptionNameOrSubcommandAsOptionValue.java +++ b/src/test/java/picocli/Issue1125_1538_OptionNameOrSubcommandAsOptionValue.java @@ -9,6 +9,7 @@ import org.junit.Test; import org.junit.contrib.java.lang.system.SystemErrRule; import picocli.CommandLine.Command; +import picocli.CommandLine.Help.Ansi; import picocli.CommandLine.IParameterPreprocessor; import picocli.CommandLine.Model.ArgSpec; import picocli.CommandLine.Model.CommandSpec; @@ -111,6 +112,11 @@ public void testAmbiguousOptionsDefault() { //-x -y=123 MyCommand obj = new MyCommand(); CommandLine cmdLine = new CommandLine(obj); + // Clear the globally cached jansiConsole value that might + // have been set in a previous test to force the + // Ansi#isJansiConsoleInstalled method to recalculate + // the cached value. + Ansi.jansiConsole = null; int exitCode = cmdLine.execute("-x", "-y=123"); assertEquals(2, exitCode); String expected = String.format("" +