Skip to content

Commit 4d24ab9

Browse files
j-bahrj-bahr
committed
ZGC Fixes from #414
Minor fixes from feedback in #414 - Fix enum types to avoid having to throw IllegalArgumentException - Fix Memory summary to have more descriptive variable names - ZGCPhase now handles the regex null case correctly for Full (legacy) phase types - Use pause time to collect optional GC duration - Rename ZFwdRef -> ZForwardReference - Remove unused methods in ShenandoahParserTest.java
1 parent b193c9a commit 4d24ab9

File tree

12 files changed

+40
-63
lines changed

12 files changed

+40
-63
lines changed

‎IT/src/main/java/com/microsoft/gctoolkit/integration/aggregation/HeapOccupancyAfterCollectionAggregator.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ public HeapOccupancyAfterCollectionAggregator(HeapOccupancyAfterCollectionAggreg
2424
}
2525

2626
private void extractHeapOccupancy(MinorZGCCycle event) {
27-
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getYoungCycle().getMemorySummary().getEnd());
27+
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getYoungCycle().getMemorySummary().getOccupancyAfter());
2828
}
2929

3030
private void extractHeapOccupancy(MajorZGCCycle event) {
31-
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getMemorySummary().getEnd());
31+
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getMemorySummary().getOccupancyAfter());
3232
}
3333

3434
private void extractHeapOccupancy(GenerationalGCPauseEvent event) {
@@ -42,7 +42,7 @@ private void extractHeapOccupancy(G1GCPauseEvent event) {
4242
}
4343

4444
private void extractHeapOccupancy(FullZGCCycle event) {
45-
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getMemorySummary().getEnd());
45+
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getMemorySummary().getOccupancyAfter());
4646
}
4747

