gctoolkit
gctoolkit copied to clipboard
microsoft
Remove if("Finished") when we start collecting this information
https://github.com/microsoft/gctoolkit/blob/4852eb3d1388fe4dc9de8e115fad28bb3581796e/parser/src/main/java/com/microsoft/gctoolkit/parser/GenerationalHeapParser.java#L351
Hi @brunoborges, would like to contribute to this.
Can you let me know the exact requirement here? Is this just a removal of that if the condition or requires more changes?
Possibly a question for @kirk-microsoft - is Finished a useful metric that folks may want to build analytics off?
grep for Finished in the gclogs test data and you'll see a ton of matching lines, none of which have a rule for matching. If you remove this line of code, then the parser will try to find a matching rule - which will fail. So the point here is, the patterns in the ignoreFrequentButUnwantedEntries are there to avoid unnecessary lookup for matching rules. Since we do not collect "Finished" information, this line should remain.
Sounds like just remove the TODO comment as the same comment applies for every other line in the ignoreFrequentButUnwantedEntries method...
Oh wait, if I got it right, you just want the TODO comment removed? 😀
I think we should add a comment at the top stating that this class covers all the lines where we don't collect any useful metrics (yet). Then I think we can remove all of the TODOs and I'd then love to see folks knowledgeable about those lines commenting each one stating "Why" they're not useful.
Don't know if I can add anything useful here but there are cases where information is only useful to those debugging the collector and really doesn't offer any great value to those tuning. This is data we can safely ignore. One example is PrintHeapAtGC. The data is really nice and concise but we already have it from other lines in the log file so... we can very safely ignore this in the log file.
As for the Forward, it's likely that there is some useful information in the data but it's likely secondary and thus wasn't a huge priority to collect. That said, the todo is a reminder that if the parsers are enhanced to collect it, you're going to want to remove this filter from this method. My guess is at this point in time that we're never going to get back to this.
