-
Notifications
You must be signed in to change notification settings - Fork 7.3k
use hrtime instead of Date.now for timeouts #5497
Comments
@isaacs, @tjfontaine please fix me - if this would works the #7603 wont appears. |
@egorse thanks for finding this issue, that commit ( timers: use uv_now instead of Date.now ) has landed in v0.11 branch. May I ask you to give it a try? It'll soon become a next stable release anywy. |
@indutny the 0.11.13 works just fine. More over, the patch works for 0.10.25 too (except minor correction |
@tjfontaine what do you think about backporting it? |
The reason I didn't backport it initially is because some people are (erroneously) relying on |
Original commit message: timers: use uv_now instead of Date.now This saves a few calls to gettimeofday which can be expensive, and potentially subject to clock drift. Instead use the loop time which uses hrtime internally. fixes nodejs#5497 In addition to the backport, this commit: - keeps _idleStart timers' property which is still set to Date.now() to avoid breaking existing code that uses it, even if its use is discouraged. - adds automated tests. These tests use a specific branch of libfaketime that hasn't been submitted upstream yet. libfaketime is git cloned if needed when running automated tests.
Original commit message: timers: use uv_now instead of Date.now This saves a few calls to gettimeofday which can be expensive, and potentially subject to clock drift. Instead use the loop time which uses hrtime internally. fixes nodejs#5497 In addition to the backport, this commit: - keeps _idleStart timers' property which is still set to Date.now() to avoid breaking existing code that uses it, even if its use is discouraged. - adds automated tests. These tests use a specific branch of libfaketime that hasn't been submitted upstream yet. libfaketime is git cloned if needed when running automated tests.
@misterdjules ... what is the status on this one? |
Uses better troff formatting. Removes v8 options from the man page. Also edits `node -h` in node.cc slightly. PR-URL: nodejs#5497 Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This is fixed in any timers code I've interacted in (e.g. io.js 1.0.0+), everything uses 0.x is EOL, so I think there is nothing left to be done. Closing. |
Semantically, we're doing something like "Do this in 1234ms", rather than "Do this at 10:45:53", so using a wall clock time for this makes hardly any sense.
At least, exposing
uv_default_loop()->now
to JS-land would be a big improvement, since that saves an extra gettimeofday syscall.The text was updated successfully, but these errors were encountered: