1091140 - telemetry for time spent in sanitize.js

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

We currently don't have any telemetry measurements for time spent clearing the various items that sanitize.js clears. This would be useful for determining which are the most pressing Forget button/Clear Recent History performance problems in the wild.

I think we could just go ahead and sprinkle some TelemetryStopwatch measurements throughout the file to get data here. Let's try to uplift that to beta, too.

Comment 1

[Neil Deakin]

Attachment: Something like this

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8544651 [details] [diff] [review]
Something like this

Looks good to me! Vladan should probably review too.

Comment 3

[Vladan Djeric (:vladan)]

Comment on attachment 8544651 [details] [diff] [review]
Something like this

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

How hard would it be to add a histogram for total time spent clearing history after the Forget button was clicked?

::: toolkit/components/telemetry/Histograms.json
@@ +7070,5 @@
>      "keyed": true,
>      "description": "Counts of plugin and content process crashes which are reported with a crash dump."
> +  },
> +  "FX_SANITIZE_CACHE": {
> +    "expires_in_version": "default",

- Set an expiry date here, use "never" if you really think this histogram will be useful long-term. This seems more like an experiment that will not last more than a few versions, but it's your call. "default" means "this histogram was defined before the concept of expiry dates came about, its expiry version still needs to be set". It's basically a TODO, we should have picked a better name than "Default" :)
- Also add the "alert_emails" field so you get automatic notifications of regressions & improvements in these histograms. Point it to the fx-team Telemetry notification email, or create a new email alias for that if one doesn't exist already.

Comment 4

[Neil Deakin]

(In reply to Vladan Djeric (:vladan) from comment #3)
> - Set an expiry date here, use "never" if you really think this histogram
> will be useful long-term. This seems more like an experiment that will not
> last more than a few versions, but it's your call. "default" means "this
> histogram was defined before the concept of expiry dates came about, its
> expiry version still needs to be set". It's basically a TODO, we should have
> picked a better name than "Default" :)
> - Also add the "alert_emails" field so you get automatic notifications of
> regressions & improvements in these histograms. Point it to the fx-team
> Telemetry notification email, or create a new email alias for that if one
> doesn't exist already.

Hopefully Gavin can answer these two questions. 1) What expiry to use. 2) What email to use

Comment 5

[:Gavin Sharp [email: [e-mail]]]

There's no fx-desktop alias set up, but you can use my email address and [e-mail].

For now I guess you can use Firefox 40 as the expiry.

Comment 6

[Neil Deakin]

Attachment: Telemetry for sanitize, Address comments

Comment 7

[Vladan Djeric (:vladan)]

Comment on attachment 8546960 [details] [diff] [review]
Telemetry for sanitize, Address comments

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

::: toolkit/components/telemetry/Histograms.json
@@ +7147,5 @@
> +    "kind": "exponential",
> +    "high": "30000",
> +    "n_buckets": 20,
> +    "extended_statistics_ok": true,
> +    "description": "Sanitize: Total time it takes to sanitize"

add the unit of time (ms) to the description :)

Comment 8

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/05dfcf108b4f

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/05dfcf108b4f