-
Notifications
You must be signed in to change notification settings - Fork 981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for ZGC on jdk11 #211
Comments
[0.065s][info][gc] Using The Z Garbage Collector |
We are also facing the same issue, my team and I are currently taking a stab at adding ZGC support by adding and categorizing ZGC log events. We are looking to open a PR on this. Cheers, |
Cool! You probably want to have a look at AbstractGCEvent (adding gc
events you unknown to GCViewer) and DataReaderUnifiedJvmLogging (for
tweaking the parser).
Best regards,
Jörg
|
Hi @chewiebug, @Marysunithajoseph and I have been working on this for the past few days, our progress so far is on https://github.com/macmms/GCViewer/tree/feature/protoZGC As of now, we are parsing the [gc,phases] for the pause/concurrent events, [gc,start] for starting a GC cycle, "[gc,heap] Capacity" for total heap size, as well as "[gc] Garbage Collection" for preUsed and postUsed heap size.
Wanted to have a checkpoint here and see what you think. Thanks, |
PR is up at #216 |
Hi @yanqilee Sorry for not answering earlier! I have very little time for GCViewer lately. Still I'd like to help your pull request along so that it can be merged - it looks good and close to finished! I'll remark on your screenshots here and add some comments to the pull request directly. Looking at the screenshots you posted, I see, that parsing if the ZGC logs already looks really good! The events are there, pauses are measured and collected in the event details panel. What strikes me as odd compared to the currently used concept in GCViewer is the introduction of the "GC causes" section. I see in your "all" sample, that ZGC thinks of all of these events as one collection. Based on your knowledge about ZGC (I have none so far), is this, what is interesting to know about ZGC collections to be able to tune the garbage collector? Comparing to the current implementation, I'd have expected one collection cycle to represent one "gc pause" (like in G1 collector - there are also gc,phases present, but they look rather like parts of one collection, than like separate pauses). To be able to treat G1 and ZGC in a more similar way, I could also imagine, to sum up the "Pause" parts of the ZGC collector and treat "Gargabe Collection (Allocation Rate)" as one GC pause and introduce a new section "gc phases", which shows the separate "Pause" parts. Would that make sense for you? The second thing: The "gc causes" in the second screenshot have huge numbers. What's the intention of those? Or what should be there? |
Hi @chewiebug, No problem! Thanks for responding! For the Gc causes section, it might have been over-engineered in hindsight. To address the second thing, the numbers are huge because they represent the amount the heap size changed after the garbage collection cycle and not the pause time (I can see this being super confusing). I’m going to remove the “Gc Causes” section. The pause and concurrent event times should give a good amount of information to tune the garbage collector. The memory usage information is also present via the “Garbage Collection” events. To compare with G1/Shenandoah, the [gc,phases] in ZGC correspond to their pause/concurrent events, for example “Pause Initial Mark”, “concurrent marking”, etc. While ZGC Garbage Collection cycles like “Garbage Collection (Allocation Rate)” correspond to events like “Pause Full”, “Pause Young”, etc. Seems to me like the [gc,phases] in G1/Shenandoah are more like sub-phases, and the actual phases like “Pause Initial Mark” are on their own [gc] line. While in ZGC logging there was a design change, in which the pause/concurrent events are moved into [gc,phases] section, with [gc] only having the big events like “Pause Full” and “Pause Young”, does that make sense? So instead of introducing a new “Gc phases” section on the viewer, I’m thinking of put GC cycles in the “Full GC Pauses” section, it won’t have any pause duration information but it will serve as a reference point to garbage collection reasons and count, as well as storing memory info. It would look something like this: I think that would make the most sense, what do you think? I have commented on the PR to questions you asked; please let me know if you need any more clarification. I’m a little occupied at the moment as well, I should have time to work on this in about two weeks, but I will be available for any questions you have, and discuss the design in the meantime :) |
Hi @yanqilee, Thank you very much for your explanations and thoughts! I have now also read some articles about ZGC myself to get a better understanding of this new garbage collection algorithm. If I try to summarise your explanations and what I got from this article and this youtube presentation I see the following concepts / differences introduces by ZGC compared to previous algorithms:
Checking the current code of GCViewer, the concepts used are the following:
ConclusionsI believe, your implementation is on a very good way. There is just one issue, which currently breaks the chart (no memory information shown) and the data panel (memory): The GCEvents don't contain both pause times and memory information. Suggestion for implementationMy suggestion to merge your current implementation concepts with the current concepts present in GCViewer: Add a the list of "phases" events and a new method "addPhase()" to the AbstractGCEvent class. Override the "addPhases()" method in GCEventUJL to accumulate the pauses present in the "phases" list. Then we will have the "standard" (GCViewer) GCEvents containing memory and pause information. To accomodate the new kind of "phases" of ZGC, you could add the separate "phases" list in GCModel and add a separate section "gc phases" (similar to the "gc causes" you added in one of your earlier attempts). The "Event details" tab would then show the following:
The (memory) data panel would again show meaningful data. What do you think of my suggestion? |
Hi @chewiebug, Thanks for your thought and detailed suggestion, that helps a lot. I think it makes sense to add a phases section and have it contribute to the gc event that is the collection cycle. To summarize your suggestion the way I understood it:
Please let me know if I misunderstood anything. I also have a question regarding the complications involving the inclusion of phases. Prior to ZGC parsing, phases were ignored. Unit tests were built around that, once I included [gc,phases] in the INCLUDE_STRINGS, unit test cases for UJL were breaking because there are [gc,phases] lines in the test files (CMS, G1, etc) that is now giving a warning "Failed to parse gc event" which fails the assertion of 0 warnings. I see you have commented on the below code in the PR, we have added those as a way to only include ZGC [gc,phases]. I don't quite like the solution and I've been trying to find better ways to go about this, I haven't come up with a better solution yet.
Do you have any suggestions regarding this? Please let me know if you have any questions. I will be making these changes soon, cheers. |
Hi @yanqilee, If think, you summarised my suggestion very nicely! If you implement it that way, I believe, we will have a solid baseline to add gc,phases parsing for other gc algorithms. Adding gc,phases support for all other gc algorithms should be fairly easy then. Which leads to your question regarding the complications (breaks some unittests, because new parser warnings are introduced). I suggest the following:
Then finish your implementation. Is this enough information for you to continue, or would you like me to also update the comments in the pull request? I am looking forward to merging your pull request! Best regards, |
Hi @chewiebug, That's perfect, that cleared all my doubts and I'll be able to continue and make all the changes. Cheers, |
#216 is now merged and support for parsing ZGC logs is present in the latest SNAPSHOT version. @jborgers: The log you posted in your second comment of this issue will be parsed, but won't yield much in terms of display in the chart. If you add the gc,start information to your logs, you will see much more. |
Thanks! Good to know!
Kind regards, met vriendelijke groet,
Jeroen Borgers
drs. Jeroen Borgers
Principal Consultant
jPinpoint Performance Services, jpinpoint com, The Netherlands.
Op ma 10 jun. 2019 om 18:48 schreef chewiebug <[email protected]>:
… #216 <#216> is now merged and
support for parsing ZGC logs is present in the latest SNAPSHOT version.
@jborgers <https://github.com/jborgers>: The log you posted in your
second comment of this issue will be parsed, but won't yield much in terms
of display in the chart. If you add the gc,start information to your logs,
you will see much more.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#211?email_source=notifications&email_token=AF3TVW42ZSYK2TIS3ZSJNPDPZ2AQJA5CNFSM4GDAAHVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXKN5YA#issuecomment-500489952>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AF3TVWZ3ZJD5JDRTYF6TSITPZ2AQJANCNFSM4GDAAHVA>
.
|
Hi,
I am using ZGC on jdk-11 and the gc-log files don't parse correctly. I get:
INFO [DataReaderFacade]: GCViewer version 1.36-SNAPSHOT (2018-10-23T20:08:13+0000)
INFO [DataReaderFactory]: File format: Oracle / OpenJDK unified jvm logging
INFO [DataReaderUnifiedJvmLogging]: Reading Oracle / OpenJDK unified jvm logging format...
INFO [DataReaderUnifiedJvmLogging]: Using The Z Garbage Collector
WARNING [DataReaderUnifiedJvmLogging]: Failed to parse gc event (com.tagtraum.perf.gcviewer.imp.UnknownGcTypeException: Unknown gc type: 'Garbage Collection (Warmup)') on line number 12 (line="[0,595s][info][gc ] GC(0) Garbage Collection (Warmup) 116M(11%)->102M(10%)")
WARNING [DataReaderUnifiedJvmLogging]: Failed to parse gc event (com.tagtraum.perf.gcviewer.imp.UnknownGcTypeException: Unknown gc type: 'Garbage Collection (Warmup)') on line number 23 (line="[1,048s][info][gc ] GC(1) Garbage Collection (Warmup) 208M(20%)->164M(16%)")
WARNING [DataReaderUnifiedJvmLogging]: Failed to parse gc event (com.tagtraum.perf.gcviewer.imp.UnknownGcTypeException: Unknown gc type: 'Garbage Collection (Warmup)') on line number 34 (line="[2,070s][info][gc ] GC(2) Garbage Collection (Warmup) 402M(39%)->494M(48%)")
WARNING [DataReaderUnifiedJvmLogging]: Failed to parse gc event (com.tagtraum.perf.gcviewer.imp.UnknownGcTypeException: Unknown gc type: 'Garbage Collection (Allocation Rate)') on line number 45 (line="[3,167s][info][gc ] GC(3) Garbage Collection (Allocation Rate) 502M(49%)->716M(70%)")
WARNING [DataReaderUnifiedJvmLogging]: Failed to parse gc event (com.tagtraum.perf.gcviewer.imp.UnknownGcTypeException: Unknown gc type: 'Garbage Collection (Allocation Rate)') on line number 56 (line="[4,573s][info][gc ] GC(4) Garbage Collection (Allocation Rate) 718M(70%)->580M(57%)")
[..]
etc.
I attached the gc log file I used.
I used -Xlog:gc,gc+phases:gc.log
Is the gc-phases info also used? Other tags/levels to include?
Would be great if ZGC is also supported.
Thanks!
Jeroen
The text was updated successfully, but these errors were encountered: