Open Bug 1709767 Opened 4 years ago Updated 8 days ago

performance.now() pauses during sleep on Linux and macOS, which is inconsistent with the High Resolution Time spec

Categories

(Core :: Performance, defect)

Firefox 88
Desktop
Linux
defect

Tracking

()

Performance Impact none

People

(Reporter: bugzilla, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fxp])

Attachments

(2 files)

Expected Behavior

Calling performance.now() provides an accurate monotonic clock, including system sleep time. For additional details on the W3C High Resolution Time recommendation and explanation of similar noncompliant behavior in Chrome (on macOS, Linux, Android, and Fuchsia), see Issue #4713 on MDN.

Steps to Reproduce

On a Linux device...

  1. Open a tab, navigate to content, open a browser console, and run performance.now().
  2. Put the device to sleep for a period of time, then wake the device.
  3. In the same open tab, run performance.now(). The difference in timestamps will not include the time that the system was suspended.

Proposed Resolution

It appears that the issue is in TimeStamp_posix.cpp. The implementation of TimeStamp on POSIX systems relies on CLOCK_MONOTONIC, which includes sleep time on macOS 10.12+ but doesn't include sleep time on Linux. Using CLOCK_BOOTTIME on Linux should resolve the issue, but may pose other challenges as discussed in the related issues.

Related Issues

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: General → Untriaged
Product: Firefox → WebExtensions
Component: Untriaged → DOM: Core & HTML
Product: WebExtensions → Core
Component: DOM: Core & HTML → Performance

Olli, any thoughts here?

Flags: needinfo?(bugs)

Jonathan has done a great summary on this today in the WebPerf meeting. A summary is that we generally agree that the clock behaviour should be the same across all implementations and across all platforms, however it's very complex to change the behaviour. Not only due to the compatibilty concerns, but also for things like if you change the behaviour for performance.now(), many other timings which rely on performance.now() are also going to be changed.

Thanks Sean! For what it's worth, during the conversation today, I found it helpful to think about breakage risks in two categories.

  • There are breakage risks from changing browser monotonic clock APIs (e.g., TimeStamp in Firefox). They're used all over the place, and who knows what expectations callers have accumulated over the years. An alternative path forward would be to create a new type of well-defined monotonic clock that ticks during sleep, then use the new clock for Performance and DOMHighResTimeStamp implementations. That's not ideal—it involves developing and maintaining another type of clock, and it involves tracking down all the uses of DOMHighResTimeStamp outside of Performance—but it might be less painful. Firefox already has a version of this concept in AwakeTimeStamp (and, to a lesser extent, Uptime).
  • There are breakage risks from changing the behavior of Performance and DOMHighResTimeStamp for web content. It's a positive sign, though, that there's already correct behavior on major platforms and that hasn't caused test failures or bug reports.
See Also: → 1685502
Whiteboard: [qf-]
Summary: performance.now() pauses during sleep on Linux, which is inconsistent with the High Resolution Time spec → performance.now() pauses during sleep on Linux and macOS, which is inconsistent with the High Resolution Time spec

Thanks to Paul for flagging that macOS is still using TimeStamp_darwin.cpp, because the switch to TimeStamp_posix.cpp was backed out. After a little more testing, I was able to reproduce this behavior in Firefox 88.0.1 on macOS 11.3.1. (I suspect I didn't spot the issue in prior testing because I didn't wait long enough in system sleep; macOS was in a light sleep mode where the clock continued to tick.) I've updated the title to include macOS.

Attached patch patch.diffSplinter Review

Here's an initial implementation of a TimeStamp variation that ticks during sleep on all platforms. If this direction seems reasonable, the next step would be updating performance.now() and all the other DOMHighResTimeStamp uses. I haven't included FuzzyFox support since that's getting removed (Bug 1666222).

Assignee: nobody → bugzilla
Assignee: bugzilla → nobody
Flags: needinfo?(padenot)
Flags: needinfo?(gsvelto)

