RFR: JMC-5327: Using HdrHistograms to visualize latencies
ebaron at redhat.com
Mon May 27 16:09:51 UTC 2019
On 2019-05-24 3:13 p.m., Marcus Hirt wrote:
> Very nice Elliot! This looks great!
Glad to hear it, thanks!
> How sure are you of the Japanese and
> Chinese translations? Would you want for someone to take a look at them?
These come from substituting the word for "percentile" (found online)
into existing localized strings with the same sort of desired phrasing
(e.g. File I/O Histogram Selection). If there is someone who could
verify/improve the translations, I would appreciate it.
> Kind regards,
> On Fri, May 24, 2019 at 1:06 AM Elliott Baron <ebaron at redhat.com
> <mailto:ebaron at redhat.com>> wrote:
> 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
> > 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
> > whereas the other tables seems to adjust (gif) . 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
> > 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
> > 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
>  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) .
> 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
> 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,
> > 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
More information about the jmc-dev