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

JvmMonitor: Handle more generation and collector scenarios. #12469

Merged
merged 5 commits into from
Apr 27, 2022

Conversation

gianm
Copy link
Contributor

@gianm gianm commented Apr 21, 2022

ZGC on Java 11 only has a generation 1 (there is no 0). This causes
a NullPointerException when trying to extract the spacesCount for
generation 0. In addition, ZGC on Java 15 has a collector number 2
but no spaces in generation 2, which breaks the assumption that
collectors always have same-numbered spaces.

This patch adjusts things to be more robust, enabling the JvmMonitor
to work properly for ZGC on both Java 11 and 15.

ZGC on Java 11 only has a generation 1 (there is no 0). This causes
a NullPointerException when trying to extract the spacesCount for
generation 0. In addition, ZGC on Java 15 has a collector number 2
but no spaces in generation 2, which breaks the assumption that
collectors always have same-numbered spaces.

This patch adjusts things to be more robust, enabling the JvmMonitor
to work properly for ZGC on both Java 11 and 15.
Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

👍

@FrankChen021
Copy link
Member

FrankChen021 commented Apr 22, 2022

I notice that we're using the JStatData APIs provided in the org.gridkit.lab dependency to get metrics of GC. And this lib requires an additional JVM argument --add-exports=java.base/jdk.internal.perf=ALL-UNNAMED to enable the counters because it uses the APIs inside the tools.jar to get the counters.

The JDK itself provides public management API to allow us to access these counters.

I think we can use these APIs to replace the old one so that we:

  • can drop the additional JVM argument requirement on JDK 11 and above
  • don't need to change our code to adapt the collector and space names.

BTW, I'm not saying we need to do this in this PR.

@gianm gianm mentioned this pull request Apr 22, 2022
9 tasks
@gianm
Copy link
Contributor Author

gianm commented Apr 22, 2022

@FrankChen021 That sounds like it would be an excellent enhancement. I'm not sure why the current code uses JStatData instead of the MXBeans. IMO we could merge this patch now, and then switch to MXBeans when someone has the opportunity to write & test a new version of the JvmMonitor.

@gianm gianm closed this Apr 25, 2022
@gianm gianm reopened this Apr 25, 2022
@gianm gianm merged commit 72d15ab into apache:master Apr 27, 2022
@gianm gianm deleted the jvm-monitor-cool-collectors branch April 27, 2022 18:18
@abhishekagarwal87 abhishekagarwal87 added this to the 24.0.0 milestone Aug 26, 2022
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.

4 participants