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 -Xverify:none to docs for OpenJ9 #50

Merged
merged 4 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@ Without any options, this tool silently records file open/close operations and u

[source,sh]
----
$ java -javaagent:path/to/file-leak-detector.jar ...your usual Java arguments follow...
$ java -javaagent:path/to/file-leak-detector.jar -Xverify:none ...your usual Java arguments follow...
----

There are several options you can pass to the agent.
For example, to dump the open file descriptors when the total number reaches 200, you can do the following:

[source,sh]
----
$ java -javaagent:path/to/file-leak-detector.jar=threshold=200 ...your usual Java arguments follow...
$ java -javaagent:path/to/file-leak-detector.jar=threshold=200 -Xverify:none ...your usual Java arguments follow...
----

Or to have it run a mini HTTP server so that you can access the information from your browser, do the following and open http://localhost:19999/:

[source,sh]
----
$ java -javaagent:path/to/file-leak-detector.jar=http=19999 ...your usual Java arguments follow...
$ java -javaagent:path/to/file-leak-detector.jar=http=19999 -Xverify:none ...your usual Java arguments follow...
----

Use the help option to see the help screen for the complete list of options:
Expand All @@ -57,7 +57,7 @@ Options can be specified in the second argument in the same format you do to the

[source,sh]
----
$ java -jar path/to/file-leak-detector.jar 1500 threshold=200,strong
$ java -jar path/to/file-leak-detector.jar 1500 threshold=200,strong -Xverify:none
----

== Supported options
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
<exclude>**/TransformerTest.java</exclude>
<exclude>**/AgentMainTest.java</exclude>
</excludes>
<argLine>-javaagent:"${project.build.directory}/file-leak-detector-${project.version}-jar-with-dependencies.jar"</argLine>
<argLine>-javaagent:"${project.build.directory}/file-leak-detector-${project.version}-jar-with-dependencies.jar" -Xverify:none</argLine>
Copy link
Member

Choose a reason for hiding this comment

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

This affects our test suite on HotSpot by decreasing verification. I think this PR should not affect HotSpot. The fact that this was working in our tests on HotSpot without -Xverify:none is evidence that this is only needed for OpenJ9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright, sounds like this pr isn't needed. abandoning.

Copy link
Member

Choose a reason for hiding this comment

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

The change to decrease verification on HotSpot in our test suite isn't needed (as far as I can tell) and isn't wanted (by me). The change to add additional documentation for OpenJ9 users seems valid to me though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I honestly don't remember a lot of details around this from 3 years ago. I think the issue I had was I couldn't run the tests locally because I had java9 setup, I had no idea it was a hotspot issue vs j9. I thought it was a java 8 vs java 9+ issue. I assume the test suite is running in something > java 8 at this point so my initial assumptions are wrong. I don't currently have the bandwidth to test this out in different jvms to see what works and what doesn't so I'm not sure how much value the documentation actually adds w/o those details. I guess I could make the docs more vague to cover it.

Copy link
Member

Choose a reason for hiding this comment

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

We run the test suite with HotSpot (Eclipse Temurin) on Java 11 and 17. Yes, I think a general suggestion in the documentation for OpenJ9 users to run without verification should cover it.

</configuration>
</execution>
<execution>
Expand Down