Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix "No Detail" Use Cases for Legacy and Generational ZGC logging #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d103fea
Apply fixes from FixNoDetailParsing branch.
jlittle-ptc Jun 23, 2025
62e5a8e
Test and fix sample Generational Events to ensure no extra events are…
jlittle-ptc Jun 23, 2025
e36fd81
Fix issue with GC Cause reporting found during test.
jlittle-ptc Jun 23, 2025
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
58 changes: 53 additions & 5 deletions parser/src/main/java/com/microsoft/gctoolkit/parser/ZGCParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,17 +194,33 @@ private void setForwardRefForPhase(ZGCPhase zgcPhase, ZGCForwardReference forwar

private void cycleStart(GCLogTrace trace, String s) {
ZGCCycleType type = ZGCCycleType.get(trace.getGroup(2));

if (diary.isGenerationalZGC()) {
// cycleStart ends up being called for ZGC logging with no details, but did not handle this situation.
// It is not clear if there is any differentiation between Young/Old Phases in no-details logging, so I've made
// an assumption here that we're strictly dealing with Young collections until we have sample logging otherwise.
if (ZGCCycleType.MAJOR.equals(type)) {
setForwardRefForPhase(
ZGCPhase.MAJOR_YOUNG,
new ZGCForwardReference(getClock(), trace.getLongGroup(1), trace.gcCause(3,0), type, ZGCPhase.MAJOR_YOUNG));
} else if (ZGCCycleType.MINOR.equals(type)) {
setForwardRefForPhase(
ZGCPhase.MINOR_YOUNG,
new ZGCForwardReference(getClock(), trace.getLongGroup(1), trace.gcCause(3,0), type, ZGCPhase.MINOR_YOUNG));
}

}

if(type == ZGCCycleType.FULL){
setForwardRefForPhase(
ZGCPhase.FULL,
new ZGCForwardReference(getClock(), trace.getLongGroup(1), trace.gcCause(3,0), type, ZGCPhase.FULL)
);
}
else {
} else {
// The cycle start message gives us the gc cause, which we need to create the GCEvent in generationStart
// When we get a cycle start, store the gc cause for later use
gcCauseMap.put(trace.getLongGroup(1), trace.gcCause(1, 2));
}
}
}

private void generationStart(GCLogTrace trace, String line){
Expand Down Expand Up @@ -549,11 +565,39 @@ private void generationEnd(GCLogTrace trace, String s) {
}

private void memorySummary(GCLogTrace trace, String s) {
if(diary.isGenerationalZGC()){
if (diary.isGenerationalZGC()) {
long gcId = trace.getLongGroup(1);
gcCauseMap.remove(gcId);

if (gcCauseMap.containsKey(gcId)) {
gcCauseMap.remove(gcId);
}

// 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));
Copy link
Member

Choose a reason for hiding this comment

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

Whitespace is slightly out

ZGCForwardReference forwardReference = ZGCCycleType.MAJOR.equals(type) ?
getForwardRefForPhase(ZGCPhase.MAJOR_YOUNG) :
getForwardRefForPhase(ZGCPhase.MINOR_YOUNG);

if (forwardReference != null && !forwardReference.hasMemorySummary()) {
Copy link
Member

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()) {
....

forwardReference.setMemorySummary(
new ZGCMemorySummary(
trace.toKBytes(4),
trace.toKBytes(7)));
publish(forwardReference.getGCEVent(getClock()));
}
} else {
ZGCForwardReference forwardReference = getForwardRefForPhase(ZGCPhase.FULL);

// For Legacy ZGC (Java 17, non-generational) with no details (gc instead of gc*), only a single line
// is logged per event. As such, getForwardRefForPhase will return NULL.
if (forwardReference == null) {
// Forward reference has not yet been created, so we'll create it now
ZGCCycleType type = ZGCCycleType.get(trace.getGroup(2));
forwardReference = new ZGCForwardReference(
getClock(), trace.getLongGroup(1), trace.gcCause(3,0), type, ZGCPhase.FULL);
}

forwardReference.setMemorySummary(
new ZGCMemorySummary(
trace.toKBytes(4),
Expand Down Expand Up @@ -884,6 +928,10 @@ public void setMemorySummary(ZGCMemorySummary summary) {
this.memorySummary = summary;
}

public boolean hasMemorySummary() {
return this.memorySummary != null;
}

public void setMetaspaceSummary(ZGCMetaspaceSummary summary) {
this.metaspaceSummary = summary;
}
Expand Down
Loading