Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default to old console if user has not set it #44796

Merged
merged 1 commit into from
Nov 29, 2024
Merged

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Nov 28, 2024

This is done because the new JLine console
which is the default from JDK 23 (maybe even 22?)
causes a large regression in startup time due to
it loading a very large number of classes.

If users really want to use the new console,
they have to start the application with

-Djdk.console=jdk.internal.le

Note on testing

I tried to create a proper test for this but it's not easy because we don't have an attached console to check against (if we did, we could open up Java IO using: --add-opens=java.base/java.io=ALL-UNNAMED and do something like:

private String determineConsoleName() {
        Console console = System.console();
        if (console == null) {
            return "null";
        }
        // in JDK 21+, the actual console is hidden behind in a delegate
        if ("java.io.ProxyingConsole".equals(console.getClass().getName())) {
            try {
                Class<?> proxyingClass = Thread.currentThread().getContextClassLoader().loadClass("java.io.ProxyingConsole");
                Field delegateField = proxyingClass.getDeclaredField("delegate");
                delegateField.setAccessible(true);
                Object jdkConsole = delegateField.get(console);
                return jdkConsole.getClass().getName();
            } catch (Exception e) {
                throw new RuntimeException(e);
            }
        }
        return console.getClass().getName();
    }

This is done because the new JLine console
which is the default from JDK 23 (maybe even 22?)
causes a large regression in startup time due to
it loading a very large number of classes.

If users really want to use the new console,
they have to start the application with
`-Djdk.console=jdk.internal.le`

Fixes: quarkusio#44471
Fixes: quarkusio#44653
@gsmet
Copy link
Member

gsmet commented Nov 28, 2024

Is it something that we should consider pushing to 3.15 at some point? Given it's a LTS, I'm pretty sure Java 23 or 24 will be used with this version.

@geoand
Copy link
Contributor Author

geoand commented Nov 28, 2024 via email

Copy link

quarkus-bot bot commented Nov 28, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 88a0dce.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging/deployment

io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector - History

  • Expecting actual: ["-4","-5","-6","-7","-8","-9","-10","-11"] to start with: ["-3", "-4", "-5", "-6"] - java.lang.AssertionError
java.lang.AssertionError: 

Expecting actual:
  ["-4","-5","-6","-7","-8","-9","-10","-11"]
to start with:
  ["-3", "-4", "-5", "-6"]

	at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:36)

⚙️ Native Tests - HTTP

📦 integration-tests/rest-client-reactive

io.quarkus.it.rest.client.BasicTestIT.shouldCreateClientSpans - History

  • expected: <1> but was: <2> - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: expected: <1> but was: <2>
	at io.quarkus.it.rest.client.BasicTest.shouldCreateClientSpans(BasicTest.java:216)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:806)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

@geoand geoand merged commit 9967650 into quarkusio:main Nov 29, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Nov 29, 2024
@quarkus-bot quarkus-bot bot added kind/bugfix kind/enhancement New feature or request labels Nov 29, 2024
@geoand geoand deleted the #44471 branch November 29, 2024 06:49
@gsmet gsmet modified the milestones: 3.18 - main, 3.17.3 Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dev console "crashes" with Java 23 JDK 23 new console is causing a serious regression in startup
3 participants