David Teller [:Yoric] - still alive but not very active Reporter
	Description • 11 years ago

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.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 1 • 11 years ago

PerformanceTiming is something else entirely (it's performance.timing).

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 2 • 10 years ago

Ms2ger, are you interested in doing this?

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 3 • 10 years ago

I probably can. Dunno when I'll have time, though.

	:Irving Reid (No longer working on Firefox)
	Comment 4 • 10 years ago

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?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 5 • 10 years ago

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.

	Jukka Jylänki
	Comment 6 • 10 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

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 10 • 10 years ago

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.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 11 • 10 years ago

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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 12 • 10 years ago

>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.  ;)

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 13 • 10 years ago

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)

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 14 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/f79c41150d2b
https://hg.mozilla.org/mozilla-central/rev/02d549361c6d
https://hg.mozilla.org/mozilla-central/rev/42c3c548cdb3

	Sylvestre Ledru [:Sylvestre]
	Comment 16 • 10 years ago

"Performance.now() for workers implemented" => added to the release notes.

	Jean-Yves Perrier [:teoli]
	Comment 17 • 10 years ago

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, …)

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 18 • 10 years ago

> - 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.

	Jean-Yves Perrier [:teoli]
	Comment 19 • 10 years ago

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

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 20 • 10 years ago

> 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..

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 21 • 10 years ago

And thank you for documenting this!

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 22 • 10 years ago

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.