[WTF] Add clock_gettime based monotonicallyIncreasingTime implementation for Linux and BSDs
Created attachment 330214 [details] Patch
Comment on attachment 330214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330214&action=review > Source/WTF/wtf/CurrentTime.cpp:277 > +} FYI, Qt's implementation of monotonic clock timer uses more sofisticated check, see https://code.woboq.org/qt5/qtbase/src/corelib/kernel/qelapsedtimer_unix.cpp.html#65 and from line 115. On Linux, it always assumes monotonic clock to be present, on other systems it checks values of macros and, if needed, what sysconf(_SC_MONOTONIC_CLOCK) returns
Here is a version of that file under LGPL 2 (so that portions could be imported if needed): http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qelapsedtimer_unix.cpp?h=5.6
Comment on attachment 330214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330214&action=review >> Source/WTF/wtf/CurrentTime.cpp:277 >> +} > > FYI, Qt's implementation of monotonic clock timer uses more sofisticated check, see https://code.woboq.org/qt5/qtbase/src/corelib/kernel/qelapsedtimer_unix.cpp.html#65 and from line 115. On Linux, it always assumes monotonic clock to be present, on other systems it checks values of macros and, if needed, what sysconf(_SC_MONOTONIC_CLOCK) returns But actually, are there any Linux/FreeBSD/OpenBSD/NetBSD environments that do not support monotonic clock? For example, libcxx assumes that supported environments have this monotonic clock[1]. And if it fails, it simply crashes. [1]: https://github.com/llvm-mirror/libcxx/blob/master/src/chrono.cpp#L220-L227
Comment on attachment 330214 [details] Patch This seems fine to me, since you've ensured that it only affects JSCOnly. GTK and WPE will be happy that they continue using g_get_monotonic_time, since WebKit events rely on WebKit's monotonic time matching GLib's. (It should be the same under the hood -- CLOCK_MONOTONIC -- either way, but continuing to use g_get_monotonic_time seems preferable.)
>libcxx assumes that supported environments have this monotonic clock[1]. And if it fails, it simply crashes. Cool. But at least it checks that CLOCK_MONOTONIC is defined,
>But actually, are there any Linux/FreeBSD/OpenBSD/NetBSD environments that do not support monotonic clock? It seems like no, there aren't
Comment on attachment 330214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330214&action=review >>> Source/WTF/wtf/CurrentTime.cpp:277 >>> +} >> >> FYI, Qt's implementation of monotonic clock timer uses more sofisticated check, see https://code.woboq.org/qt5/qtbase/src/corelib/kernel/qelapsedtimer_unix.cpp.html#65 and from line 115. On Linux, it always assumes monotonic clock to be present, on other systems it checks values of macros and, if needed, what sysconf(_SC_MONOTONIC_CLOCK) returns > > But actually, are there any Linux/FreeBSD/OpenBSD/NetBSD environments that do not support monotonic clock? > For example, libcxx assumes that supported environments have this monotonic clock[1]. And if it fails, it simply crashes. > > [1]: https://github.com/llvm-mirror/libcxx/blob/master/src/chrono.cpp#L220-L227 Interestingly, it seems like on Linux CLOCK_MONOTONIC isn't really monotonic, but can be affected by adjtime(3) and NTP. There is additional CLOCK_MONOTONIC_RAW which is available since Linux 2.6.28, which is truly monotonic. Neither Qt nor GLib attempt to use it in their monotonic time wrappers
(In reply to Konstantin Tokarev from comment #8) > > Interestingly, it seems like on Linux CLOCK_MONOTONIC isn't really > monotonic, but can be affected by adjtime(3) and NTP. There is additional > CLOCK_MONOTONIC_RAW which is available since Linux 2.6.28, which is truly > monotonic. Neither Qt nor GLib attempt to use it in their monotonic time > wrappers I think both are really monotonic, the difference is that the first is affected by time frequency adjustments via adjtime() (aka: the speed of a second), meanwhile the second doesn't gets affected by that. https://stackoverflow.com/a/14270415
Comment on attachment 330214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330214&action=review > Source/WTF/wtf/CurrentTime.cpp:276 > + return static_cast<double>(ts.tv_sec) + ts.tv_nsec * 1.0e9; This code has a bug, it should either be static_cast<double>(ts.tv_sec) + ts.tv_nsec / 1.0e9 or static_cast<double>(ts.tv_sec) * 1.0e9 + ts.tv_nsec I would prefer the former as it will result in numbers of smaller order
Comment on attachment 330214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330214&action=review >> Source/WTF/wtf/CurrentTime.cpp:276 >> + return static_cast<double>(ts.tv_sec) + ts.tv_nsec * 1.0e9; > > This code has a bug, it should either be > > static_cast<double>(ts.tv_sec) + ts.tv_nsec / 1.0e9 > > or > > static_cast<double>(ts.tv_sec) * 1.0e9 + ts.tv_nsec > > I would prefer the former as it will result in numbers of smaller order Oops, fixed. double should be in seconds. The former is fine.
Committed r226304: <https://trac.webkit.org/changeset/226304>
<rdar://problem/36261381>
<rdar://problem/36261385>
Comment on attachment 330214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330214&action=review >>> Source/WTF/wtf/CurrentTime.cpp:276 >>> + return static_cast<double>(ts.tv_sec) + ts.tv_nsec * 1.0e9; >> >> This code has a bug, it should either be >> >> static_cast<double>(ts.tv_sec) + ts.tv_nsec / 1.0e9 >> >> or >> >> static_cast<double>(ts.tv_sec) * 1.0e9 + ts.tv_nsec >> >> I would prefer the former as it will result in numbers of smaller order > > Oops, fixed. double should be in seconds. The former is fine. I would have suggested 1.0e-9 instead of switching from multiplication to division.