4848
private void extractHeapOccupancy(ShenandoahCycle event) {

‎api/src/main/java/com/microsoft/gctoolkit/event/zgc/ZGCCollectionType.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package com.microsoft.gctoolkit.event.zgc;
22

3+
import java.util.Arrays;
4+
import java.util.Objects;
5+
36
public enum ZGCCollectionType {
47
FULL("Garbage"), // Legacy ZGC
58
MINOR("Minor"),
@@ -12,11 +15,9 @@ public enum ZGCCollectionType {
1215
}
1316

1417
public static ZGCCollectionType get(String label) {
15-
for (ZGCCollectionType status : ZGCCollectionType.values()) {
16-
if (status.collectionLabel.equalsIgnoreCase(label.trim())) {
17-
return status;
18-
}
19-
}
20-
throw new IllegalArgumentException("No matching ZGCCollectionType found for: " + label);
18+
return Arrays.stream(ZGCCollectionType.class.getEnumConstants())
19+
.filter(collectionType -> Objects.equals(collectionType.collectionLabel, label))
20+
.findFirst()
21+
.orElse(null);
2122
}
2223
}

‎api/src/main/java/com/microsoft/gctoolkit/event/zgc/ZGCMemorySummary.java

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
11
package com.microsoft.gctoolkit.event.zgc;
22

33
public class ZGCMemorySummary {
4-
private final long start;
5-
private final long end;
4+
private final long occupancyBefore;
5+
private final long occupancyAfter;
66

7-
public ZGCMemorySummary(long start, long end) {
8-
this.start = start;
9-
this.end = end;
7+
public ZGCMemorySummary(long occupancyBefore, long occupancyAfter) {
8+
this.occupancyBefore = occupancyBefore;
9+
this.occupancyAfter = occupancyAfter;
1010
}
1111

12-
public long getStart() {
13-
return start;
12+
public long getOccupancyBefore() {
13+
return occupancyBefore;
1414
}
1515

16-
public long getEnd() {
17-
return end;
16+
public long getOccupancyAfter() {
17+
return occupancyAfter;
1818
}
1919

2020
}

‎api/src/main/java/com/microsoft/gctoolkit/event/zgc/ZGCPhase.java

+8-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package com.microsoft.gctoolkit.event.zgc;
22

3+
import java.util.Arrays;
4+
import java.util.Objects;
5+
36
public enum ZGCPhase {
4-
FULL(""),
7+
FULL(null),
58
MAJOR_YOUNG("Y"),
69
MAJOR_OLD("O"),
710
MINOR_YOUNG("y");
@@ -17,11 +20,9 @@ public String getPhase() {
1720
}
1821

1922
public static ZGCPhase get(String label) {
20-
for (ZGCPhase zgcPhase : ZGCPhase.values()) {
21-
if (zgcPhase.getPhase().equals(label.trim())) {
22-
return zgcPhase;
23-
}
24-
}
25-
throw new IllegalArgumentException("No matching ZGCPhase found for: " + label);
23+
return Arrays.stream(ZGCPhase.class.getEnumConstants())
24+
.filter(phase -> Objects.equals(phase.getPhase(), label))
25+
.findFirst()
26+
.orElse(null);
2627
}
2728
}

‎parser/src/main/java/com/microsoft/gctoolkit/parser/GCLogTrace.java

+1-6
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,6 @@ public void notYetImplemented() {
304304
}
305305

306306
public ZGCPhase getZCollectionPhase() {
307-
String phase = getGroup(1);
308-
if (phase == null) {
309-
return FULL;
310-
} else {
311-
return ZGCPhase.get(phase);
312-
}
307+
return ZGCPhase.get(getGroup(1));
313308
}
314309
}

‎parser/src/main/java/com/microsoft/gctoolkit/parser/GenericTokens.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@ public interface GenericTokens {
2020
//Time
2121
String TIME = "(-?" + REAL_NUMBER + ")";
2222
String DURATION_MS = TIME + "\\s?ms";
23-
String DURATION_S = TIME + "\\s?s";
2423
String INT_DURATION_MS = INTEGER + "ms";
2524
//0.0700188
26-
String PAUSE_TIME = TIME + "\\s?(?:secs?|ms)";
25+
String PAUSE_TIME = TIME + "\\s?(?:secs?|ms|s)";
2726
String CONCURRENT_TIME = PAUSE_TIME;
2827

2928
// Date values

‎parser/src/main/java/com/microsoft/gctoolkit/parser/ZGCParser.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public class ZGCParser extends UnifiedGCLogParser implements ZGCPatterns {
5959
private final boolean debugging = Boolean.getBoolean("microsoft.debug");
6060
private final boolean develop = Boolean.getBoolean("microsoft.develop");
6161

62-
private ZFwdRef forwardReference;
62+
private ZForwardReference forwardReference;
6363

6464
private final long[] markStart = new long[3];
6565
private final long[] markEnd = new long[3];
@@ -512,11 +512,11 @@ public void publish(JVMEvent event) {
512512
forwardReference = null;
513513
}
514514

515-
private interface ZFwdRef {
515+
private interface ZForwardReference {
516516
GCEvent getGCEVent(DateTimeStamp endTime);
517517
}
518518

519-
private static class ZGCMinorForwardReference implements ZFwdRef {
519+
private static class ZGCMinorForwardReference implements ZForwardReference {
520520
private final DateTimeStamp startTimeStamp;
521521
private final GCCause gcCause;
522522
private final ZGCCollectionType type;
@@ -567,7 +567,7 @@ public void setMemorySummary(ZGCMemorySummary cycleMemorySummary) {
567567
}
568568
}
569569

570-
private static class ZGCMajorForwardReference implements ZFwdRef {
570+
private static class ZGCMajorForwardReference implements ZForwardReference {
571571
private final DateTimeStamp startTimeStamp;
572572
private final GCCause gcCause;
573573
private final ZGCCollectionType type;
@@ -625,7 +625,7 @@ public GCEvent getGCEVent(DateTimeStamp endTime) {
625625
}
626626
}
627627

628-
private static class ZGCForwardReference implements ZFwdRef {
628+
private static class ZGCForwardReference implements ZForwardReference {
629629
private final DateTimeStamp startTimeStamp;
630630
private final GCCause gcCause;
631631
private final ZGCCollectionType type;

‎parser/src/main/java/com/microsoft/gctoolkit/parser/unified/ZGCPatterns.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,14 @@ public interface ZGCPatterns extends UnifiedPatterns {
106106

107107
// [info][gc,phases ] GC(58) Y: Young Generation 16052M(44%)->3022M(8%) 1.017s
108108
// [info][gc,phases ] GC(58) O: Old Generation 3022M(8%)->5486M(15%) 1.757s
109-
GCParseRule END_OF_PHASE_SUMMARY_GEN = new GCParseRule("End of Phase Summary", OPT_GEN + "(Old|Young) Generation " + MEMORY_PERCENT + "->" + MEMORY_PERCENT + "\\s*" + DURATION_S);
109+
GCParseRule END_OF_PHASE_SUMMARY_GEN = new GCParseRule("End of Phase Summary", OPT_GEN + "(Old|Young) Generation " + MEMORY_PERCENT + "->" + MEMORY_PERCENT + "\\s*" + PAUSE_TIME);
110110

111111

112112
//[3.596s][info ][gc ] GC(3) Garbage Collection (Warmup) 894M(22%)->186M(5%)
113113
// or
114114
// Gen GC
115115
//[3.596s][info][gc ] GC(7) Minor Collection (Allocation Rate) 14720M(40%)->2054M(6%) 0.689s
116-
GCParseRule MEMORY_SUMMARY = new GCParseRule("Memory Summary", "(Garbage|Minor|Major) Collection " + GenericTokens.GC_CAUSE + MEMORY_PERCENT + "->" + MEMORY_PERCENT + "(?:\\s*" + DURATION_S + ")?" );
116+
GCParseRule MEMORY_SUMMARY = new GCParseRule("Memory Summary", "(Garbage|Minor|Major) Collection " + GenericTokens.GC_CAUSE + MEMORY_PERCENT + "->" + MEMORY_PERCENT + "(?:\\s*" + PAUSE_TIME + ")?");
117117

118118
/*
119119
todo: capture and report on these log entries

‎parser/src/test/java/com/microsoft/gctoolkit/parser/GenerationalZGCParserTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ private boolean checkCompactedSummary(ZGCCompactedSummary summary, long relocate
422422
}
423423

424424
private boolean checkMemorySummary(ZGCMemorySummary summary, long start, long end) {
425-
return summary.getStart() == (start * 1024) && summary.getEnd() == (end * 1024);
425+
return summary.getOccupancyBefore() == (start * 1024) && summary.getOccupancyAfter() == (end * 1024);
426426
}
427427

428428
private int toInt(double value, int significantDigits) {

‎parser/src/test/java/com/microsoft/gctoolkit/parser/ShenandoahParserTest.java

-19
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44

55
import com.microsoft.gctoolkit.event.jvm.JVMEvent;
66
import com.microsoft.gctoolkit.event.shenandoah.ShenandoahCycle;
7-
import com.microsoft.gctoolkit.event.zgc.OccupancySummary;
8-
import com.microsoft.gctoolkit.event.zgc.ZGCReclaimSummary;
9-
import com.microsoft.gctoolkit.event.zgc.ZGCMemoryPoolSummary;
107
import com.microsoft.gctoolkit.jvm.Diarizer;
118
import com.microsoft.gctoolkit.parser.jvm.UnifiedDiarizer;
129
import org.junit.jupiter.api.Assertions;
@@ -89,20 +86,4 @@ public void infoLevelShenandoahCycle() {
8986
Assertions.fail(t);
9087
}
9188
}
92-
93-
private boolean checkZGCMemoryPoolSummary(ZGCMemoryPoolSummary summary, long capacity, long free, long used) {
94-
return summary.getCapacity() == capacity && summary.getFree() == free && summary.getUsed() == used;
95-
}
96-
97-
private boolean checkOccupancySummary(OccupancySummary summary, long markEnd, long relocateStart, long relocateEnd) {
98-
return summary.getMarkEnd() == markEnd && summary.getReclaimStart() == relocateStart && summary.getReclaimEnd() == relocateEnd;
99-
}
100-
101-
private boolean checkReclaimSummary(ZGCReclaimSummary summary, long relocateStart, long relocateEnd) {
102-
return summary.getReclaimStart() == relocateStart && summary.getReclaimEnd() == relocateEnd;
103-
}
104-
105-
private int toInt(double value, int significantDigits) {
106-
return (int)(value * (double)significantDigits);
107-
}
10889
}

‎parser/src/test/java/com/microsoft/gctoolkit/parser/ZGCParserTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ private boolean checkReclaimSummary(ZGCReclaimSummary summary, long relocateStar
159159
}
160160

161161
private boolean checkMemorySummary(ZGCMemorySummary summary, long relocateStart, long relocateEnd) {
162-
return summary.getStart() == relocateStart && summary.getEnd() == relocateEnd;
162+
return summary.getOccupancyBefore() == relocateStart && summary.getOccupancyAfter() == relocateEnd;
163163
}
164164

165165
private int toInt(double value, int significantDigits) {

‎sample/src/main/java/com/microsoft/gctoolkit/sample/aggregation/HeapOccupancyAfterCollection.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ public HeapOccupancyAfterCollection(HeapOccupancyAfterCollectionAggregation resu
2424
}
2525

2626
private void extractHeapOccupancy(MinorZGCCycle event) {
27-
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getMemorySummary().getEnd());
27+
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getMemorySummary().getOccupancyAfter());
2828
}
2929

3030
private void extractHeapOccupancy(MajorZGCCycle event) {
31-
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getMemorySummary().getEnd());
31+
aggregation().addDataPoint(event.getGarbageCollectionType(), event.getDateTimeStamp(), event.getMemorySummary().getOccupancyAfter());
3232
}
3333

3434
private void extractHeapOccupancy(GenerationalGCPauseEvent event) {

0 commit comments

Comments
 (0)