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

Ability to configure extension Dev mode JVM options #44305

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

aloubyansky
Copy link
Member

@aloubyansky aloubyansky commented Nov 4, 2024

This change allows extension authors to configure JVM options in extension metadata that would be added to the command line launching an application in dev mode.

At this point, this is targeting JVM options that start with --, such as --enable-preview, --add-opens, --add-modules and boolean or single valued -XX: options.

Dev mode JVM options can be configured when building extensions in the following way

            <plugin>
                <groupId>io.quarkus</groupId>
                <artifactId>quarkus-extension-maven-plugin</artifactId>
                <version>${quarkus.version}</version>
                <executions>
                    <execution>
                        <phase>compile</phase>
                        <goals>
                            <goal>extension-descriptor</goal>
                        </goals>
                        <configuration>
                            <deployment>${project.groupId}:${project.artifactId}-deployment:${project.version}</deployment>
                            <devMode>
                                <!-- standard JVM options -->
                                <jvmOptions>
                                    <add-modules>jdk.incubator.vector</add-modules>
                                    <add-opens>java.base/java.util=ALL-UNNAMED</add-opens>
                                    <add-opens>java.base/java.io=ALL-UNNAMED</add-opens>
                                    <add-opens>java.base/java.nio=ALL-UNNAMED</add-opens>
                                    <enable-preview/>
                                </jvmOptions>
                                <!-- -XX: options -->
                                <xxJvmOptions>
                                    <UseThreadPriorities>false</UseThreadPriorities>
                                    <UseLargePages>true</UseLargePages>
                                    <AllocatePrefetchStyle>1</AllocatePrefetchStyle>
                                </xxJvmOptions>
                            </devMode>
...

When an ApplicationModel is resolved, Dev mode JVM options from all the extensions are merged and when the application is launched in Dev mode, these options are added to the command line.

A user can disable JVM options coming extensions by adding the following quarkus-maven-plugin configuration

            <plugin>
                <groupId>io.quarkus</groupId>
                <artifactId>quarkus-maven-plugin</artifactId>
                <version>${quarkus.version}</version>
                <configuration>
                    <extensionJvmOptions>
                        <disableAll>true</disableAll>
                    </extensionJvmOptions>
                </configuration>
...

or disable JVM arguments from extensions matching an artifact pattern

            <plugin>
                <groupId>io.quarkus</groupId>
                <artifactId>quarkus-maven-plugin</artifactId>
                <version>${quarkus.version}</version>
                <configuration>
                    <extensionJvmOptions>
                        <disableFor>
                            <!-- disable JVM options from all the extensions with groupId org.acme -->
                            <extension>org.acme</extension>
                            <!-- disable JVM options configured in io.quarkiverse:quarkus-magic -->
                            <extension>org.quarkiverse:quarkus-magic</extension>
                        </disableFor>
                    </extensionJvmOptions>
                </configuration>
...

Here is a playground repository I use to test this https://github.com/aloubyansky/playground/tree/ext-dev-jvm-args

https://github.com/aloubyansky/playground/blob/ext-dev-jvm-args/quarkus-blue/runtime/pom.xml and
https://github.com/aloubyansky/playground/blob/ext-dev-jvm-args/quarkus-red/runtime/pom.xml
configure dev mode JVM options.

Application https://github.com/aloubyansky/playground/blob/ext-dev-jvm-args/app/pom.xml depends on both of the extensions and can filter out some or all the options from the extensions it depends on.

The project can be launched with mvn clean compile quarkus:dev -X from the root directory to see the final command line launching the application logged.

It looks like

[DEBUG] Launching JVM with command line: /home/aloubyansky/jdk/jdk-17.0.10/bin/java --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED -XX:AllocatePrefetchStyle=1 --add-modules=java.management,jdk.incubator.vector -XX:+UseLargePages --enable-preview -XX:-UseThreadPriorities -Dquarkus-internal.serialized-app-model.path=/home/aloubyansky/git/playground/app/target/quarkus/bootstrap/dev-app-model.dat -javaagent:/home/aloubyansky/.m2/repository/io/quarkus/quarkus-class-change-agent/999-SNAPSHOT/quarkus-class-change-agent-999-SNAPSHOT.jar -XX:TieredStopAtLevel=1 -agentlib:jdwp=transport=dt_socket,address=localhost:5005,server=y,suspend=n -Djava.util.logging.manager=org.jboss.logmanager.LogManager -jar /home/aloubyansky/git/playground/app/target/acme-app-dev.jar

@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/maven labels Nov 4, 2024
@geoand
Copy link
Contributor

geoand commented Nov 5, 2024

This is awesome!

I assume that to test it, I should update one of the extensions I have in mind with the <extensionJvmArgs> section?

@maxandersen
Copy link
Member

looks good but can you clarify whats the options/semantics if/when user want to configure these?

do they override them all or can they append/prepend? (order matters for some of these afaik and also behavior of runtime will differ greatly between dev and prod - not saying thats bad but should be documented/make users aware)

Also, this is purely for devmode so for prod users are on their own figuring out what to add? anything we could do there to help?

@geoand
Copy link
Contributor

geoand commented Nov 5, 2024

Also, this is purely for devmode so for prod users are on their own figuring out what to add? anything we could do there to help?

My hope is that once we figure this one out, we can do something similar for prod-mode

@maxandersen
Copy link
Member

just to give a usecase i'm concerned about - two extensions adding conflicting arguments; how can/will user resolve that?

@geoand
Copy link
Contributor

geoand commented Nov 5, 2024

@aloubyansky how would the

-enable-native-access=ALL-UNNAMED

JVM flag be enabled?

@aloubyansky
Copy link
Member Author

looks good but can you clarify whats the options/semantics if/when user want to configure these?

do they override them all or can they append/prepend?

For now everything is merged. It's pretty basic at this point. A user can disable arguments from all or specific extensions and configure their own on top of those.
Exposing jvmArgs as a string parameter in DevMojo now doesn't look like the best idea. We should probably refactor it to have a similar break down as for extensions in this PR.
There is the modules parameter though in DevMojo which fits well in here.

(order matters for some of these afaik and also behavior of runtime will differ greatly between dev and prod - not saying thats bad but should be documented/make users aware)

Could you give an example where order matters? For now I actually order everything alphabetically, which may be a bad idea but it results in a stable command line.

Also, this is purely for devmode so for prod users are on their own figuring out what to add? anything we could do there to help?

This is still being explored even for dev mode.

@aloubyansky
Copy link
Member Author

just to give a usecase i'm concerned about - two extensions adding conflicting arguments; how can/will user resolve that?

I think the validation and conflict resolution will have to be implemented per argument type, they are quite different. We can point out which arguments from which extensions conflict and let the user resolve it. We can't guarantee a conflict free solution, it may happen as well as with dependencies, capabilities, etc.

@aloubyansky
Copy link
Member Author

Here is my playground project with a couple of extensions and an app https://github.com/aloubyansky/playground/tree/ext-dev-jvm-args

Here is how one configures its arguments https://github.com/aloubyansky/playground/blob/ext-dev-jvm-args/quarkus-red/runtime/pom.xml#L34-L42
and here is the other one https://github.com/aloubyansky/playground/blob/ext-dev-jvm-args/quarkus-blue/runtime/pom.xml#L34-L39

And here is the app depending on both of them https://github.com/aloubyansky/playground/blob/ext-dev-jvm-args/app/pom.xml#L59-L66 also adding an extra module https://github.com/aloubyansky/playground/blob/ext-dev-jvm-args/app/pom.xml#L57

To test it the project could be launched from the root with mvn clean compile quarkus:dev
Adding -X to the mvn command will log the actual command launching the app.

@aloubyansky
Copy link
Member Author

@aloubyansky how would the

-enable-native-access=ALL-UNNAMED

JVM flag be enabled?

It should be --enable-native-access, right?

            <plugin>
                <groupId>io.quarkus</groupId>
                <artifactId>quarkus-extension-maven-plugin</artifactId>
                <version>${quarkus.version}</version>
                <executions>
                    <execution>
                        <phase>compile</phase>
                        <goals>
                            <goal>extension-descriptor</goal>
                        </goals>
                        <configuration>
                            <deployment>${project.groupId}:${project.artifactId}-deployment:${project.version}</deployment>
                            <devMode>
                                <jvmArgs>
                                    <enable-native-access>ALL-UNNAMED</enable-native-access>
                                </jvmArgs>
                            </devMode>
...

@aloubyansky
Copy link
Member Author

If I add it that way to quarkus-blue in my playground project, the command will look like

[DEBUG] Launching JVM with command line: /home/aloubyansky/jdk/jdk-17.0.10/bin/java --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --enable-native-access=ALL-UNNAMED --add-modules=java.management,jdk.incubator.vector --enable-preview -Dquarkus-internal.serialized-app-model.path=/home/aloubyansky/git/playground/app/target/quarkus/bootstrap/dev-app-model.dat -javaagent:/home/aloubyansky/.m2/repository/io/quarkus/quarkus-class-change-agent/999-SNAPSHOT/quarkus-class-change-agent-999-SNAPSHOT.jar -XX:TieredStopAtLevel=1 -agentlib:jdwp=transport=dt_socket,address=localhost:5005,server=y,suspend=n -Djava.util.logging.manager=org.jboss.logmanager.LogManager -jar /home/aloubyansky/git/playground/app/target/acme-app-dev.jar

