Review request for JMC-4645: Add size distribution chart to IO Pages

Alex Macdonald almacdon at redhat.com
Fri Apr 5 18:36:21 UTC 2019


Hi Ken,

On Fri, Mar 29, 2019 at 4:08 PM Ken Dobson <kdobson at redhat.com> wrote:

> Hi all,
>
> Please review this patch I've made that adds a size distribution chart to
> the IO Pages.
>
> Bug: https://bugs.openjdk.java.net/projects/JMC/issues/JMC-4645
> Webrev: http://cr.openjdk.java.net/~kdobson/sizedistributionchart/webrev/


Overall the code looks good from what I can tell.

Having said that, I just want to confirm what I'm reading off the chart,
let's use this sample screen shot [0] (https://imgur.com/sPo9Xeo) as an
example. The y-axis displays the number of reads/writes, and the x-axis
displays the size of that read or write? If that's the case I'm a bit
confused. The blue bar (write) hits 1 on the y axis for 1 read, and is
displayed at ~6.16KiB on the x axis because that's how much it wrote.
However, the red bar (read) hits 1797 on the y axis for 1797 reads as
expected, but why does the x axis display 0 - 256B when there was 1.89 KiB
read in total?

A couple of formatting nits:

1. There are some unused imports in the DataPageToolkit

[...]
+import org.openjdk.jmc.flightrecorder.ui.PageManager;
[...]
+import org.openjdk.jmc.flightrecorder.ui.pages.itemhandler.ItemHandlerPage;+import
org.openjdk.jmc.flightrecorder.ui.pages.itemhandler.ItemHandlerPage.ItemHandlerUiStandIn;
[...]

2. Excess white space just before (and inside)
DataPageToolkit.buildSizeHistogram()
3. JDKAttributes, SocketIO & FileIO pages also each have a few instances of
excess/trailing white space


>
> Thanks,
>
> Ken Dobson
>

Cheers,

Alex

[0] https://imgur.com/sPo9Xeo


More information about the jmc-dev mailing list