Gabriele asked about replacing TimeStamp entirely, rather than implementing a new class. I completely agree with the end goal; there should be no ambiguity about expected system sleep behavior. In the near term, I lean toward implementing a new class, for a couple reasons.

  • TimeStamp is used throughout the Mozilla codebase for a wide variety of purposes, with unclear expectations from callers. In some instances, swapping TimeStamp with AwakeTimeStamp or BootTimeStamp would be trivial. In others, I wouldn't be confident making the switch without a close look from a module owner.

  • There's a bunch of legacy functionality in TimeStamp, such as the FuzzyFox implementation, QueryPerformanceCounter corner cases for old hardware, and slightly varying timestamps and timestamp math. I wouldn't be confident making decisions about which functionality is necessary, outdated, or bloat.

The approach I had in mind was recommending AwakeTimeStamp and BootTimeStamp as lean versions of TimeStamp with well-defined system sleep behavior, deprecating TimeStamp in favor of the two new classes, and then gradually replacing TimeStamp.

SGTM. I feel your worry about replacing TimeStamp, I did some heavy surgery on that class in the past and testing that your change works everywhere is challenging. While I don't expect any surprises in switching from CLOCK_MONOTONIC to CLOCK_BOOTTIME on desktop Linux, I expect plenty of problems on Android. We've encountered buggy kernel implementations of CLOCK_MONOTONIC there (cough NVidia cough) so I expect more problems with CLOCK_BOOTTIME.

Since FuzzyFox support is being removed (see bug 1666222) we can do the following:

  1. Introduce the new BootTimeStamp class
  2. Change TimeStamp to BootTimeStamp where it fixes known problems (e.g. Performance.now())
  3. File a bug to evaluate using BootTimeStamp as a replacement for TimeStamp after FuzzyFox is removed

Note, the Windows part of TimeStamp is high on my priority list of things that needs to be overhauled so we might as well do that part within 3.

Flags: needinfo?(gsvelto)
Performance Impact: --- → -
Whiteboard: [qf-]

Clearing this needinfo (as part of going through my old needinfos :) ).

https://github.com/mdn/content/issues/4713 is still open

Flags: needinfo?(smaug)
Whiteboard: [fxp]

User of Unity reported this as an issue just recently. I developed a monkeypatch to fix the issue in JS land:

(()=>{
  var p0 = performance.now(), d0 = Date.now(), offset = 0, P = performance;
  P.now_ = P.now;
  P.now2 = () => {
    var p = performance.now_(), d = Date.now(), po = p + offset, dd = d - d0;
    if (dd-po+p0 > 1e4) ret = p0 + dd, offset = ret - po;
    else ret = po;
    d0 = d;
    return p0 = ret;
  };
})();

The above code cross-references against Date.now() to hide any big >10s jumps in performance.now().

Used one of those convenient JS microbenchmarking sites to measure the performance impact of such a monkeypatch:

Firefox Windows:

  • performance.now(): 606 Mops/sec
  • monkeypatched: 10Mops/sec

Chrome Windows:

  • performance.now(): 10 Mops/sec
  • monkeypatched: 5 Mops/sec

Quite happy to see how hyperoptimized Firefox performance was on performance.now(), (assuming Firefox didn't optimize out the performance.now() altogether), but that really makes the monkeypatch version look super bad.

In any case, I hope the performance impact of the implementation of performance.now() will not be worsened after this issue is fixed, since performance overhead in performance.now() can cause quality problems to e.g. real-time game engines timing.

How about we try this, since now we have https://searchfox.org/mozilla-central/source/mozglue/misc/AwakeTimeStamp.cpp, that we've been using for some time in media, and works well? I've recently added a non "low resolution" for all platforms.

Flags: needinfo?(padenot) → needinfo?(gsvelto)

Yeah, that's a good idea. We haven't encountered a single issue with that one and it's been a while.

Flags: needinfo?(gsvelto)
Blocks: 1948866

Ripple effect is a bit high for this, at least:

  • DOMNavigation API
  • Glean
  • Firefox Profiler

I'm started munching through that, but it's even more worrying that all of this is incorrect (and inconsistent between platform).

WIP patch attached that does a lot of it, but it goes everywhere and I'm not sure I have that kind of time right now.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: