RFR: JMC-5327: Using HdrHistograms to visualize latencies
Jie Kang
jkang at redhat.com
Tue May 28 17:19:39 UTC 2019
On Mon, May 27, 2019 at 12:10 PM Elliott Baron <ebaron at redhat.com> wrote:
>
> Hi Marcus,
>
> 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.
Hi Elliott,
For what it's worth, I asked a native Chinese speaker to look at the
Chinese translations without knowledge of the English ones and he says
they make sense and was also able to translate them to what was
written in English.
Regards,
>
> Thanks,
> Elliott
>
> >
> > Kind regards,
> > Marcus
> >
> > On Fri, May 24, 2019 at 1:06 AM Elliott Baron <ebaron at redhat.com
> > <mailto:ebaron at redhat.com>> wrote:
> >
> > 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
> >
More information about the jmc-dev
mailing list