If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[Workers] Implement Performance.now() for workers

RESOLVED FIXED in mozilla34

Status

()

Core
DOM: Workers
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Yoric, Assigned: bz)

Tracking

({dev-doc-complete})

unspecified
mozilla34
x86
Mac OS X
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox 34+)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Since we have PerformanceTiming.now() for high-precision timing on the main thread, it would be nice to have the same feature for workers.

Investigation is in progress but I suspect that discrepancies in Date.now() on a worker are causing bug 874425.
Whiteboard: [Async]
PerformanceTiming is something else entirely (it's performance.timing).
Summary: [Workers] Implement PerformanceTiming for workers → [Workers] Implement Performance.now() for workers
Keywords: dev-doc-needed

Updated

4 years ago
Whiteboard: [Async]
Ms2ger, are you interested in doing this?
Flags: needinfo?(Ms2ger)
I probably can. Dunno when I'll have time, though.
Flags: needinfo?(Ms2ger)

Updated

4 years ago
Flags: needinfo?(Ms2ger)
It would be very useful to have performance.now() for all chrome code, not just workers - telemetry suffers greatly from non-monotonic Date.now(). Should we scope-creep this bug, or file another?
Another.  The change to hang a performance object off a random non-WebIDL mainthread global is quite different from doing it off a worker global.
Blocks: 969490

Comment 6

3 years ago
This would be very useful for measuring performance in SharedArrayBuffer+Mutex work in bug https://bugzilla.mozilla.org/show_bug.cgi?id=979594
Created attachment 8476950 [details] [diff] [review]
part 1.  Make [Exposed] work correctly on jsonifiers and stringifiers and don't create an unforgeable toJSON just because we have a jsonifier
Attachment #8476950 - Flags: review?(khuey)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Created attachment 8476952 [details] [diff] [review]
part 2.  Implement performance.now() on workers
Attachment #8476952 - Flags: review?(khuey)
Created attachment 8476954 [details] [diff] [review]
part 2.  Implement performance.now() on workers
Attachment #8476954 - Flags: review?(khuey)
Attachment #8476952 - Attachment is obsolete: true
Attachment #8476952 - Flags: review?(khuey)
Flags: needinfo?(Ms2ger)
Comment on attachment 8476950 [details] [diff] [review]
part 1.  Make [Exposed] work correctly on jsonifiers and stringifiers and don't create an unforgeable toJSON just because we have a jsonifier

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

::: dom/bindings/Codegen.py
@@ +2128,5 @@
>          if not static:
>              stringifier = descriptor.operations['Stringifier']
>              if (stringifier and
> +                unforgeable == MemberIsUnforgeable(stringifier, descriptor) and
> +                isMaybeExposedIn(stringifier, descriptor)):

The continuation should be over on the right (aligned with the other args).

@@ +2143,5 @@
>                      self.regular.append(toStringDesc)
>              jsonifier = descriptor.operations['Jsonifier']
> +            if (jsonifier and
> +                unforgeable == MemberIsUnforgeable(jsonifier, descriptor) and
> +                isMaybeExposedIn(jsonifier, descriptor)):

here too.
Attachment #8476950 - Flags: review?(khuey) → review+
Comment on attachment 8476954 [details] [diff] [review]
part 2.  Implement performance.now() on workers

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

Ideally we would run whatever test we have for performance.now in a window inside a worker too ...

::: dom/workers/WorkerScope.cpp
@@ +55,5 @@
>                                                    DOMEventTargetHelper)
>    tmp->mWorkerPrivate->AssertIsOnWorkerThread();
>    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mConsole)
> +  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPerformance)
> +  // XXXbz what about mLocation and mNavigator?

Uh, excellent point.  File a bug, please?

::: dom/workers/test/test_worker_interfaces.js
@@ +97,5 @@
>  // IMPORTANT: Do not change this list without review from a DOM peer!
>      "MessagePort",
>  // IMPORTANT: Do not change this list without review from a DOM peer!
> +    "Performance",
> +// IMPORTANT: Do not change this list without review from a DOM peer!

