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

Small JFR clean-up #8037

Merged
merged 2 commits into from
Dec 20, 2023
Merged

Small JFR clean-up #8037

merged 2 commits into from
Dec 20, 2023

Conversation

roberttoyonaga
Copy link
Collaborator

@roberttoyonaga roberttoyonaga commented Dec 15, 2023

Summary

Some minor house cleaning in 3 areas:

  1. Remove the "hack" in JfrEventSubstitution.java. It's no longer needed because JDK code has changed to make EventWriter.reset() public.
  2. Added javadoc and logging in Target_jdk_jfr_internal_JVM.java. Currently, whenever a new JFR @deprecated event is used an unclear error message is logged:
 [warn][jfr,setting] Exception occurred when setting value "forRemoval" for class jdk.jfr.internal.Control
Passed

One issue is that there may be too much useless logger output that drowns out meaningful logs. A possible solution is to simply log that a feature is unimplemented only once. Instead of logging every time an attempt to use it is made. What do you think about this?
3. Avoid unnecessary module opening. This caused issues with Quarkus. I think this might simply be outdated code and is safe to remove now.

style 2
Copy link
Member

@christianhaeubl christianhaeubl left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, I plan to merge them into master and 24.0.

With the extra logging that you added in setMiscellaneous(), two warnings will be printed per invocation (one because of the explicit logging, one because of the exception), which is not ideal. I will remove the exception throw so that only the more specific message gets printed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
native-image native-image-jfr OCA Verified All contributors have signed the Oracle Contributor Agreement. redhat-interest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants