951435 - Show thread hang stats in about:telemetry

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

Thread hang stats have been turned on since 28, and about:telemetry should show these data.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Add exponential option to histogram bar graph (v1)

Thread activity histograms are best visualized in a log plot. This patch adds an exponential option when rendering histograms.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Add thread hang stats to about:telemetry (v1)

This patch adds a section in about:telemetry to display data from nsITelemetry::threadHangStats (http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/nsITelemetry.idl#102)

Comment 3

[Vladan Djeric (:vladan)]

Comment on attachment 8356761 [details] [diff] [review]
Add exponential option to histogram bar graph (v1)

Review of attachment 8356761 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/aboutTelemetry.js
@@ +534,5 @@
>     *
>     * @param aDiv Outer parent div
>     * @param aValues Histogram values
>     * @param aMaxValue Value of the longest bar (length, not label)
>     * @param aSumValues Sum of all bar values

* @param aOptions ...

Comment 4

[Vladan Djeric (:vladan)]

Comment on attachment 8356762 [details] [diff] [review]
Add thread hang stats to about:telemetry (v1)

Review of attachment 8356762 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/aboutTelemetry.js
@@ +433,5 @@
> +
> +  /**
> +   * Creates and fills data corresponding to a thread
> +   */
> +  renderThread: function(thread) {

aThread

@@ +443,5 @@
> +
> +    Histogram.render(div, thread.name + "/" + this.threadActivityCaption,
> +                     thread.activity, {exponential: true});
> +    thread.hangs.forEach((hang) => {
> +      Histogram.render(div, hang.stack.join("/"), hang.histogram, {exponential: true});

The histogram's name is all the hang stack frames concatenated together? So the overflow just gets clipped by the histogram border? Why not put the hang stack in a separate div below the histogram?

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Add exponential option to histogram bar graph (v1.1)

(In reply to Vladan Djeric (:vladan) from comment #3)
> Comment on attachment 8356761 [details] [diff] [review]
> Add exponential option to histogram bar graph (v1)
> 
> Review of attachment 8356761 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/content/aboutTelemetry.js
> @@ +534,5 @@
> >     *
> >     * @param aDiv Outer parent div
> >     * @param aValues Histogram values
> >     * @param aMaxValue Value of the longest bar (length, not label)
> >     * @param aSumValues Sum of all bar values
> 
> * @param aOptions ...

Added.

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Add thread hang stats to about:telemetry (v2)

(In reply to Vladan Djeric (:vladan) from comment #4)
> Comment on attachment 8356762 [details] [diff] [review]
> Add thread hang stats to about:telemetry (v1)
> 
> Review of attachment 8356762 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/content/aboutTelemetry.js
> @@ +433,5 @@
> > +
> > +  /**
> > +   * Creates and fills data corresponding to a thread
> > +   */
> > +  renderThread: function(thread) {
> 
> aThread

Changed.

> @@ +443,5 @@
> > +
> > +    Histogram.render(div, thread.name + "/" + this.threadActivityCaption,
> > +                     thread.activity, {exponential: true});
> > +    thread.hangs.forEach((hang) => {
> > +      Histogram.render(div, hang.stack.join("/"), hang.histogram, {exponential: true});
> 
> The histogram's name is all the hang stack frames concatenated together? So
> the overflow just gets clipped by the histogram border? Why not put the hang
> stack in a separate div below the histogram?

Changed the patch to add a stack div below the histogram title, inside the histogram div (everything is inside the histogram div to keep layout correct).

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e6d0f23b23c4
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d6ce0f21d10

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e6d0f23b23c4
https://hg.mozilla.org/mozilla-central/rev/4d6ce0f21d10