Closed Bug 951435 Opened 7 years ago Closed 7 years ago

Show thread hang stats in about:telemetry

Categories

(Toolkit :: Telemetry, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: jchen, Assigned: jchen)

Details

Attachments

(2 files, 2 obsolete files)

Thread hang stats have been turned on since 28, and about:telemetry should show these data.
Thread activity histograms are best visualized in a log plot. This patch adds an exponential option when rendering histograms.
Attachment #8356761 - Flags: review?(vdjeric)
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)
Attachment #8356762 - Flags: review?(vdjeric)
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 ...
Attachment #8356761 - Flags: review?(vdjeric) → review+
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?
Attachment #8356762 - Flags: review?(vdjeric)
Flags: needinfo?(nchen)
(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.
Attachment #8356761 - Attachment is obsolete: true
Attachment #8358424 - Flags: review+
(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).
Attachment #8356762 - Attachment is obsolete: true
Attachment #8358426 - Flags: review?(vdjeric)
Flags: needinfo?(nchen)
Attachment #8358426 - Flags: review?(vdjeric) → review+
https://hg.mozilla.org/mozilla-central/rev/e6d0f23b23c4
https://hg.mozilla.org/mozilla-central/rev/4d6ce0f21d10
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.