Add JFR GC Heap Summary event - #23652
Conversation
|
Testing: test jfr GCHeapSummary - approx 30 seconds ---TEST RESULTS---
|
a8a794e to
25becdd
Compare
|
@amicic, @TobiAjila please review |
25becdd to
0c69530
Compare
|
|
||
| /* Populate virtual space fields */ | ||
| jfrEvent->vStart = (U_64)(uintptr_t)heapBase; | ||
| jfrEvent->vCommittedEnd = (U_64)(uintptr_t)heapTop; |
There was a problem hiding this comment.
not sure what's the intention of vCommittedEnd, but in general committed and uncommitted memory can be interleaved (especially true with Balanced GC with Offheap, but to a small degree even in Gencon), but this (heapTop) is probably a good enough simple answer as a start.
| jfrEvent->vStart = (U_64)(uintptr_t)heapBase; | ||
| jfrEvent->vCommittedEnd = (U_64)(uintptr_t)heapTop; | ||
| jfrEvent->vCommittedSize = (U_64)heapSize; | ||
| jfrEvent->vReservedEnd = (U_64)(uintptr_t)heapTop; |
There was a problem hiding this comment.
again, for Balanced GC with Offheap, reserved start and end that you get from the our API do not account for Offheap
but for a start, let's not worry about that
| jfrEvent->gcWhenID = gcWhenID; /* BeforeGC or AfterGC */ | ||
|
|
||
| /* Get heap information from memory manager */ | ||
| void *heapBase = javaVM->memoryManagerFunctions->j9gc_get_heap_base(javaVM); |
There was a problem hiding this comment.
why are the local variables needed?
There was a problem hiding this comment.
It's so that I don't have to go fetch them through the memory manager functions again. The values are used in a couple places. Also, just to be consistent i.e. freeMemory and heapBase don't need them but consistency.
|
jenkins test sanity.functional xlinux jdk17 |
|
jenkins test extended.functional alinux64 jdk21 |
| "concurrent kickoff" | ||
| }; | ||
|
|
||
| static constexpr const char * const gcWhens[] = { |
There was a problem hiding this comment.
this needs to be "Before GC" and "After GC" case sensitive as JMC keys on this.
0c69530 to
52a77c4
Compare
|
jenkins test sanity.functional xlinux jdk17 |
|
@adpopescu Have you verified that the data makes sense with something like verbose GC logs? |
|
You mean for the actual sizes or for the heap addresses? I didn't compare them directly since they are essentially using the same values more or less? I do want to make a couple more changes to the values though now that you mention it. I went back and rechecked some of the values and I think they should be slightly different. |
|
I wasnt sure about the output I was seeing in JMC thats why I asked. |
This change adds the JFR GC Heap summary event. It is emitted at both cycle start and cycle end. Signed-off-by: Adrian Popescu <adpopescu@ibm.com>
52a77c4 to
96405bb
Compare
|
@adpopescu Have you had a chance to verify this behaves similar to GC verbose logs? |
|
I'm investigating an issue that is only showing up on metronome. Need to make sure it's unrelated to the changes. It's a runtime exception so I have to check it before we can merge anything in. |
|
jenkins test sanity.functional xlinux jdk17 |
|
@tajila The issue with metronome seems unrelated to this change. Also, yes I checked the values against verbose gc and they are the same. This should be good to go now. |
|
jenkins test sanity.functional xlinux jdk17 |
3 participants
This change adds the JFR GC Heap summary event. It is emitted at both cycle start and cycle end.