Closed Bug 1411632 Opened 2 years ago Closed 2 years ago

Introduce few TIME_TO_* probes for all-the-time active tab, split according if any active tab network load optimizations took effect during the period

Categories

(Toolkit :: Performance Monitoring, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: mayhemer, Assigned: mayhemer)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 8 obsolete files)

(Filing under the same component as bug 1344893)

We would like to run a shield pref study or simply have telemetry for the Necko CDP project (see dep list) on optimization we've introduced specifically for optimizing active tab page load.  These optimizations include limiting background request parallelism and throttling of background and download responses in progress.

These new probes should be captured only for pages that were in the active tab all the time between navigation start and the respective probe hit, same as TIME_TO_NON_BLANK_PAINT_MS does (conditioned with nsDOMNavigationTiming::mDocShellHasBeenActiveSinceNavigationStart.) 

The further split of these probes has to be according if any of the CDP optimizations (when the respective prefs are enabled for them) took effect during the period, so that we can observe if there is any real influence on the active tab page load performance and also see the rate of actual trigger.


To sum:
* TIME_TO_LOAD_EVENT_START_WHILE_ACTIVE_MS will be captured for all-the-time active tabs while none of the CDP optimizations has triggered during the page load time

* TIME_TO_DOM_CONTENT_LOADED_START_WHILE_ACTIVE_MS falls under the same set of conditions

* TIME_TO_LOAD_EVENT_START_WHILE_ACTIVE_AFFECTED_MS will be captured for all-the-time active tabs while at least some of the related CDP optimization triggered

* TIME_TO_DOM_CONTENT_LOADED_START_WHILE_ACTIVE_AFFECTED_MS - same conditions
Attached patch wwip1 (obsolete) — Splinter Review
(doesn't even build)

added:
TIME_TO_LOAD_EVENT_START_ACTIVE_NETOPT_MS
TIME_TO_LOAD_EVENT_START_ACTIVE_MS
TIME_TO_DOM_CONTENT_LOADED_START_ACTIVE_NETOPT_MS
TIME_TO_DOM_CONTENT_LOADED_START_ACTIVE_MS
TIME_TO_NON_BLANK_PAINT_NETOPT_MS
Attached patch wip2 (obsolete) — Splinter Review
builds, but not tested.  there are 6 new probes added.
Attachment #8922043 - Attachment is obsolete: true
Summary: Introduce TIME_TO_{LOAD_EVENT / DOM_CONTENT_LOADED}_START_WHILE_ACTIVE(_AFFECTED)_MS → Introduce few TIME_TO_* probes for all-the-time active tab, split according if any active tab network load optimizations took effect during the period
Attached patch wip3 (obsolete) — Splinter Review
Attachment #8922451 - Attachment is obsolete: true
Attached patch v1 (obsolete) — Splinter Review
- we keep a timestamp when any of the optimizations that (may) affect the active tab load is hit; this happens only on the parent process
- this timestamp is sent with every http channel onstoprequest to the child process; it's perfectly sufficient since any of the probes we collect will be accumulated directly or just after an http channel onstop
- there are 6 new probes added, accompanying 3 existing ones:

TIME_TO_LOAD_EVENT_START_MS
TIME_TO_DOM_CONTENT_LOADED_START_MS
TIME_TO_NON_BLANK_PAINT_MS

Each of the above probes is accompanied with additional two new probes that are captured only when the docshell has been active all the time between navigation start and the event and respectively for when a network optimization has or has not been hit between the navigation start and the event.

This is a base for a shield study testing these active/background-tab network optimizations.
Attachment #8922929 - Attachment is obsolete: true
Attachment #8929090 - Flags: review?(valentin.gosu)
Attachment #8929090 - Flags: review?(rweiss)
Comment on attachment 8929090 [details] [diff] [review]
v1

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

Looks good.
Attachment #8929090 - Flags: review?(valentin.gosu) → review+
Attachment #8929090 - Flags: review?(rweiss) → review?(francois)
Comment on attachment 8929090 [details] [diff] [review]
v1

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

Looks good, the only thing that's missing is a person's email address in the alerts section.

Having a team email is fine, but we now also require that a person be tied to each new probe.

Also, the data review process has changed and we now ask people to answer a few questions: https://wiki.mozilla.org/Firefox/Data_Collection#Step_1:_Submit_Request

Essentially, all you have to do is download the text file from Github (https://github.com/mozilla/data-review/blob/master/request.md), answer the questions in it and attach the completed file to Bugzilla. You can flag me as r? on it and I will complete the rest of the review process.
Attachment #8929090 - Flags: review?(francois) → review-
Attached patch v1.1 (obsolete) — Splinter Review
probe emails update, data r requests will be made as per the previous comment
Attachment #8929090 - Attachment is obsolete: true
Attachment #8932124 - Flags: review+
Attached file request.md
Attachment #8932125 - Flags: review?(francois)
Comment on attachment 8932125 [details]
request.md

1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes, in histograms.json.

2) Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, telemetry pref.

3) If the request is for permanent data collection, is there someone who will monitor the data over time?**

Not permanent.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?  **

Category 1.

5) Is the data collection request for default-on or default-off?

Default-on.

6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc.  See the appendix for more details)?

No.

7) Is the data collection covered by the existing Firefox privacy notice?

Yes.

8) Does there need to be a check-in in the future to determine whether to renew the data?

No, telemetry alerts are fine.
Attachment #8932125 - Flags: review?(francois) → review+
Comment on attachment 8932465 [details] [diff] [review]
v1.1 [merged before bug 1386746]

(needs one more small fix)
Attachment #8932465 - Attachment is obsolete: true
Attached patch v1.2 (obsolete) — Splinter Review
Added some !IsNull() checks before using timestamps to calculate.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=280759b2835f0c4820bbf9a75cc48f69ca49f88a
Attachment #8933709 - Flags: review+
Attachment #8933709 - Attachment is obsolete: true
Attachment #8933929 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ce1cf08082b
Introduce onload and DOMContentLoaded telemetry for active tab and network optimization. r=valentin, r=francois
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4ce1cf08082b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.