JMC-5640: Used Heap After GC Chart

[Jie Kang]

On Tue, Jul 23, 2019 at 10:29 AM Ken Dobson <kdobson at redhat.com> wrote:
>
> Hi all,
>
> Here is a patch that adds a Used Heap Post GC chart to the Garbage
> Collections page.
> The implementation is a bit hacky but the only way I could find to add a
> variation of an attribute already being displayed on a page was to create a
> duplicate attribute in JdkAttributes.java as well as compare names for
> uniqueness as identifiers are no longer unique if we intend to look at
> different subsets of the same attribute.
>
> If anyone has any ideas as to how to enact this change in a way that allows
> us to maintain consistency with the way the other pages operate as well as
> a way to avoid multiple copies of attributes feel free to share them.
>
> Bug Id: https://bugs.openjdk.java.net/browse/JMC-5640
> Webrev: http://cr.openjdk.java.net/~kdobson/JMC-5640/webrev/

Hey Ken,

One minor nit:


DataPageToolkit.buildLinesRow(Messages.GarbageCollectionsPage_ROW_HEAP,

Messages.GarbageCollectionsPage_ROW_HEAP_DESC, allItems, false,
HEAP_SUMMARY,
 legendFilter,
                                        UnitLookup.BYTE.quantity(0),
null).ifPresent(rows::add);
+                       DataPageToolkit.buildLinesRow("Heap Post GC",
+                                       "Heap Post GC", allItems,
false, HEAP_SUMMARY_POST_GC, legendFilter,
+                                       UnitLookup.BYTE.quantity(0),
null).ifPresent(rows::add);

You should add a String for "Heap Post GC" to Messages and reference
that instead, like how it's done above your addition. The rest looks
fine to me.

Regarding the 'hacky' Attributes addition, it's a bit complicated. The
toolkit code being used doesn't cover the situation where you are
showing the same attribute filtered two different ways in the same
chart. Though it can be confusing, the distinction does need to be
made somewhere along the line. I think this patch is okay but I could
see an issue made to address this use-case more cleanly. Trying to
re-work the existing code might take some time.


Regards,

>
> Thanks,
>
> Ken