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