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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GR-52942] Add JFR events and track peak usage for native memory tracking. #8668

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

graalvmbot
Copy link
Collaborator

No description provided.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Mar 29, 2024
@graalvmbot graalvmbot force-pushed the chaeubl/GR-52942 branch 2 times, most recently from 5d22ffc to 580a18d Compare April 2, 2024 12:54
@Description("Native memory peak usage for a given memory type in the JVM.")
@Category({"Java Virtual Machine", "Memory"})
@StackTrace(false)
public class NativeMemoryUsagePeakEvent extends Event {
Copy link
Collaborator

Choose a reason for hiding this comment

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

All of the other events with the "jdk" prefix have been ported over from OpenJDK. Maybe there should be some way of differentiating these NMT events. If we want to continue using the "jdk" prefix, do you think it would be a good idea to include a note in the event description indicating these events are built-in specific to Native Image?

Copy link
Member

Choose a reason for hiding this comment

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

We've also marked the new events as experimental for now, that's how I would currently look for them. But yes, a short note like (GraalVM Native Image only) or so in the description sounds reasonable to me.

@Label("Native Memory Usage Total Peak")
@Description("Information about native memory peak usage of committed virtual memory and malloc.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I initially specified that the peak usage is reported with regard to committed virtual memory + mallocs, since we report both committed and reserved memory in the other NMT events. Do you think it's not necessary? I'm fine leaving out those details if it's already obvious.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should report both committed and reserved for the Native Image-specific events as well. I will change the code accordingly.

@roberttoyonaga
Copy link
Collaborator

Thank you for working on integrating these changes @christianhaeubl !
Mostly everything looks good to me. I just left a few small questions above.

@graalvmbot graalvmbot closed this Apr 4, 2024
@graalvmbot graalvmbot deleted the chaeubl/GR-52942 branch April 4, 2024 23:19
@graalvmbot graalvmbot merged commit 62922eb into master Apr 4, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants