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

Author: j-bahr

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

@@ -1,5 +1,8 @@
 package com.microsoft.gctoolkit.event.zgc;
 
+import java.util.Arrays;
+import java.util.Objects;
+
 public enum ZGCCollectionType {
     FULL("Garbage"), // Legacy ZGC
     MINOR("Minor"),
@@ -12,11 +15,9 @@ public enum ZGCCollectionType {
     }
 
     public static ZGCCollectionType get(String label) {
-        for (ZGCCollectionType status : ZGCCollectionType.values()) {
-            if (status.collectionLabel.equalsIgnoreCase(label.trim())) {
-                return status;
-            }
-        }
-        throw new IllegalArgumentException("No matching ZGCCollectionType found for: " + label);
+        return Arrays.stream(ZGCCollectionType.class.getEnumConstants())
+                .filter(collectionType -> Objects.equals(collectionType.collectionLabel, label))
+                .findFirst()
+                .orElse(null);
     }
 }

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

@@ -1,20 +1,20 @@
 package com.microsoft.gctoolkit.event.zgc;
 
 public class ZGCMemorySummary {
-    private final long start;
-    private final long end;
+    private final long occupancyBefore;
+    private final long occupancyAfter;
 
-    public ZGCMemorySummary(long start, long end) {
-        this.start = start;
-        this.end = end;
+    public ZGCMemorySummary(long occupancyBefore, long occupancyAfter) {
+        this.occupancyBefore = occupancyBefore;
+        this.occupancyAfter = occupancyAfter;
     }
 
-    public long getStart() {
-        return start;
+    public long getOccupancyBefore() {
+        return occupancyBefore;
     }
 
-    public long getEnd() {
-        return end;
+    public long getOccupancyAfter() {
+        return occupancyAfter;
     }
 
 }

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

@@ -1,7 +1,10 @@
 package com.microsoft.gctoolkit.event.zgc;
 
+import java.util.Arrays;
+import java.util.Objects;
+
 public enum ZGCPhase {
-    FULL(""),
+    FULL(null),
     MAJOR_YOUNG("Y"),
     MAJOR_OLD("O"),
     MINOR_YOUNG("y");
@@ -17,11 +20,9 @@ public String getPhase() {
     }
 
     public static ZGCPhase get(String label) {
-        for (ZGCPhase zgcPhase : ZGCPhase.values()) {
-            if (zgcPhase.getPhase().equals(label.trim())) {
-                return zgcPhase;
-            }
-        }
-        throw new IllegalArgumentException("No matching ZGCPhase found for: " + label);
+        return Arrays.stream(ZGCPhase.class.getEnumConstants())
+                .filter(phase -> Objects.equals(phase.getPhase(), label))
+                .findFirst()
+                .orElse(null);
     }
 }

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

@@ -304,11 +304,6 @@ public void notYetImplemented() {
     }
 
     public ZGCPhase getZCollectionPhase() {
-        String phase = getGroup(1);
-        if (phase == null) {
-            return FULL;
-        } else {
-            return ZGCPhase.get(phase);
-        }
+        return ZGCPhase.get(getGroup(1));
     }
 }

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

@@ -20,10 +20,9 @@ public interface GenericTokens {
     //Time
     String TIME = "(-?" + REAL_NUMBER + ")";
     String DURATION_MS = TIME + "\\s?ms";
-    String DURATION_S = TIME + "\\s?s";
     String INT_DURATION_MS = INTEGER + "ms";
     //0.0700188
-    String PAUSE_TIME = TIME + "\\s?(?:secs?|ms)";
+    String PAUSE_TIME = TIME + "\\s?(?:secs?|ms|s)";
     String CONCURRENT_TIME = PAUSE_TIME;
 
     // Date values

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

@@ -59,7 +59,7 @@ public class ZGCParser extends UnifiedGCLogParser implements ZGCPatterns {
     private final boolean debugging = Boolean.getBoolean("microsoft.debug");
     private final boolean develop = Boolean.getBoolean("microsoft.develop");
 
-    private ZFwdRef forwardReference;
+    private ZForwardReference forwardReference;
 
     private final long[] markStart = new long[3];
     private final long[] markEnd = new long[3];
@@ -512,11 +512,11 @@ public void publish(JVMEvent event) {
         forwardReference = null;
     }
 
-    private interface ZFwdRef {
+    private interface ZForwardReference {
         GCEvent getGCEVent(DateTimeStamp endTime);
     }
 
-    private static class ZGCMinorForwardReference implements ZFwdRef {
+    private static class ZGCMinorForwardReference implements ZForwardReference {
         private final DateTimeStamp startTimeStamp;
         private final GCCause gcCause;
         private final ZGCCollectionType type;
@@ -567,7 +567,7 @@ public void setMemorySummary(ZGCMemorySummary cycleMemorySummary) {
         }
     }
 
-    private static class ZGCMajorForwardReference implements ZFwdRef {
+    private static class ZGCMajorForwardReference implements ZForwardReference {
         private final DateTimeStamp startTimeStamp;
         private final GCCause gcCause;
         private final ZGCCollectionType type;
@@ -625,7 +625,7 @@ public GCEvent getGCEVent(DateTimeStamp endTime) {
         }
     }
 
-    private static class ZGCForwardReference implements ZFwdRef {
+    private static class ZGCForwardReference implements ZForwardReference {
         private final DateTimeStamp startTimeStamp;
         private final GCCause gcCause;
         private final ZGCCollectionType type;

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

@@ -106,14 +106,14 @@ public interface ZGCPatterns extends UnifiedPatterns {
 
     // [info][gc,phases   ] GC(58) Y: Young Generation 16052M(44%)->3022M(8%) 1.017s
     // [info][gc,phases   ] GC(58) O: Old Generation 3022M(8%)->5486M(15%) 1.757s
-    GCParseRule END_OF_PHASE_SUMMARY_GEN = new GCParseRule("End of Phase Summary", OPT_GEN + "(Old|Young) Generation " + MEMORY_PERCENT + "->" + MEMORY_PERCENT + "\\s*" + DURATION_S);
+    GCParseRule END_OF_PHASE_SUMMARY_GEN = new GCParseRule("End of Phase Summary", OPT_GEN + "(Old|Young) Generation " + MEMORY_PERCENT + "->" + MEMORY_PERCENT + "\\s*" + PAUSE_TIME);
 
 
     //[3.596s][info ][gc         ] GC(3) Garbage Collection (Warmup) 894M(22%)->186M(5%)
     // or
     // Gen GC
     //[3.596s][info][gc          ] GC(7) Minor Collection (Allocation Rate) 14720M(40%)->2054M(6%) 0.689s
-    GCParseRule MEMORY_SUMMARY = new GCParseRule("Memory Summary", "(Garbage|Minor|Major) Collection " + GenericTokens.GC_CAUSE + MEMORY_PERCENT + "->" + MEMORY_PERCENT + "(?:\\s*" + DURATION_S + ")?" );
+    GCParseRule MEMORY_SUMMARY = new GCParseRule("Memory Summary", "(Garbage|Minor|Major) Collection " + GenericTokens.GC_CAUSE + MEMORY_PERCENT + "->" + MEMORY_PERCENT + "(?:\\s*" + PAUSE_TIME + ")?");
 
     /*
     todo: capture and report on these log entries

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

@@ -422,7 +422,7 @@ private boolean checkCompactedSummary(ZGCCompactedSummary summary, long relocate
     }
 
     private boolean checkMemorySummary(ZGCMemorySummary summary, long start, long end) {
-        return summary.getStart() == (start * 1024) && summary.getEnd() == (end * 1024);
+        return summary.getOccupancyBefore() == (start * 1024) && summary.getOccupancyAfter() == (end * 1024);
     }
 
     private int toInt(double value, int significantDigits) {

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

@@ -4,9 +4,6 @@
 
 import com.microsoft.gctoolkit.event.jvm.JVMEvent;
 import com.microsoft.gctoolkit.event.shenandoah.ShenandoahCycle;
-import com.microsoft.gctoolkit.event.zgc.OccupancySummary;
-import com.microsoft.gctoolkit.event.zgc.ZGCReclaimSummary;
-import com.microsoft.gctoolkit.event.zgc.ZGCMemoryPoolSummary;
 import com.microsoft.gctoolkit.jvm.Diarizer;
 import com.microsoft.gctoolkit.parser.jvm.UnifiedDiarizer;
 import org.junit.jupiter.api.Assertions;
@@ -89,20 +86,4 @@ public void infoLevelShenandoahCycle() {
             Assertions.fail(t);
         }
     }
-
-    private boolean checkZGCMemoryPoolSummary(ZGCMemoryPoolSummary summary, long capacity, long free, long used) {
-        return summary.getCapacity() == capacity && summary.getFree() == free && summary.getUsed() == used;
-    }
-
-    private boolean checkOccupancySummary(OccupancySummary summary, long markEnd, long relocateStart, long relocateEnd) {
-        return summary.getMarkEnd() == markEnd && summary.getReclaimStart() == relocateStart && summary.getReclaimEnd() == relocateEnd;
-    }
-
-    private boolean checkReclaimSummary(ZGCReclaimSummary summary, long relocateStart, long relocateEnd) {
-        return summary.getReclaimStart() == relocateStart && summary.getReclaimEnd() == relocateEnd;
-    }
-
-    private int toInt(double value, int significantDigits) {
-        return (int)(value * (double)significantDigits);
-    }
 }

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

@@ -159,7 +159,7 @@ private boolean checkReclaimSummary(ZGCReclaimSummary summary, long relocateStar
     }
 
     private boolean checkMemorySummary(ZGCMemorySummary summary, long relocateStart, long relocateEnd) {
-        return summary.getStart() == relocateStart && summary.getEnd() == relocateEnd;
+        return summary.getOccupancyBefore() == relocateStart && summary.getOccupancyAfter() == relocateEnd;
     }
 
     private int toInt(double value, int significantDigits) {