@geoand
Copy link
Contributor

geoand commented Nov 5, 2024

I'll check it out, thanks

@maxandersen
Copy link
Member

A user can disable arguments from all or specific extensions and configure their own on top of those.

how is that done by the user?

@aloubyansky
Copy link
Member Author

A user can disable arguments from all or specific extensions and configure their own on top of those.

how is that done by the user?

See the description (the last XML snippet)

@maxandersen
Copy link
Member

Could you give an example where order matters? For now I actually order everything alphabetically, which may be a bad idea but it results in a stable command line.

example of order matters that I know of top of my head:

-Dkey=value -Dkey=value2 here value2 wins. flip it around its value.

add-modules, defines module path order and thus like dependencies have priority/influence on classloading resolution.

I'll try find where I read that the javac/java command line tools in general allows duplicates of arguments but last one wins.

@aloubyansky
Copy link
Member Author

I am not covering -D here currently. I was thinking system properties could have a dedicated config block. There already is systemProperties parameter in DevMojo. We could add a similar config to extensions.

In the current impl, values of arguments with the same name are merged, meaning there wouldn't be the same argument name/key appearing more than once on the command line. But again, -D is not among those at this point.

@maxandersen
Copy link
Member

rather than sorting alphabetically why not sort by dependency order? (and if java honors first/last maybe add it in reverse order)

@maxandersen
Copy link
Member

actually just realizing that if we just expose explict options and from that generates one argument, i.e.

ALL-UNNAMED
some-other-module

no matter where it comes from results in:

--enable-native-access=ALL-UNNAMED,some-other-module

things are less "chaotic" and if put first in command line give user somewhat freedom to at least append.

Would it make sense to have these generated into a common location (i.e. /target/quarkus/devmodeflags) and then refer to it using@target/quarkus/devmodeflags and also avoid hitting command line limits on windows?

@maxandersen
Copy link
Member

maxandersen commented Nov 5, 2024

I am not covering -D here currently.

-D was just example, add-modules is in here.

But basically I think it is last one wins, though things like --add-modules are additive.

@aloubyansky
Copy link
Member Author

Would it make sense to have these generated into a common location (i.e. /target/quarkus/devmodeflags) and then refer to it using@target/quarkus/devmodeflags and also avoid hitting command line limits on windows?

Sure, could you point me to a doc or some other impl using this approach?

@aloubyansky
Copy link
Member Author

rather than sorting alphabetically why not sort by dependency order? (and if java honors first/last maybe add it in reverse order)

We could, although in the new resolver impl the dependencies (including extension metadata) are processed in parallel.

@maxandersen
Copy link
Member

We could, although in the new resolver impl the dependencies (including extension metadata) are processed in parallel.

with the option of disabling and everything gets grouped its probably fine to use alpha ordering of the values.

@maxandersen
Copy link
Member

See the description (the last XML snippet)

will this disable it for everything?

<disableFor>
             <extension/>
</disableFor>

@aloubyansky
Copy link
Member Author

I haven't tested that but this (the previous snippet in the description) will

                    <extensionJvmArgs>
                        <disableAll>true</disableAll>
                    </extensionJvmArgs>

@geoand
Copy link
Contributor

geoand commented Nov 5, 2024

@aloubyansky is there a way that the Maven / Gradle option added in #43988 could be "enabled"?
Essentially what the options is to prevent -XX:TieredStopAtLevel=1 from being added to the JVM args

@aloubyansky
Copy link
Member Author

@aloubyansky is there a way that the Maven / Gradle option added in #43988 could be "enabled"? Essentially what the options is to prevent -XX:TieredStopAtLevel=1 from being added to the JVM args

From an extension? That's a tricky one. Either we create an alias for it or add (more or less) proper support for configuring (some of the) -XX options. I could look into this.

@geoand
Copy link
Contributor

geoand commented Nov 5, 2024

That would be very nice, thanks!

The idea is that if the extension is present, we need C2 otherwise dev-mode is unusable (as in, execution takes minutes instead of seconds).

@aloubyansky
Copy link
Member Author

Here is what I see with that config

