1267904 - Add telemetry for WorkerMainThreadRunnable

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[(no longer active)]

We need to start collecting telemetry for MainThreadWorkerControlRunnable to measure how long various things that we proxy to the main thread take in the wild.  Based on this we can get information about what cases are real world performance issues that we need to fix.

I'm imagining that we'd make the consumer of MainThreadWorkerControlRunnable pass in an identifier of some sort so that we can differentiate between the telemetry histograms for all of the consumers, both existing ones and the ones that we add in the future.  The ID can just be the histogram ID.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I don't know what you think you are measuring but MainThreadWorkerControlRunnable is almost certainly not what you want.

Comment 2

[(no longer active)]

The goal is to measure how long it takes to proxy work to the main thread and get the results back on the worker side.  Baku suggested this class, if this isn't the right place can you please update the summary to make it point to the right thing?  Thanks!

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Well there's not just one class to look at ...  But WorkerControlRunnables are used for bookkeeping, not script observable functionality, and MainThreadWorkerControlRunnable is just a WorkerControlRunnable that can be dispatched from the main thread (instead of the parent thread).

I'd start by looking at every time we spin up a sync loop, but there is probably more than just that do be done.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: tele.patch

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8746172 [details] [diff] [review]
tele.patch

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

::: dom/workers/WorkerRunnable.cpp
@@ +585,5 @@
>  WorkerMainThreadRunnable::Dispatch(ErrorResult& aRv)
>  {
>    mWorkerPrivate->AssertIsOnWorkerThread();
>  
> +  TimeStamp startTime = TimeStamp::Now();

NowLoRes?

::: dom/workers/WorkerRunnable.h
@@ +444,5 @@
>  
>  private:
>    NS_IMETHOD Run() override;
> +
> +  const nsCString mTelemetryKey;

Place this with the other member variables above please.

@@ +464,1 @@
>    {}

Please move this ctor out of line into WorkerRunnable.cpp

::: toolkit/components/telemetry/Histograms.json
@@ +10594,1 @@
>    }

I don't know enough about this to know whether this is right or not ... but it looks reasonable ...

Please add [e-mail] to the alert_emails.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/08f3c7a9f5a8

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/08f3c7a9f5a8