I totally missed where we created this test.  It's beautiful.
Attachment #8476954 - Flags: review?(khuey) → review+
>The continuation should be over on the right (aligned with the other args).
...
>here too.

Ignored, per irc.

> Ideally we would run whatever test we have for performance.now in a window inside a
> worker too ...

Done.

> Uh, excellent point.  File a bug, please?

Bug 1060621.

> I totally missed where we created this test. 

bkelly rocks.  ;)
Created attachment 8481756 [details] [diff] [review]
Followup to fix randomorange in test_eventTimeStamp.html

Landed this, but would like a once-over review for it.  Note that this caught a bug: in the WorkerPrivateParent ctor, we need to use mLoadInfo, not aLoadInfo, since we already did mLoadInfo.StealFrom(aLoadInfo)
Attachment #8481756 - Flags: review?(khuey)
https://hg.mozilla.org/integration/mozilla-inbound/rev/f79c41150d2b
https://hg.mozilla.org/integration/mozilla-inbound/rev/02d549361c6d
https://hg.mozilla.org/integration/mozilla-inbound/rev/42c3c548cdb3
Flags: in-testsuite+
Target Milestone: --- → mozilla34
Keywords: relnote
https://hg.mozilla.org/mozilla-central/rev/f79c41150d2b
https://hg.mozilla.org/mozilla-central/rev/02d549361c6d
https://hg.mozilla.org/mozilla-central/rev/42c3c548cdb3
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
"Performance.now() for workers implemented" => added to the release notes.
relnote-firefox: --- → 34+
Keywords: relnote
In order to document this, I have a few assertions to check:
- This bug makes Performance (but only with now() ) available on WorkerGlobalScope — and therefore (Shared|ServiceWorker)GlobalScope —, the global objects of worker
- Performance.now() — introduced in Fx 15 — is the only method/attribute available to both workers and non-worker context. All the other method/attributes are only available on non-worker contexts.

Is there a plan, or a wish, to make the rest of Performance available to workers in the future? If not, why not creating a WorkerPerformance interface (like WorkerNavigator, WorkerLocation, …)
Flags: needinfo?(bzbarsky)
> - This bug makes Performance (but only with now() ) available on WorkerGlobalScope — and
> therefore (Shared|ServiceWorker)GlobalScope —, the global objects of worker

Correct, this is exposed in all worker types.  The way the 0 time is determined is different for different worker types, though.  Specifically:

1)  A shared or service worker uses the worker start time as 0.
2)  A dedicated worker started from a Window uses that window's navigationStart as a
    0 (like performance.now() in a window).
3)  A dedicated worker started from another worker uses that other worker's 0 time as
    its 0 time.

> Performance.now() — introduced in Fx 15 — is the only method/attribute available to both
> workers and non-worker context.

Correct.

> Is there a plan, or a wish, to make the rest of Performance available to workers in the
> future? 

Not so far.

> If not, why not creating a WorkerPerformance interface (like WorkerNavigator,
> WorkerLocation

Because those interfaces were a specification hackaround for the problem of not having [Exposed] in IDL when they were defined.  Now that we have [Exposed] we are no longer adding worker-specific interfaces if we just want to subset functionality.
Flags: needinfo?(bzbarsky)
Thank you very much, I have adapted the way we document interfaces to be able to indicate attribute/methods not exposed to workers.

Doc updated:
https://developer.mozilla.org/en-US/docs/Web/API/Performance
https://developer.mozilla.org/en-US/docs/Web/API/Performance.now
https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope.performance
https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope
and
https://developer.mozilla.org/en-US/Firefox/Releases/34
Keywords: dev-doc-needed → dev-doc-complete
> I have adapted the way we document interfaces to be able to indicate attribute/methods
> not exposed to workers

Note that there can also be things exposed to workers but not windows. And things exposed to some kinds of workers but not other kinds..
And thank you for documenting this!
Comment on attachment 8481756 [details] [diff] [review]
Followup to fix randomorange in test_eventTimeStamp.html

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

Nice.  Tests ftw.
Attachment #8481756 - Flags: review?(khuey) → review+
You need to log in before you can comment on or make changes to this bug.