[DEBUG] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] (main) Adding JVM options from org.acme:quarkus-blue::jar
[DEBUG] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] (main)   enable-native-access: [ALL-UNNAMED]
[DEBUG] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] (main)   add-modules: [jdk.incubator.vector]
[DEBUG] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] (main)   enable-preview: []
[DEBUG] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] (main) org.acme:quarkus-blue::jar locks JVM options [agentlib:jdwp]
[DEBUG] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] (main) org.acme:quarkus-blue::jar locks JVM options [TieredStopAtLevel]
[DEBUG] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] (main) Executable jar: /home/aloubyansky/git/playground/app/target/acme-app-dev.jar
[DEBUG] Launching JVM with command line: /home/aloubyansky/jdk/jdk-17.0.12/bin/java -Dquarkus-internal.serialized-app-model.path=/home/aloubyansky/git/playground/app/target/quarkus/bootstrap/dev-app-model.dat -javaagent:/home/aloubyansky/.m2/repository/io/quarkus/quarkus-class-change-agent/999-SNAPSHOT/quarkus-class-change-agent-999-SNAPSHOT.jar --enable-native-access=ALL-UNNAMED --add-modules=jdk.incubator.vector --enable-preview -Djava.util.logging.manager=org.jboss.logmanager.LogManager -jar /home/aloubyansky/git/playground/app/target/acme-app-dev.jar
WARNING: Using incubator modules: jdk.incubator.vector

@aloubyansky
Copy link
Member Author

Ah, it still adds -javaagent:path

@aloubyansky
Copy link
Member Author

@geoand is adding the instrumentation agent an issue in your case?

@geoand
Copy link
Contributor

geoand commented Nov 12, 2024

Nope, it doesn't affect execution

@aloubyansky
Copy link
Member Author

How about the following?

[WARNING] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] Extension org.acme:quarkus-blue enables the C2 compiler which is disabled by default in Dev mode for optimal performance.
[WARNING] [io.quarkus.deployment.dev.DevModeCommandLineBuilder] Extension org.acme:quarkus-blue disables the debug mode for optimal performance. Debugging can still be enabled in the Quarkus plugin configuration or with -Ddebug on the command line.

These would be logged only in case an extension's config is effective. Re-running with -Ddebug, for example, will remove the warning.

@geoand
Copy link
Contributor

geoand commented Nov 13, 2024

I like the messages, but maybe it should be INFO instead of WARNING?

@aloubyansky
Copy link
Member Author

Ok, I don't mind. I'm thinking of changing

                                <lockDefaultJvmOptions>agentlib:jdwp</lockDefaultJvmOptions>
                                <lockDefaultXxJvmOptions>TieredStopAtLevel</lockDefaultXxJvmOptions>

to simply

                                <lockJvmOptions>agentlib:jdwp</lockJvmOptions>
                                <lockXxJvmOptions>TieredStopAtLevel</lockXxJvmOptions>

Otherwise, I think it's in a good shape, unless you have further remarks @geoand.

I still need to implement extension option filtering in the Gradle plugin and document it though.

@geoand
Copy link
Contributor

geoand commented Nov 13, 2024

Thanks a lot @aloubyansky!

I will try it soon and let you know if all is good on my side.

@geoand
Copy link
Contributor

geoand commented Nov 13, 2024

Tried it and it works like a charm! So from my pov, this is ready to go functionally.

@aloubyansky
Copy link
Member Author

Gradle support and the basic docs added.

Copy link

github-actions bot commented Nov 14, 2024

🙈 The PR is closed and the preview is expired.

Copy link

quarkus-bot bot commented Nov 14, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 75ebde9.

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

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Nov 15, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 75ebde9.

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

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Awesome stuff!

@aloubyansky
Copy link
Member Author

Unless there are objections, I'd prefer merging this one, since I have another PR touching some of the same classes, that will result in conflicts.

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

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

each extension can express things they want to set and if something goes against something we traditionally hardwired in devmode a info message is logged and users can alwas expliclity set it in their configuration. If conflicts there is a hard fail and user will need to adjust.

Generally +1

I'm not fully sure we shouldn't print WARN for things that goes counter to devmode defaults - like Debugging not being enabled but lets see; also annoying having warnings for something that is expected result from something like jlama needs...

@aloubyansky
Copy link
Member Author

I'm not fully sure we shouldn't print WARN for things that goes counter to devmode defaults - like Debugging not being enabled but lets see; also annoying having warnings for something that is expected result from something like jlama needs...

I changed those to INFO after @geoand's feedback.

@aloubyansky aloubyansky merged commit 65b55fd into quarkusio:main Nov 15, 2024
69 of 70 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Nov 15, 2024
@geoand
Copy link
Contributor

geoand commented Nov 15, 2024

I added the release/noteworthy-feature because of it's impact on how the user experience for various extensions can be improved in the future

@aloubyansky
Copy link
Member Author

I really hope it won't get popular though :D

@geoand
Copy link
Contributor

geoand commented Nov 15, 2024

😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/gradle Gradle area/maven release/noteworthy-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants