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

Disable C2 for GraalVM as well in dev mode by default #45483

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

aloubyansky
Copy link
Member

I tested it with GraalVM 17 and 21.

@aloubyansky aloubyansky requested a review from geoand January 9, 2025 16:21
@quarkus-bot quarkus-bot bot added the area/core label Jan 9, 2025
@gsmet gsmet requested a review from zakkak January 9, 2025 18:29
@gsmet
Copy link
Member

gsmet commented Jan 9, 2025

Let's wait for @zakkak 's feedback.

Copy link

quarkus-bot bot commented Jan 9, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 539929a.

✅ 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 Windows

📦 integration-tests/opentelemetry-quickstart

io.quarkus.it.opentelemetry.OpenTelemetryDisabledTest.buildTimeDisabled - History

  • Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryDisabledTest was not fulfilled within 200 milliseconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryDisabledTest was not fulfilled within 200 milliseconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.it.opentelemetry.OpenTelemetryDisabledTest.buildTimeDisabled(OpenTelemetryDisabledTest.java:29)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

⚙️ Maven Tests - JDK 17

📦 integration-tests/devmode

io.quarkus.test.devui.DevUIGrpcSmokeTest.testTestService - History

  • Too many recursions, message not returned for id [509968663] - java.lang.RuntimeException
java.lang.RuntimeException: Too many recursions, message not returned for id [509968663]
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:164)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)

@zakkak
Copy link
Contributor

zakkak commented Jan 10, 2025

I don't see anything wrong with this. @aloubyansky what prevented you from setting this option with graalvm in the first place?

FWIW when using GraalVM (but not the Mandrel build) C2 is replaced by the Graal JIT compiler, so you are essentially disabling that instead of C2.

cc @galderz

@aloubyansky
Copy link
Member Author

I think the initial logic was implemented by @geoand, there must have been a reason. This category of options is not guaranteed to be working across versions and vendors, so it's brittle by definition.

@geoand
Copy link
Contributor

geoand commented Jan 10, 2025

Back in the day you couldn't turn off C2 in GraalVM - or at least that's what I seen empirically

@geoand geoand merged commit 6742705 into quarkusio:main Jan 10, 2025
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Jan 10, 2025
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.

4 participants