Adjust histograms for TIME_TO_FIRST_(Interaction) probes

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Harald, Assigned: wchen)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Discussed in #perf a bit what to do about the overflowing last buckets in all histograms

https://mzl.la/2iPVBNC

Observations:

1. The last bucket to 100s is taking in all the overflowing values
2. The 100s limit: Comparing TOTAL_CONTENT_PAGE_LOAD_TIME (30s max) and TIME_TO_NON_BLANK_PAINT_MS (100s max), 30s gives a good enough limit for page load. Going from there, 50s would be a good limit for page load interactions.
3. The exponential scale becomes too coarse after a 5s+, loosing information for an important important range of page load

Steps to make this histogram more useful:

1. Change max value to 50s and drop values > 50s (don't record them)
2. Change n_buckets to 100
3. Rename probes to *_MS, as we change the buckets to avoid data collision
(Reporter)

Comment 1

2 years ago
:wchen, any concerns with this approach to fix the histograms?
Flags: needinfo?(wchen)
(Reporter)

Comment 2

2 years ago
Dominik, r? my proposal for fixing the histograms.
Flags: needinfo?(dstrohmeier)
r+

I also see the need to have a more fine resolution in the low range. High values, as Harald suggests, don't have much meaning when we try to correlate first interaction to page load. I'd say that we could even go down to 30s as max value. But with the exponential scale, there is enough detail in the low values and acceptable loss of resolution for larger values.
Flags: needinfo?(dstrohmeier)
(Reporter)

Comment 4

2 years ago
Georg, for histogram changes to avoid coming up with creative post-fixes; should we just add _V1 (and so on) to iterate on probes? I could not find any best practice in past bugs that changes probes that really made this clear and the overhead of changing probe names could add stop energy for adjusting settings for existing probes.
Flags: needinfo?(gfritzsche)
Yes, the current approach is to use versioning in the name, post-fixing with _2, _3, etc. (best to start with _2 to avoid confusion).
In the future we want to handle this a bit better, see bug 1322802.

FWIW, we have a histogram simulator tool: https://telemetry.mozilla.org/histogram-simulator
Flags: needinfo?(gfritzsche)
(Assignee)

Comment 6

2 years ago
Created attachment 8832347 [details] [diff] [review]
Adjust telemetry histograms for time to first input probes.

In addition to the changes in c0, I've also done the following:

- Started the timing from first non-blank paint instead of first paint (as requested here: https://bugzilla.mozilla.org/show_bug.cgi?id=1307675#c18)
- Record the time to when the input events were created instead of just before the events are dispatched to JS, that way we get a bit closer the the time the user initiated the input.
- Record the time to the first of any type of interaction (as requested here: https://bugzilla.mozilla.org/show_bug.cgi?id=1307675#c24)
Assignee: nobody → wchen
Flags: needinfo?(wchen)
Attachment #8832347 - Flags: review?(amarchesini)
Comment on attachment 8832347 [details] [diff] [review]
Adjust telemetry histograms for time to first input probes.

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

I'm not layout peer.
Attachment #8832347 - Flags: review?(amarchesini) → review?(jwatt)
(Reporter)

Comment 8

2 years ago
As Georg recommended, can we please change the new probe names from *_MS to *_2?
Flags: needinfo?(wchen)

Comment 9

2 years ago
If you do that, please make sure that the probe description specifies that everything is measured in millisecs.
(Reporter)

Comment 10

2 years ago
We could also do _MS_2 to make that clear.
Comment on attachment 8832347 [details] [diff] [review]
Adjust telemetry histograms for time to first input probes.

wchen: can you take account of comments 8, 9 and 10. f+ for the layout parts, but you really need a Telemetry peer review for this.

https://wiki.mozilla.org/Modules/Toolkit#Telemetry
Attachment #8832347 - Flags: review?(jwatt) → feedback+
(Assignee)

Comment 12

2 years ago
Created attachment 8835029 [details] [diff] [review]
Adjust telemetry histograms for time to first input probes. v2

Adding telemetry peer review per comment 11

data review: This patch changes the telemetry parameters according to comment 0 to make the time to first interaction histograms more useful. It also adds a new probe to measure the time from non-blank paint to any of the currently recorded interaction types as requested in https://bugzilla.mozilla.org/show_bug.cgi?id=1307675#c24
Flags: needinfo?(wchen)
Attachment #8835029 - Flags: review?(chutten)
Attachment #8835029 - Flags: feedback?(benjamin)
Comment on attachment 8835029 [details] [diff] [review]
Adjust telemetry histograms for time to first input probes. v2

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

Two small things

::: toolkit/components/telemetry/Histograms.json
@@ +10576,5 @@
>      "kind": "enumerated",
>      "n_values": 4,
>      "description": "Counts the number of asm.js vs WebAssembly modules instanciations, at the time modules are getting instanciated."
>    },
> +  "TIME_TO_FIRST_CLICK_MS_2": {

If TIME_TO_FIRST_CLICK_MS has never been submitted to the telemetry servers, you can just use it without the _2 suffix.

@@ +10581,2 @@
>      "alert_emails": ["hkirschner@mozilla.com"],
> +    "bug_numbers": [1332511],

bug_numbers is an array, so you can include 1307675 as well as 1332511.
Attachment #8835029 - Flags: review?(chutten)
Comment on attachment 8835029 [details] [diff] [review]
Adjust telemetry histograms for time to first input probes. v2

data-r=me
Attachment #8835029 - Flags: feedback?(benjamin) → feedback+
Comment on attachment 8835029 [details] [diff] [review]
Adjust telemetry histograms for time to first input probes. v2

Okay, and layout r+ from me then.
Attachment #8835029 - Flags: review+

Comment 16

2 years ago
Pushed by wchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/30b4ee186ad8
Adjust telemetry histograms for time to first input probes. r=jwatt, data=bsmedberg

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/30b4ee186ad8
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.