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

Adds environment variables #212

Merged
merged 4 commits into from Mar 29, 2022
Merged

Adds environment variables #212

merged 4 commits into from Mar 29, 2022

Conversation

bric3
Copy link
Contributor

@bric3 bric3 commented Nov 12, 2021

This proposed change is about the support of environment variables when launching the jmh jar. (Because the org.openjdk.jmh.annotations.Fork don't allow to pass environments).

The jmh block is of type JmhParameters and WithJavaToolchain so in order to support a similar API as JavaExecSpec it has to be explicitly exposed.

Current alternative
The currently API does not allow this so we have to executes the jmh jar outside gradle.

$ ./gradlew :jmh-panama:jmhJar
$ env JAVA_LIBRARY_PATH=$(grealpath jmh-panama/jni) \
  java -jar jmh-panama/build/libs/jmh-panama-jmh.jar \
  -jvmArgs '-Xms256m -Xmx256m -Djmh.separateClasspathJAR=true --add-modules=jdk.incubator.foreign --enable-native-access=ALL-UNNAMED'

Note
I didn't see a functional test with jmh options so I'm not sure how you'd want to test that.

@bric3 bric3 mentioned this pull request Nov 12, 2021
@bric3
Copy link
Contributor Author

bric3 commented Nov 12, 2021

I'd like something closer to the JavaExecSpec, but the jmh block is not extending it.

environment(mapOf("JAVA_HOME" to System.getenv("JAVA_HOME")))
environment("JAVA_HOME", System.getenv("JAVA_HOME"))

And I am not sure how best to proceed. Same for the tests.

Sometimes it is necessary to pass some environment variables to configure
the JVM process (in particular when native libraries are involved). And
when there's no system property equivalent, or when it's not possible to do in the benchmark setup (e.g. `LD_PRELOAD`, `JAVA_LIBRARY_PATH`).

Alas, JMH does not support to set environment variable on the forked
process, however it will propagate the ones that it has been launched with.
E.g.

```
env JAVA_LIBRARY_PATH=/somewhere java -jar bench.jar
```

This change aims at providing the ability to pass the variable to JMH so it cna be used on the JMH forked process.
@bric3
Copy link
Contributor Author

bric3 commented Mar 28, 2022

Wow I wonder how the PR description was empty, was it me that forgot 🤔🤦 ?
I just rebased this pr on the latest from master.

Copy link
Owner

@melix melix left a comment

Choose a reason for hiding this comment

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

Would be great to have a test of some kind. Could be a benchmark which spits out an environment variable via System.out and that the test would search for in the output.

src/main/java/me/champeau/jmh/JMHTask.java Outdated Show resolved Hide resolved
Drop the parameter conversion indirection
@bric3
Copy link
Contributor Author

bric3 commented Mar 29, 2022

Would be great to have a test of some kind. Could be a benchmark which spits out an environment variable via System.out and that the test would search for in the output.

Done

Copy link
Owner

@melix melix left a comment

Choose a reason for hiding this comment

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

Great, thank you!

@melix melix merged commit 60479e7 into melix:master Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants