Disable single parsing of compiler graphs by default #21018

zakkak · 2021-10-26T18:29:55Z

Disable single parsing of compiler graphs till the impact of it on heap usage decreases see oracle/graal#3435 and graalvm/mandrel#304 (comment) for more details

#19511 didn't take into account runs with constraint memory and it also measured the max committed heap size instead of the max heap usage. Bringing back -H:-ParseOnce reduces the heap usage significantly, from 3.9G down to 2.7G, in integration-tests/main.

zakkak · 2021-10-26T20:03:38Z

The "Native Tests - Messaging2" issue appears to be unrelated to this PR (and transient). See https://github.com/quarkusio/quarkus/runs/4008277751?check_suite_focus=true for a failure on main.

quarkus-bot · 2021-10-26T23:10:35Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `514fac4`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 17	`Build`	Failures	Logs	Raw logs
✖	Native Tests - Messaging2	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: integration-tests/reactive-messaging-rabbitmq

📦 integration-tests/reactive-messaging-rabbitmq

✖ io.quarkus.it.rabbitmq.RabbitMQConnectorTest.test line 24 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a io.quarkus.it.rabbitmq.RabbitMQConnectorTest expected: <6> but was: <0> within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)

⚙️ Native Tests - Messaging2 #

- Failing: integration-tests/reactive-messaging-rabbitmq

📦 integration-tests/reactive-messaging-rabbitmq

✖ io.quarkus.it.rabbitmq.RabbitMQConnectorIT.test - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a io.quarkus.it.rabbitmq.RabbitMQConnectorTest expected: <6> but was: <0> within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)

zakkak · 2021-10-27T05:06:10Z

For completeness I am pasting here some measurements from compilations of integration-tests/main graalvm/mandrel#304 (comment)

With single parsing enabled

logs/mandrel-21.2.0.2-Final-main-1:[INFO] Total time:  18:14 min
logs/mandrel-21.2.0.2-Final-main-2:[INFO] Total time:  17:48 min
logs/mandrel-21.2.0.2-Final-main-3:[INFO] Total time:  18:31 min
logs/mandrel-21.2.0.2-Final-main-4:[INFO] Total time:  17:58 min
logs/mandrel-21.2.0.2-Final-main-5:[INFO] Total time:  18:10 min

logs/mandrel-21.3.0.0-Final-main-1.txt:[INFO] Total time:  18:22 min
logs/mandrel-21.3.0.0-Final-main-2.txt:[INFO] Total time:  17:39 min
logs/mandrel-21.3.0.0-Final-main-3.txt:[INFO] Total time:  29:56 min
logs/mandrel-21.3.0.0-Final-main-4.txt:[INFO] Total time:  19:35 min
logs/mandrel-21.3.0.0-Final-main-5.txt:[INFO] Total time:  33:06 min

With single parsing disabled

logs/mandrel-21.2.0.2-Final-main-noParseOnce-1.txt:[INFO] Total time:  09:59 min
logs/mandrel-21.2.0.2-Final-main-noParseOnce-2.txt:[INFO] Total time:  04:45 min
logs/mandrel-21.2.0.2-Final-main-noParseOnce-3.txt:[INFO] Total time:  04:45 min
logs/mandrel-21.2.0.2-Final-main-noParseOnce-4.txt:[INFO] Total time:  04:48 min
logs/mandrel-21.2.0.2-Final-main-noParseOnce-5.txt:[INFO] Total time:  04:45 min

logs/mandrel-21.3.0.0-Final-main-noParseOnce-1.txt:[INFO] Total time:  05:54 min
logs/mandrel-21.3.0.0-Final-main-noParseOnce-2.txt:[INFO] Total time:  05:42 min
logs/mandrel-21.3.0.0-Final-main-noParseOnce-3.txt:[INFO] Total time:  05:36 min
logs/mandrel-21.3.0.0-Final-main-noParseOnce-4.txt:[INFO] Total time:  05:38 min
logs/mandrel-21.3.0.0-Final-main-noParseOnce-5.txt:[INFO] Total time:  05:28 min

galderz

Before integrating this I wanted to get some further clarification:

I'm happy with making -H:-ParseOnce default, but I'm not totally sure about the need for a higher level configuration option. The native image build step could be improved to deal with -Dquarkus.native.additional-build-args=-H:+ParseOnce, detect the change of option and apply the one passed in? What's the exact reason behind adding a top level option for this?

If we want to keep the high level option in place, I wonder if the javadoc should mention that the increase is most noticeable in constrained environments. As noted, unconstrained environments do not seem to suffer from same issues.

zakkak · 2021-10-27T07:11:02Z

I'm happy with making -H:-ParseOnce default, but I'm not totally sure about the need for a higher level configuration option. The native image build step could be improved to deal with -Dquarkus.native.additional-build-args=-H:+ParseOnce, detect the change of option and apply the one passed in? What's the exact reason behind adding a top level option for this?

The reason I added the option is 1) for us to be able to keep testing this, 2) for advanced users to benefit from the faster build times.

Ideally I would like it to be marked as "experimental", but AFAIK there is no such option in Quarkus.
Checking for -Dquarkus.native.additional-build-args=-H:+ParseOnce is indeed an alternative, but I am afraid it might get a bit messy if we start doing such checks.

Sanne · 2021-10-27T10:21:08Z

Since we learned the hard way that order of such properties matter... would an advanced user be able to experiment with it by having -Dquarkus.native.additional-build-args=-H:+ParseOnce appended, even though Quarkus always appends -H:-ParseOnce ?

That way we could avoid the configuration property, which is always nice.

zakkak · 2021-10-27T11:14:45Z

Since we learned the hard way that order of such properties matter... would an advanced user be able to experiment with it by having -Dquarkus.native.additional-build-args=-H:+ParseOnce appended, even though Quarkus always appends -H:-ParseOnce ?

That would be great, but Quarkus prepends -Dquarkus.native.additional-build-args and doing so would result in an invocation like native-image ... -H:+ParseOnce ... -H:-ParseOnce ... which is the same as just passing -H:-ParseOnce (the last option wins), so it wouldn't work. For this to work we need to ensure that
quarkus.native.additional-build-args come after the default -H:-ParseOnce but since that will be an exception (for all other arguments it comes before the additional args) it might be confusing.

If we really don't want the configuration option, I am OK with parsing quarkus.native.additional-build-args to see if it affects ParseOnce, which seems the next safest option to me.

Sanne · 2021-10-27T12:37:33Z

That would be great, but Quarkus prepends [...]

ah I see. I wonder if we could put it last instead? To really be "advanced" I believe we should allow it to override other flags. Would that work?

If we really don't want the configuration option

I'm not strongly against it; just thinking that it would be nicer if the above suggestion could work - but sure if that gets complicated ignore me :)

zakkak · 2021-10-27T13:11:00Z

I placed -H:-ParseOnce before the handling of additional args, so that it can be overridden. Note that this is the only option placed there. I made sure to add a comment explaining way, let's hope that this want cause any confusion.

@galderz @Sanne please have a look once more :)

Sanne

Nice! thanks

galderz · 2021-10-27T15:16:24Z

Looks good, thx @zakkak!

quarkus-bot · 2021-10-27T17:32:11Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `0574676`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 17	`Build`	Failures	Logs	Raw logs
✖	Native Tests - Messaging2	`Build`	Failures	Logs	Raw logs
✖	Native Tests - Windows - hibernate-validator	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: integration-tests/container-image/maven-invoker-way

📦 integration-tests/container-image/maven-invoker-way

✖ Failed to execute goal org.apache.maven.plugins:maven-invoker-plugin:3.2.2:run (integration-tests) on project quarkus-integration-test-container-image-invoker: 1 build failed. See console output above for details.

⚙️ Native Tests - Messaging2 #

- Failing: integration-tests/reactive-messaging-rabbitmq

📦 integration-tests/reactive-messaging-rabbitmq

✖ io.quarkus.it.rabbitmq.RabbitMQConnectorIT.test - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a io.quarkus.it.rabbitmq.RabbitMQConnectorTest expected: <6> but was: <0> within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)

⚙️ Native Tests - Windows - hibernate-validator #

- Failing: integration-tests/hibernate-validator

📦 integration-tests/hibernate-validator

✖ Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project quarkus-integration-test-hibernate-validator: Failed to build quarkus application

zakkak · 2021-10-27T21:13:23Z

Hmmm, the windows failure is interesting!

LINK : fatal error LNK1102: out of memory

Has anyone seen this before?
I am rerunning to see whether it's persistent or not.

zakkak · 2021-10-28T11:56:07Z

I was not able to reproduce the issue in https://github.com/graalvm/mandrel/actions/runs/1391994314
Can someone please restart the CI for this PR?

Disable single parsing of compiler graphs till the impact of it on heap usage decreases see oracle/graal#3435 and graalvm/mandrel#304 (comment) quarkusio#19511 didn't take into account runs with constraint memory. Bringing back `-H:-ParseOnce` reduces the heap usage significantly, from 3.9G down to 2.7G in integration-tests/main.

quarkus-bot · 2021-10-28T22:11:20Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `201b9a6`

Status	Name	Step	Failures	Logs	Raw logs
✖	Gradle Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✖	Native Tests - Windows - hibernate-validator	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Gradle Tests - JDK 11 Windows #

- Failing: integration-tests/gradle

📦 integration-tests/gradle

✖ io.quarkus.gradle.devmode.BasicKotlinApplicationModuleDevModeTest.main line 18 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

⚙️ Native Tests - Windows - hibernate-validator #

- Failing: integration-tests/hibernate-validator

📦 integration-tests/hibernate-validator

✖ Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project quarkus-integration-test-hibernate-validator: Failed to build quarkus application

zakkak requested a review from Sanne October 26, 2021 18:30

quarkus-bot bot added the area/core label Oct 26, 2021

zakkak requested a review from galderz October 26, 2021 18:30

This was referenced Oct 26, 2021

Re-enable single parsing of compiler graphs in native-image #19511

Merged

Update to GraalVM 21.3 #20894

Merged

gsmet approved these changes Oct 26, 2021

View reviewed changes

gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 26, 2021

Sanne approved these changes Oct 26, 2021

View reviewed changes

galderz reviewed Oct 27, 2021

View reviewed changes

zakkak force-pushed the disable-parseonce branch from 514fac4 to 0574676 Compare October 27, 2021 13:09

zakkak requested review from galderz and Sanne October 27, 2021 13:11

Sanne approved these changes Oct 27, 2021

View reviewed changes

galderz approved these changes Oct 27, 2021

View reviewed changes

zakkak force-pushed the disable-parseonce branch from 0574676 to 201b9a6 Compare October 28, 2021 18:03

gsmet merged commit efaff14 into quarkusio:main Oct 29, 2021

quarkus-bot bot added this to the 2.5 - main milestone Oct 29, 2021

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 29, 2021

zakkak deleted the disable-parseonce branch October 29, 2021 18:16

This was referenced Nov 4, 2021

Previously passing configuration times out on GH actions graalvm/mandrel#304

Closed

Increased compilation times with GraalVM 21.1.0 #18360

Closed

gsmet added the triage/backport-2.2? label Dec 1, 2021

gsmet modified the milestones: 2.5.0.CR1, 2.2.4.Final Dec 1, 2021

gsmet removed the triage/backport-2.2? label Dec 1, 2021

This was referenced Feb 22, 2024

Native: Introduce flag to favor build time improvements at the cost of using more resources and vice versa #38968

Open

[GR-52400] Native: Introduce flag to favor build time improvements at the cost of using more resources and vice versa oracle/graal#8468

Closed

Disable single parsing of compiler graphs by default #21018

Disable single parsing of compiler graphs by default #21018

Conversation

zakkak commented Oct 26, 2021 • edited Loading

zakkak commented Oct 26, 2021

quarkus-bot bot commented Oct 26, 2021 • edited Loading

Failing Jobs - Building 514fac4

Failures

⚙️ JVM Tests - JDK 17 #

📦 integration-tests/reactive-messaging-rabbitmq

⚙️ Native Tests - Messaging2 #

📦 integration-tests/reactive-messaging-rabbitmq

zakkak commented Oct 27, 2021 • edited Loading

With single parsing enabled

With single parsing disabled

galderz left a comment

Choose a reason for hiding this comment

zakkak commented Oct 27, 2021

Sanne commented Oct 27, 2021

zakkak commented Oct 27, 2021

Sanne commented Oct 27, 2021

zakkak commented Oct 27, 2021

Sanne left a comment

Choose a reason for hiding this comment

galderz commented Oct 27, 2021

quarkus-bot bot commented Oct 27, 2021 • edited Loading

Failing Jobs - Building 0574676

Failures

⚙️ JVM Tests - JDK 17 #

📦 integration-tests/container-image/maven-invoker-way

⚙️ Native Tests - Messaging2 #

📦 integration-tests/reactive-messaging-rabbitmq

⚙️ Native Tests - Windows - hibernate-validator #

📦 integration-tests/hibernate-validator

zakkak commented Oct 27, 2021

zakkak commented Oct 28, 2021

quarkus-bot bot commented Oct 28, 2021 • edited Loading

Failing Jobs - Building 201b9a6

Failures

⚙️ Gradle Tests - JDK 11 Windows #

📦 integration-tests/gradle

⚙️ Native Tests - Windows - hibernate-validator #

📦 integration-tests/hibernate-validator

zakkak commented Oct 26, 2021 •

edited

Loading

quarkus-bot bot commented Oct 26, 2021 •

edited

Loading

Failing Jobs - Building `514fac4`

zakkak commented Oct 27, 2021 •

edited

Loading

quarkus-bot bot commented Oct 27, 2021 •

edited

Loading

Failing Jobs - Building `0574676`

quarkus-bot bot commented Oct 28, 2021 •

edited

Loading

Failing Jobs - Building `201b9a6`