RFR: JMC-5327: Using HdrHistograms to visualize latencies

[Elliott Baron]

Hi,

Sorry for the delay. Thank you both for your reviews!

On 2019-05-02 12:55 p.m., Alex Macdonald wrote:
> A handful of minor formatting nits:
> 
> - Messages.properties (x3)
> - - the DurationPercentileTable_PERCENTILE_COL_NAME should come after 
> DUMP_RECORDING_* for alphabetical order I believe

Should be fixed now. I took this into account for the new keys added in 
this revision too.

> - There are some cases of extra whitespace, I went through hg diff and 
> counted the following number of occurrences:
> - - DurationHdrHistogram (8)
> - - DurationPercentileTable (38)
> - - FileIOPage (5)
> - - SocketIOPage (6)
> - - IOPageTest (5)
> - - IOPageTestBase (4)
> - - SocketIOPageTest(5)

This revised patch should not have any trailing whitespace.

> I noticed that when resizing the page, the durations table stays static 
> whereas the other tables seems to adjust (gif) [3]. This only seems to 
> cause an issue when my JMC window gets resized to just under 50% on my 
> monitor width, and the chart will get hidden. In the gif my JMC window 
> is around half my monitor width (of 1920), and with the JVM 
> Browser/Outline window open then I don't have much more room to shrink 
> the application before the chart disappears. I was just curious if the 
> table could adjust to resizing like the other tables.

This was a bit tricky due to the table sitting side-by-side with the 
chart. I used the (unused?) SimpleLayout class within JMC to define 
weights to keep the percentage of the width occupied by the chart and 
table constant, even while resizing.

> Also not caused by this patch (but thought it'd be interesting to 
> share), I'm seeing a bug where my chart text gets really large [4] when 
> I run JMC locally and not as an RCP application (I recall seeing a JIRA 
> bug for this before, but cannot find it), and my durations tab will only 
> show the table (and not even the whole thing) [5].
I am still seeing this as well.

On 2019-05-02 3:02 p.m., Marcus Hirt wrote:
> This is a great start! Aside from the comments already provided by Alex, 
> here is some feedback:
> 
> * It would be great to be able to select a percentile and be able to do 
> set as focused selection, and add the events for that percentile to the 
> focused selection.

This should be working now. When setting a table row as the focused 
selection, all events with duration at least as long as the percentile 
values in that row will be selected.

e.g. Setting the 99th percentile row as the focused selection, with read 
duration of 10ms and write duration of 50ms, will only show read events 
 >= 10ms and write events >= 50ms.

> * The current UI splits read and writes in two different lanes, perhaps 
> that would make sense here too, also for selection purposes. And just as 
> with the lanes, only show when there is available data.

With this revised patch, when there are no matching read or write 
events, the corresponding columns will not be shown. There is a slight 
issue when columns are hidden and then reappear, the last column is too 
large and needs be resized manually. I can reproduce this in other 
tables by hiding two columns, and restoring them. I'm not sure of the cause.

> * Perhaps the counts shown should be the number of events in the 
> percentile or above, as those would be the number of events selected - 
> i.e. if you select events in the 99.9 percentile, you probably don't 
> want all of the events up to and including the ones in the 99.9th 
> percentile, but rather the outliers, the ones in 99.9 and above.

Makes sense to me, the event counts now show events with duration 
greater or equal to the value in that row.

> * It would be cool to add a normalized backdrop in the tables to get a 
> rough visual representation of how many of the events are actually in 
> the different buckets.

I have added this as well.

Here is a revised webrev that should address all of the feedback 
received so far.

Bug: https://bugs.openjdk.java.net/browse/JMC-5327
Webrev: http://cr.openjdk.java.net/~ebaron/JMC-5327/webrev.0/
Preview: https://imgur.com/a/oqX0fai

Thanks,
Elliott