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.now() returns incorrect value #17892

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

performance.now() returns incorrect value #17892

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

Comments

@TimothyGu
Copy link
Member

  • Version: v9.x / master
  • Platform: all
  • Subsystem: perf_hooks
$ node -p 'perf_hooks.performance.now()'
174195586.611404

The spec (1 which leads to 2) describes the return value of performance.now() as:

the high resolution time from the time origin to the present time (typically called "now")

where time origin is defined as the point at which the "browsing context is first created", which translates to Node.js as the time Node.js is launched.

In other words, I would expect performance.now() to return a value close to 0 as it is executed as soon as Node.js is launched, rather than process.hrtime() converted to milliseconds as it currently does.

/cc @jasnell

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

fhinkel commented Jan 14, 2018

AFAIK W3C isn't a spec for Node, but we try to match browser behavior as much as possible. Would we break anything in Node if we change time origin? How badly are we breaking the module ecosystems by introducing this change?

@TimothyGu
Copy link
Member Author

W3C isn't a spec for Node, but we try to match browser behavior as much as possible

The performance interface was written to conform to the W3C spec.

Browsers all perform the way as described in the spec, but the recent Meltdown/Spectre mitigations include reducing performance's precision in general: w3c/hr-time#56

Would we break anything in Node if we change time origin?

Not that I am aware.

How badly are we breaking the module ecosystems by introducing this change?

I expect near-zero breakage since this is a relatively new API.

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

2 participants