Record and report safepoint information from logs - #588
Conversation
|
|
Martijn Verburg (@karianna), Derek Keeler (@d3r3kk) can you please review this MR, i am not able to add reviewers |
|
For historical reasons, Safepoint is captured in it's own parser (SafepointParser), not in the individual parsers for each collector. There are examples on how to capture safepoints. That said, the formats for parsing haven't been kept up to date. |
Kirk Pepperdine (@kcpeppe) thank you very much for checking the MR. One thing to confirm, since SafepointParser extends PreUnifiedGCLogParser, I'm planning to add UnifiedSafepointParser extending UnifiedGCLogParser, registered in the same pre-unified/unified split JVMEventParser, SurvivorMemoryPoolParser and other parsers have, and not widen the SafepointParser itself. It'll publish Safepoint on EventSource.SAFEPOINT, with the JDK 14+ phases added to that class alongside the existing PrintSafepointStatistics fields, following how G1GCPauseEvent carries unified-only region summaries with a sentinel for the pre-unified case. |
|
Odd, I don't see a sentinel in G1GCPauseEvent. At any rate, mixing pre-unified with unified wouldn't be something I'd recommend doing. Theory being, it should be possible to remove pre-unified without leaving any trace of it behind and without breaking unified. So, pre-unified and unified share but only what is common to both. As for safepoints, they are treated differently than GC events for historical reasons that have long since gone away. I had ideas for how to integrate safepoint events into the overall event hierarchy in a more sensible way but life events parked this effort. Anyways, not my call but to me what you're proposing is consistant with how things are currently structured. |
|
Kirk Pepperdine (@kcpeppe) thank you for the confirmation, I will move forward with this.
I was reffering to RegionSummary in G1GCPauseEvent, which is getting constructed only in UniefiedG1GCParser, but even if this is the case, one mismatch with the approach does not mean we have to allow new ones. |
|
AlinaHovakimyan , do you mind pointing to this sentinal by providing a class::method::line#? I'm not finding it. AFAIK, there is no mixing a pre-unified and unified events. There is sharing in the hierarchy but that sharing is limited to things that are sharable. There should be no denial of inheritance or other like antipatterns in the code base. |



On ZGC logs example i saw that Safepoint lines were not getting reported by gctoolkit. The pattern existed, but was never used. So I have added parsing and reporting of it via ApplicationStoppedTimes.
I have added fromName() function to VMOperation's enum, as in every release vm operations get added or deleted, and by this gctoolkit will return null rather than throwing for unknowns, so that an unrecognized operation costs the reason and not the whole event.
Safepoint line differs between jdk versions, so i added optional parts to the pattern: cleanup -jdk17, jdk21, Leaving safepoint - jdk21, Threads - jdk25
Also added tests and tested locally manually