Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

performance.timeOrigin set incorrectly #17893

Closed
TimothyGu opened this issue Dec 28, 2017 · 8 comments
Closed

performance.timeOrigin set incorrectly #17893

TimothyGu opened this issue Dec 28, 2017 · 8 comments
Assignees
Labels
perf_hooks Issues and PRs related to the implementation of the Performance Timing API.

Comments

@TimothyGu
Copy link
Member

TimothyGu commented Dec 28, 2017

  • Version: v9.x / master
  • Platform: all
  • Subsystem: perf_hooks
$ ./node -p 'new Date(perf_hooks.performance.timeOrigin).toString()'
Sat Jan 03 1970 08:35:02 GMT+0800 (CST)

while in browsers

> new Date(performance.timeOrigin).toString()
"Thu Dec 28 2017 15:02:33 GMT+0800 (CST)"

performance.timeOrigin is spec'd to return the time origin timestamp, which is a high-resolution UNIX time, rather than a time from an arbitrary position in the past (what it currently returns).

Unfortunately, after looking over the libuv documentation I could not find a function that returns something akin to clock_gettime(CLOCK_REALTIME, tp) on POSIX systems. The ideal solution is, of course, adding such a method to libuv. But if it is not possible to do so in a timely manner, I did outline a hack that allows me to get the correct high-precision value of timeOrigin in jsdom/jsdom#2094 (comment), reproduced below:

const { timeOrigin } = process.binding(...);

// Currently:
// performance.timeOrigin = timeOrigin;

// Offset between the time returned by process.hrtime() and Date.now(). A.k.a. the |t1|
// in W3C [HR-TIME] spec.
const hrtimeOffset = getHrtimeOffset();

const timeOriginTimestamp = hrtimeOffset + timeOrigin;
performance.timeOrigin = timeOriginTimestamp;

function getHrtimeOffset() {
  // Wait at most 1 millisecond, a negligible amount of time for initialization.
  let cur = Date.now();
  const next = cur + 1;
  while (cur !== next) {
    cur = Date.now();
  }

  // At this point cur "just" became equal to the next millisecond -- the unseen digits after cur
  // are approximately all 0, and cur is the closest to the actual value of the UNIX time.
  // Now, get the current global monotonic clock value:

  const [hrSec, hrNS] = process.hrtime();
  const globalMonotonicClockMS = hrSec * 1e3 + hrNS / 1e6;

  // Let |t1| be the DOMHighResTimeStamp representing the high resolution Unix time at
  // which the global monotonic clock is zero.
  // I.e., t1 + globalMonotonicClockMS = cur
  //       t1                          = cur - globalMonotonicClockMS
  return cur - globalMonotonicClockMS;
}

but it requires at most 1ms of initialization time when perf_hooks is required.

Thoughts?

/cc @bnoordhuis @cjihrig @jasnell

@TimothyGu TimothyGu added the perf_hooks Issues and PRs related to the implementation of the Performance Timing API. label Dec 28, 2017
@bnoordhuis
Copy link
Member

The ideal solution is, of course, adding such a method to libuv.

libuv/libuv#1674 - Colin was going to work on that.

@TimothyGu
Copy link
Member Author

@bnoordhuis Thanks for the link. In the mean time however, what do you think of having a JS workaround like the one I wrote in the OP?

@bnoordhuis
Copy link
Member

If you are going to write C++ code anyway, you might as well add a wrapper for gettimeofday() / GetSystemTimeAsFileTime() as a stop-gap measure until libuv grows the necessary APIs.

I see at least two issues with the snippet you posted:

  1. const next = cur + 1; while (cur !== next) cur = Date.now() busy-loops when cur jumps from next-1 to next+1 or more, either because the process is rescheduled between statements or the system clock is changed.

  2. It assumes Date.now() and process.hrtime() are near-instantaneous, which they usually are but not always. If the system clock is emulated (virtualized), you'll miss the mark by many microseconds, maybe by even more than a millisecond.

@jasnell
Copy link
Member

jasnell commented Dec 29, 2017

Yep. Glad you're looking at this. This was one I had been wanting to get back to but it's been lower on my priority list. Adding it libuv is definitely the best option. The wrapper that @bnoordhuis suggests is next best option.

@TimothyGu
Copy link
Member Author

Thanks for noticing those issues. We'll need clock_gettime() because sub-millisecond resolution is needed for this API, and probably the Windows equivalent as well. It's quite unfortunate how slow non-vDSO timer syscalls are...

@bnoordhuis
Copy link
Member

gettimeofday() return microseconds and GetSystemTimeAsFileTime() returns the time in tenths of microseconds1. Is that fast enough?

1 The real granularity may of course be worse.

It's quite unfortunate how slow non-vDSO timer syscalls are...

The vDSO is not exempt either. Virtualization software normally intercepts the rdtsc instruction, the most common clock source inside the vDSO.

@TimothyGu
Copy link
Member Author

@bnoordhuis Ah heh, I read the unit wrong. Yes, that’ll be enough. (The spec calls for at least 5-μs resolution, and if that is not possible, 1-ms resolution.)

Virtualization software normally intercepts the rdtsc instruction, the most common clock source inside the vDSO.

Is rdtsc intercepted for hardware-assisted virtualization or KVM as well?

@bnoordhuis
Copy link
Member

For products like VMware and VirtualBox I think the answer is "practically always."

AMD and Intel CPUs have a feature called "TSC offsetting" that lets rdtsc execute at full speed but since it doesn't play well with live migrations to other systems, it's disabled by default (I think - I remember I had to jump through a few hoops to enable it.)

AMD and recently Intel CPUs too support something called "TSC scaling" which can also work across migrations (because it doesn't use a fixed offset like TSC offsetting does) but I can't vouch for it, I don't have hardware that is new enough.

Xen and KVM use paravirtualized clocks (pvclocks) where the guest calls out to the host to read the clock. The vDSO handles that transparently if your kernel has paravirt support enabled. It's slower than an untrapped rdtsc instruction but a lot faster than full-blown emulation.

@TimothyGu TimothyGu self-assigned this Feb 25, 2018
@TimothyGu TimothyGu mentioned this issue Feb 25, 2018
4 tasks
TimothyGu added a commit to TimothyGu/node that referenced this issue Mar 4, 2018
@jasnell jasnell closed this as completed in 9256dbb Mar 6, 2018
MylesBorins pushed a commit that referenced this issue Mar 6, 2018
Fixes: #17892
Fixes: #17893
Fixes: #18992

PR-URL: #18993
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this issue Mar 6, 2018
Fixes: #17892
Fixes: #17893
Fixes: #18992

PR-URL: #18993
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this issue Mar 6, 2018
Fixes: #17892
Fixes: #17893
Fixes: #18992

PR-URL: #18993
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this issue Mar 6, 2018
Fixes: #17892
Fixes: #17893
Fixes: #18992

PR-URL: #18993
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this issue Mar 7, 2018
Fixes: #17892
Fixes: #17893
Fixes: #18992

PR-URL: #18993
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this issue May 8, 2018
Fixes: nodejs#17892
Fixes: nodejs#17893
Fixes: nodejs#18992

PR-URL: nodejs#18993
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
jasnell pushed a commit to jasnell/node that referenced this issue Aug 17, 2018
Fixes: nodejs#17892
Fixes: nodejs#17893
Fixes: nodejs#18992

PR-URL: nodejs#18993
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this issue Sep 6, 2018
Fixes: #17892
Fixes: #17893
Fixes: #18992

Backport-PR-URL: #22380
PR-URL: #18993
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf_hooks Issues and PRs related to the implementation of the Performance Timing API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants