Review Request for JMC-6467

[Alex Macdonald]

Hi Henrik,

On Thu, Jul 25, 2019 at 1:37 PM Henrik Dafgård <hdafgard at gmail.com> wrote:

> Hi all,
>
> Please review this request for generalizing the G1 Heap Layout page. The
> pages.xml file that contains the new default color values could use one or
> two suggestions for Shenandoah events, if anyone has particular suggestions
> for default color values there.
>

I've applied the patch and tried to open the Heap Layout page, however I'm
running into a NPE when G1Page.createRegionList() [0] is adding items to
"allRegions".

For type jdk.ShenandoahHeapRegionInformation, the "typeAccessor" variable
[1] is coming back null, which ultimately causes an NPE when trying to to
retrieve a value from it [2]. Taking a bit of a deeper look there seems to
be a mismatch between the structure of REGION_STATE and what I'm observing
in the list of accessors. G1Constants is looking for (state, Type(text)),
however the actual value of the key involving state is (state,
Linear(number)). To quickly test I was able to retrieve a value using
attr("state", "State", NUMBER), however this doesn't match the return type
of customAccessor().

I only have one shenandoah jfr file that I've been using, if you aren't
experiencing this exception would you be able to provide a basic jfr file
for verifying the behaviour?

[0]
http://hg.openjdk.java.net/jmc/jmc/file/91a803e741ac/application/org.openjdk.jmc.flightrecorder.ext.g1/src/main/java/org/openjdk/jmc/flightrecorder/ext/g1/G1Page.java#l442
[1]
http://hg.openjdk.java.net/jmc/jmc/file/91a803e741ac/application/org.openjdk.jmc.flightrecorder.ext.g1/src/main/java/org/openjdk/jmc/flightrecorder/ext/g1/G1Page.java#l453
[2]
http://hg.openjdk.java.net/jmc/jmc/file/91a803e741ac/application/org.openjdk.jmc.flightrecorder.ext.g1/src/main/java/org/openjdk/jmc/flightrecorder/ext/g1/G1Page.java#l458


> JIRA: https://bugs.openjdk.java.net/browse/JMC-6467
> Webrev: http://cr.openjdk.java.net/~hdafgard/JMC-6467/webrev.0/
>
>
> Cheers,
> Henrik Dafgård
>

Cheers,

Alex