-
Notifications
You must be signed in to change notification settings - Fork 162
Fix "No Detail" Use Cases for Legacy and Generational ZGC logging #448
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
base: main
Are you sure you want to change the base?
Conversation
Just checking that this will be merged at some point? I really don't want to have to redo them a second time if another breaking change makes it in first :) |
|
||
// We use the lack of a MemorySummary in the forwardReference as a sign that this is the case and | ||
// that we need to publish a Generational no-details event. | ||
ZGCCycleType type = ZGCCycleType.get(trace.getGroup(2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whitespace is slightly out
getForwardRefForPhase(ZGCPhase.MAJOR_YOUNG) : | ||
getForwardRefForPhase(ZGCPhase.MINOR_YOUNG); | ||
|
||
if (forwardReference != null && !forwardReference.hasMemorySummary()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpcik, but I have a slight preference for putting that logic into a function that effectively says what you said in the comment. e.g. Not the hill I'm going to die on though :-)
// Comment
if (noMemorySummary()) {
....
@jlittle-ptc Apologies, PR LGTM, just some small nitpicks |
Re-did the changes from PR #440 after a conflicting PR was brought in first.
Specifically, this PR addresses the following use cases:
Log lines from a Java 17/21 Legacy ZGC log without details (no gc*)
Log lines from Java 21 (Generational) GC Log without details: