Closed Bug 1328398 Opened 7 years ago Closed 6 years ago

Add telemetry to measure how long the Service Worker takes to start up

Categories

(Core :: DOM: Service Workers, defect, P3)

Product:
Core
Component:
DOM: Service Workers
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox53 --- affected
firefox57 --- fixed

People

(Reporter: overholt, Assigned: tt)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1328398 - Add telemetry to measure service worker launch time. datareview=francois
59 bytes, text/x-review-board-request
bkelly
: review+
Details 
      No description provided.
We track the information for this now, I think, in "LaunchServiceWorker" time fields.  All we need to do is add the probes.

HoPang, do you have any time available to do this?
(In reply to Ben Kelly [:bkelly] from comment #1)
> We track the information for this now, I think, in "LaunchServiceWorker"
> time fields.  All we need to do is add the probes.
> 
> HoPang, do you have any time available to do this?

I am also CC Tom here and see if he has time to help (feel free to let me know if you don't, please).
Hi Tom,

I am kind of occupied by alternative data stuff. It would be great if you can help, do you mind taking this?
Flags: needinfo?(ttung)
Steal this from Ho-Pang :P
Assignee: nobody → ttung
Flags: needinfo?(ttung)
Hi Ben,

This patch is to measure the launch time for a SW. Could you help me to review it? Thanks! 

I'll send it to data-review after getting r+ from you.
Comment on attachment 8898583 [details]
Bug 1328398 - Add telemetry to measure service worker launch time.  datareview=francois

https://reviewboard.mozilla.org/r/169608/#review175312

::: dom/workers/ServiceWorkerPrivate.cpp:1512
(Diff revision 1)
>    WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
>    {
>      MOZ_ASSERT(aWorkerPrivate);
>  
>      if (mMarkLaunchServiceWorkerEnd) {
> -      mInterceptedChannel->SetLaunchServiceWorkerEnd(TimeStamp::Now());
> +      TimeStamp launchStartTime = TimeStamp();

nit: You can just write this as `TimeStamp launchStartTime;`.  No need for the assignment.

::: toolkit/components/telemetry/Histograms.json:11160
(Diff revision 1)
>      "alert_emails": ["safebrowsing-telemetry@mozilla.org"],
>      "bug_numbers": [1058133]
>    },
> +  "SERVICE_WORKER_LAUNCH_TIME": {
> +    "record_in_processes": ["main", "content"],
> +    "expires_in_version": "61",

I think we should make the case that this probe should not expire.  Large sites care a lot about SW performance and in particular the possible delay from starting the worker.  We need to monitor this for the foreseeable future.
Attachment #8898583 - Flags: review?(bkelly) → review+
(In reply to Tom Tung [:tt] from comment #8)
> Comment on attachment 8898583 [details]
> Bug 1328398 - Add telemetry to measure service worker launch time.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/169608/diff/1-2/

data-collection review
Comment on attachment 8898583 [details]
Bug 1328398 - Add telemetry to measure service worker launch time.  datareview=francois

https://reviewboard.mozilla.org/r/169608/#review176442

This is [Category 1](https://wiki.mozilla.org/Firefox/Data_Collection#Data_Collection_Categories) data.

datareview+
Attachment #8898583 - Flags: review?(francois) → review+
(In reply to Ben Kelly [:bkelly] from comment #7)
(In reply to François Marier [:francois] from comment #10)
Thanks for the review!
Pushed by ttung@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cd6b4d4a1bdc
Add telemetry to measure service worker launch time. r=bkelly,francois datareview=francois
https://hg.mozilla.org/mozilla-central/rev/cd6b4d4a1bdc
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.