What interface do you mean? Performance.timing? That's not available in workers at all.

I guess you mean this: http://www.w3.org/TR/user-timing/ Yea, not on workers at all yet.

(In reply to Ben Kelly [:bkelly] from comment #2) > I guess you mean this: > > http://www.w3.org/TR/user-timing/ > > Yea, not on workers at all yet. Exactly. I was trying to run Raptor [1] tests to measure service workers performance. [1] https://developer.mozilla.org/en-US/Firefox_OS/Automated_testing/Raptor

Correct, User Timing relies on DOM timing APIs, and hence isn't available in service workers. CC-ing qDot in case he has anything else to add, since he wrote the original implementation.

I don't think user-timing is spec'd to be on workers. We are somewhat non-spec conformant in that we allow Performance.now() on workers.

Yeah, user timing is a DOM API, so workers can't use it. bkelly's suggestion of Performance.now() is probably the best we got for the moment.

Per spec, even performance.now() is supposed to not exist in workers. But that's because the spec is written by a working group that's ... yeah. Anyway. It would make some sense to support user timing in workers if we can figure out how it should work...

Yeah, there seems to be some consensus that we could shuffle times back and forth from a worker since the API is not hitting nodes. Will at least reopen since it's feasible.

Is the goal here to be able to use Performance.mark() on both the main thread and the service worker so that Performance.measure() can be used to measure the latency from the main thread to the SW and vice versa?

I'll take this. Hopefully have something landed mid-May.

Comment 9 is a good question... because service workers and mainthread don't have a common 0 time, so you can't compare marks between them.

I think we will have to do some kind of time normalization when the values are read by a script.

I think whoever wants to implement this should talk to the webperf wg to figure out what the heck their plans are here. Assuming they have any.

Eli, can you outline how you want this to work for your Raptor framework? Note that SharedWorkers and ServiceWorkers currently have a zero-time corresponding to worker startup, not the navigation start for the window.

I've tried to keep Raptor pretty agnostic to the types of tests needing to be written. Redirecting to Fernando as he is writing the test.

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #9) > Is the goal here to be able to use Performance.mark() on both the main > thread and the service worker so that Performance.measure() can be used to > measure the latency from the main thread to the SW and vice versa? I think the goal is to have an independent Performance.mark and Performance.measure for the Worker. Especially since trying to associate Performance.mark from the main thread and the worker is a bit tricky as there could be multiple context using the Service Worker (or a SharedWorker).

Having Performance.mark and Performance.measure on the worker scope would be a good start indeed. But it would be great if we could also be able to do cross thread measurements. I am probably missing a lot here, but I was wondering if it would be possible to expose a way to modify the zero time of a scope. Or as an alternative to that, to add an extra argument on Performance.mark indicating an offset between scopes.

Changing the signature on a standard method for this intent seems like a violation of concerns. Measuring A difference between 2 points not designated to navigationStart seems better suited to performance.measure.

I propose we expose Performance.measure/mark on Worker, but keep it isolated to the worker. The marks are not shared with marks from the window. I think this is consistent with how these methods work between a window and an iframe. We could then add something to get an absolute time reference for the zero-time. You could then use postMessage() to communicate the measured values and the time offset back to the window. Thoughts?

I'm ok with this solution. Thank you!

Ok, I will likely start working on this when I get back from PTO around May 13.

I'm working on this API. bkelly, I'm stealing this bug from you.

mochitests are missing. I'll add it and then I'll ask for a review.

Has there been an intent to implement/ship here? Has someone actually contacted the public-web-perf working group?

Comment on attachment 8607468 [details] [diff] [review] timing.patch Review of attachment 8607468 [details] [diff] [review]: ----------------------------------------------------------------- r- mostly because of the way you're delegating Traverse/Unlink to the base class. ::: dom/base/PerformanceEntry.cpp @@ +27,4 @@ > mName(aName), > mEntryType(aEntryType) > { > + // mParent is null in workers. Can you lease convert this into a MOZ_ASSERT? ::: dom/base/PerformanceMark.cpp @@ +17,2 @@ > { > + // mParent is null in workers. Same. ::: dom/base/PerformanceMeasure.cpp @@ +17,4 @@ > mStartTime(aStartTime), > mDuration(aEndTime - aStartTime) > { > + // mParent is null in workers. Ditto. ::: dom/base/nsPerformance.cpp @@ +401,5 @@ > > > NS_IMPL_CYCLE_COLLECTION_CLASS(nsPerformance) > > NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsPerformance, DOMEventTargetHelper) Don't you need to change the base class here? @@ +409,5 @@ > tmp->mMozMemory = nullptr; > mozilla::DropJSObjects(this); > + > + // Unlink method from the PerformanceBase class. > + tmp->Unlink(); Why not make this work the same was as all other cycle collectible classes? @@ +419,3 @@ > mParentPerformance) > + // Traverse method from the PerformanceBase class. > + tmp->Traverse(cb); Ditto. @@ +777,5 @@ > +{} > + > +void > +PerformanceBase::GetEntries(nsTArray<nsRefPtr<PerformanceEntry>>& aRetval) > +{ I'm assuming most of this stuff is just moving the code. If you are changing the implementation here as well, can you please point me to the specific places? Thanks! @@ +989,5 @@ > + } > +} > + > +void > +PerformanceBase::Unlink() This and Traverse will go away, hopefully. ::: dom/base/nsPerformance.h @@ +394,5 @@ > > virtual JSObject* WrapObject(JSContext *cx, JS::Handle<JSObject*> aGivenProto) override; > > // Performance WebIDL methods > + virtual DOMHighResTimeStamp Now() const override; Please drop the virtual keyword. @@ +420,5 @@ > > private: > ~nsPerformance(); > + > + virtual nsISupports* GetAsISupports() override Same, and in other occurrences below. @@ +425,5 @@ > + { > + return this; > + } > + > + virtual void InsertUserEntry(PerformanceEntry* aEntry); You should mark this as override too. ::: dom/base/test/test_performance_user_timing.js @@ +1,1 @@ > +var steps = [ Also assuming this is just moving the code as well. ::: dom/webidl/Performance.webidl @@ +29,5 @@ > jsonifier; > }; > > // http://www.w3.org/TR/performance-timeline/#sec-window.performance-attribute > +[Exposed=(Window,Worker)] Please send an intent to implement and ship. ::: dom/workers/Performance.cpp @@ +70,5 @@ > + if (aTime == 0) { > + return 0; > + } > + > + return aTime - mWorkerPrivate->NowBaseTimeHighRes(); This is probably worth mentioning in the intent email. ::: dom/workers/Performance.h @@ +43,5 @@ > return nullptr; > } > > // WebIDL (public APIs) > + virtual DOMHighResTimeStamp Now() const override; Please drop virtual in the appropriate places in this file too.

Thanks for the review. The changes I did in the code are: 1. I removed mWindow in nsPerformance and replaced it with GetOwner() 2. some MOZ_ASSERT for main-thread only methods (something that is not exposed to workers) 3. in the test I implemented a check for navigationStart 4. My patch uses DOMHighResTimeStamp in DeltaFromNavigationStart() and other methods instead DOMTimeMilliSec otherwise this code: return aTime - GetDOMTiming()->GetNavigationStart(); would return -0.something instead 0. This was an existing bug and we didn't have any test at all for it. I added some. 5. nsPerformance::IsEnabled()

Release Note Request (optional, but appreciated) [Why is this notable]: It's an useful API to write benchmarks, tests and so on in workers. [Suggested wording]: User Timing API exposed to workers

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #29) > > + virtual void InsertUserEntry(PerformanceEntry* aEntry); > > You should mark this as override too. Looks like this review comment was missed -- the subclass version of this method (in nsPerformance) needs an 'override' annotation, or else clang 3.6 & newer complains. Landed a followup to add the annotation, with blanket r+ that ehsan granted me for fixes of this sort over in bug 1126447 comment 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/3ceaaa0aec90

Note added to: https://developer.mozilla.org/en-US/docs/Web/API/Performance Also added to: https://developer.mozilla.org/en-US/docs/Web/API/Worker/Functions_and_classes_available_to_workers#APIs_available_in_workers I have opened a new card on the MDN Trello board for documenting the User Timing/Performance Timeline APIs, which specify the currently-undocumented parts of Performance.