1052716 - Add telemetry for GGC

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

I expect we'll want pause time in microseconds for the minor gc itself and possibly also time we spend in compacting.

Comment 1

[Terrence Cole [:terrence]]

Attachment: telemetry_ggc-v0.diff

I think this will do what we want.

Comment 2

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8606001 [details] [diff] [review]
telemetry_ggc-v0.diff

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

r=me but according to the letter of https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Adding_a_new_Telemetry_probe it seems like we need a privacy review.

::: js/src/gc/Nursery.cpp
@@ +426,5 @@
>      rt->gc.stats.count(gcstats::STAT_MINOR_GC);
>  
>      TraceMinorGCStart();
>  
> +    int64_t timstampStart_total = PRMJ_Now();

Who is tim and why are you stamping him?

::: toolkit/components/telemetry/Histograms.json
@@ +345,5 @@
> +  },
> +  "GC_MINOR_US": {
> +    "expires_in_version": "never",
> +    "kind": "exponential",
> +    "high": "20000",

Can't we get worse than 20ms? I thought we'd seen seconds before.

Seems like we should at least go to 1s.

Comment 3

[Steve Fink [:sfink] [:s:]]

Request for adding new telemetry probes. This should be a no-brainer; these are well within the umbrella of existing probes.

Comment 4

[Vladan Djeric (:vladan)]

Comment on attachment 8606001 [details] [diff] [review]
telemetry_ggc-v0.diff

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

LGTM

::: toolkit/components/telemetry/Histograms.json
@@ +330,5 @@
>      "high": "500",
>      "n_buckets": 50,
>      "description": "Time spent sweeping slowest compartment SCC (ms)"
>    },
> +  "GC_MINOR_REASON": {

Who should we contact if we notice unexpected changes in this histogram's data? i.e. http://alerts.telemetry.mozilla.org/index.html)
You can either add the "alert_emails" field and point it to some "GC team" alias, or just list the bugzilla component (in the histograms' description field) where we should file bugs

Comment 5

[Terrence Cole [:terrence]]

Oh, that's a neat feature! Guess I can close bug 1164985 then.

Comment 6

[Terrence Cole [:terrence]]

Actually, it turns out that Vladan is filing this bugs by hand, so I've filed a ServiceNow request to create a new mailing list: dev-telemetry-gc-alerts.

Comment 7

[Terrence Cole [:terrence]]

(In reply to Steve Fink [:sfink, :s:] from comment #2)
> Comment on attachment 8606001 [details] [diff] [review]
> telemetry_ggc-v0.diff
> 
> Review of attachment 8606001 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me but according to the letter of
> https://developer.mozilla.org/en-US/docs/Mozilla/Performance/
> Adding_a_new_Telemetry_probe it seems like we need a privacy review.
> ::: toolkit/components/telemetry/Histograms.json
> @@ +345,5 @@
> > +  },
> > +  "GC_MINOR_US": {
> > +    "expires_in_version": "never",
> > +    "kind": "exponential",
> > +    "high": "20000",
> 
> Can't we get worse than 20ms? I thought we'd seen seconds before.
> 
> Seems like we should at least go to 1s.

We only get 50 buckets though, so this does cut into our resolution on the low end, even with exponential scaling. We also don't have enough information here to tell why they're going so long. I guess it's a balance between knowing how long the tail is vs being able to see how normal the distribution is lower down. I guess it's probably better to see how long the tail is here, so I suppose I'll extend this as you suggest.

Comment 8

[Terrence Cole [:terrence]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc668b08e3e8

I'm going to add the "alert_emails" field to every GC telemetry probe at once in bug 1164985.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed84a14db655

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ed84a14db655