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

Add dev-mode setting for forcing the use of C2 #43988

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Oct 21, 2024

This setting allows users to opt-in to C2 when it makes sense (for example when running LLM inference via Jlama or Llama3.java)

This comment was marked as resolved.

@quarkus-bot quarkus-bot bot added area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/maven labels Oct 21, 2024
@gsmet
Copy link
Member

gsmet commented Oct 21, 2024

I think we might need to balance what the current default brings vs what it costs in some cases.

If it brings just a tiny bit more speed in standard cases and makes some use cases really really worse, it might make sense to just use C2 all the time.
Suggesting as you might miss this knobs entirely given it's quite advanced.

Now if the current default actually makes things a lot better in most cases then it makes sense to keep it.

@geoand
Copy link
Contributor Author

geoand commented Oct 21, 2024

C2 should not be used by default as it does come with a non-trivial startup penalty.
For example, in the rest-json quickstart on my machine the penalty was more than 300ms for the first start.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks for having a look. I was asking because from time to time, we are micro-optimizing because it doesn't cost us anything. It's worth revisiting when we actually identify a cost.

LGTM, then!

@geoand
Copy link
Contributor Author

geoand commented Oct 21, 2024

Yeah, I'm glad you brought it up as we haven't checked in a long time!

@geoand geoand added triage/waiting-for-ci Ready to merge when CI successfully finishes area/devmode and removed area/gradle Gradle area/core area/maven area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins labels Oct 21, 2024
Copy link

quarkus-bot bot commented Oct 21, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 19e1a7b.

✅ 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/opentelemetry/deployment

io.quarkus.opentelemetry.deployment.metrics.HttpServerMetricsTest.collectsHttpRouteFromEndAttributes - History

  • Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter.assertCountPointsAtLeast(InMemoryMetricExporter.java:131)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter_ClientProxy.assertCountPointsAtLeast(Unknown Source)

⚙️ JVM Tests - JDK 17 Windows

📦 extensions/opentelemetry/deployment

io.quarkus.opentelemetry.deployment.metrics.HttpServerMetricsTest.collectsHttpRouteFromEndAttributes - History

  • Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter.assertCountPointsAtLeast(InMemoryMetricExporter.java:131)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter_ClientProxy.assertCountPointsAtLeast(Unknown Source)
  • Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter.assertCountPointsAtLeast(InMemoryMetricExporter.java:131)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter_ClientProxy.assertCountPointsAtLeast(Unknown Source)

@geoand geoand merged commit de27583 into quarkusio:main Oct 21, 2024
52 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 21, 2024
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Oct 21, 2024
@geoand geoand deleted the forceC2 branch October 21, 2024 17:05
@gsmet gsmet modified the milestones: 3.17 - main, 3.16.0 Oct 21, 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.

3 participants