RFR: JMC-5327: Using HdrHistograms to visualize latencies
marcus.hirt at datadoghq.com
Mon May 27 18:00:11 UTC 2019
Can you help verify the Japanese translation?
On Mon, May 27, 2019 at 6:09 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.
> > 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) . 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
> >  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
> > 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