G1GCForward reference is cumbersome

[dsgrieve]

G1GCForwardReference grew organically and needs to be looked at to see if it could be refactored into something more extensible. One thought is to have a G1GCForwardReference interface with specific implementation for the different patterns. But this may lead to unwanted casting.

[dsgrieve]

From @kirk-microsoft:

At the moment, the G1 parser (well all of the concurrent parsers) will issue events in time order of when they started. The events themselves have immutable properties of; the type of event (represented by the class), the time the event started, and how long the event lasted. Generally we don’t see how long the event lasted until we see the last record. Thus we can’t create the event until we’ve seen that record. This leaves us with the problem of, how do we retain data until that event has been seen.. the answer is, we use a forward reference and then have that forward reference build the event and then emit it.. a variation of the builder pattern. When I first started this code, the amount of data collected in the forward reference wasn’t huge. The lazy implementation was to use a single forward reference implementation for all G1 phases and cycles. Now, this isn’t the case and it’s time to specialize the forward reference implantations to ease the confusion.

[brunoborges]

/relevance-check

[github-actions]

Relevance Assessment: Still Relevant

• Summary: The G1GCForwardReference class remains a single monolithic 905-line class with ~96 private fields. No refactoring into specialized implementations has been performed since this issue was filed.
• Evidence:
  • G1GCForwardReference.java still exists at parser/src/main/java/com/microsoft/gctoolkit/parser/G1GCForwardReference.java as a single class extending ForwardReference (905 lines, ~96 private fields, ~121 member declarations).
  • No subclasses or specialized implementations of G1GCForwardReference exist — it remains the only G1-specific forward reference class. The only other ForwardReference subclass is GenerationalForwardReference.
  • The class is used in UnifiedG1GCParser.java as a single type for all G1 GC events, exactly as described in the issue.
  • The only related PR (Move g1 heapRegionSize from class variable to member variable #324, merged Jan 2024) was a small fix to move heapRegionSize from a static to a member variable — not a structural refactoring.
  • Related open issues (Getters for string&symbol table #373 — missing getters for string/symbol table, G1GC Concurrent Cleanup event #378 — missing concurrent events) further indicate the class continues to accumulate concerns without being decomposed.
  • The class still contains a comment //bag of stuff to maybe eliminate (line ~76), underscoring its organic growth.
• Recommendation: ✅ Keep open — The described problem (monolithic forward reference that should be specialized per G1 phase/cycle type) is fully present in the current codebase and has not been addressed. The class has only grown since this issue was filed, making the refactoring increasingly valuable.

Generated by Relevance Check Agent for issue #133