Closed Bug 1381777 Opened 2 years ago Closed 2 years ago

Record how far over budget GC slices go

Categories

(Core :: JavaScript: GC, enhancement)

55 Branch
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: jonco, Assigned: jonco)

Details

(Whiteboard: [qf])

Attachments

(1 file)

We currently have telemetry for how long GC slices take, but now we are getting irregular slice budgets due to these being scheduled in idle time this is less useful.

We should record telemetry about how far over the budget we go.
Patch to record how far we went over the budget in microseconds (because we'd like to have greater accuracy than one millisecond).  I set the histogram max to 100ms which would be more than twice the longest slice we expect.

To start with this will report almost all slices as being over budget by some amount.  We should change our budget check to include a small margin and attempt to return to the mutator slightly before the budget expires so that we can hit the budget in most cases.  We can work out what margin to use from the initial telemetry.
Attachment #8887415 - Flags: review?(sphink)
Comment on attachment 8887415 [details] [diff] [review]
bug1381777-record-overrun

Requesting data review for new telemetry histogram.
Attachment #8887415 - Flags: review?(benjamin)
Comment on attachment 8887415 [details] [diff] [review]
bug1381777-record-overrun

microseconds!? That's pretty amazing resolution (not a problem, just surprising).

I presume that this is part of the larger GC/CC metrics project and ongoing alerting/monitoring will be part of that project, so I'm granting data-r=me based on that.
Attachment #8887415 - Flags: review?(benjamin) → review+
(In reply to Benjamin Smedberg [:bsmedberg] from comment #3)
Not promising anything, but we'd like to be able to have budgets in the low single-digits milliseconds, so millisecond granularity seems too coarse.
Comment on attachment 8887415 [details] [diff] [review]
bug1381777-record-overrun

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

::: js/src/jsfriendapi.h
@@ +125,5 @@
>      JS_TELEMETRY_GC_REASON,
>      JS_TELEMETRY_GC_IS_ZONE_GC,
>      JS_TELEMETRY_GC_MS,
>      JS_TELEMETRY_GC_BUDGET_MS,
> +    JS_TELEMETRY_GC_BUDGET_OVERRUN,

It's scary seeing the numeric value of these shift. I guess the stable numbering comes from Histograms.json? It should be commented here that it's ok to change the order. Assuming it is.
Attachment #8887415 - Flags: review?(sphink) → review+
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91467cd8d9d4
Record how long we went over the time budget r=sfink data-r=bsmedberg
https://hg.mozilla.org/mozilla-central/rev/91467cd8d9d4
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.