[PATCH resend 1/3] ntdll: Fix tick count calculation on Mac.
Inspired by a patch by Andrew Eikum. macOS's mach_absolute_time() stops counting when the computer goes to sleep/suspend/hibernate/etc. However, Windows's GetTickCount() does not stop counting. mach_continuous_time() matches Windows's behavior. BSD's CLOCK_MONOTONIC already counts asleep time. Unfortunately, there is no clock source on Linux which does exactly what we want. CLOCK_MONOTONIC_RAW is unaffected by NTP adjustment, but like mach_absolute_time() doesn't keep ticking when the computer is asleep. CLOCK_BOOTTIME does keep ticking, but it is affected by NTP adjustments. CLOCK_MONOTONIC has both problems. What's needed is a CLOCK_BOOTTIME_RAW, which would not be slewed by adjtimex(2) and would count time spent asleep. To avoid issues with skew and performance, this patch falls back to mach_absolute_time() on macOS if mach_continuous_time() is unavailable. Note that mach_continuous_time() was introduced in macOS 10.12, meaning that if the minimum version required is less than that, it will be linked weakly. Therefore we must check that it is nonnull before attempting to call it. Signed-off-by: Chip Davis <cdavis(a)codeweavers.com> --- configure.ac | 1 + dlls/ntdll/time.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/configure.ac b/configure.ac index 754dbe8b36e..2a049909a65 100644 --- a/configure.ac +++ b/configure.ac @@ -2206,6 +2206,7 @@ AC_CHECK_FUNCS(\ getopt_long_only \ kqueue \ lstat \ + mach_continuous_time \ pipe2 \ poll \ port_create \ diff --git a/dlls/ntdll/time.c b/dlls/ntdll/time.c index 91e5887b879..4e3d5583c7b 100644 --- a/dlls/ntdll/time.c +++ b/dlls/ntdll/time.c @@ -111,6 +111,10 @@ static inline ULONGLONG monotonic_counter(void) static mach_timebase_info_data_t timebase; if (!timebase.denom) mach_timebase_info( &timebase ); +#ifdef HAVE_MACH_CONTINUOUS_TIME + if (&mach_continuous_time != NULL) + return mach_continuous_time() * timebase.numer / timebase.denom / 100; +#endif return mach_absolute_time() * timebase.numer / timebase.denom / 100; #elif defined(HAVE_CLOCK_GETTIME) struct timespec ts; -- 2.21.0
Signed-off-by: Chip Davis <cdavis(a)codeweavers.com> --- dlls/kernel32/time.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/kernel32/time.c b/dlls/kernel32/time.c index 817a8305186..716230aa7a2 100644 --- a/dlls/kernel32/time.c +++ b/dlls/kernel32/time.c @@ -78,6 +78,10 @@ static inline ULONGLONG monotonic_counter(void) static mach_timebase_info_data_t timebase; if (!timebase.denom) mach_timebase_info( &timebase ); +#ifdef HAVE_MACH_CONTINUOUS_TIME + if (&mach_continuous_time != NULL) + return mach_continuous_time() * timebase.numer / timebase.denom / 100; +#endif return mach_absolute_time() * timebase.numer / timebase.denom / 100; #elif defined(HAVE_CLOCK_GETTIME) struct timespec ts; -- 2.21.0
Signed-off-by: Chip Davis <cdavis(a)codeweavers.com> --- server/request.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/request.c b/server/request.c index d2adb08a183..5610d392cf9 100644 --- a/server/request.c +++ b/server/request.c @@ -529,6 +529,10 @@ unsigned int get_tick_count(void) static mach_timebase_info_data_t timebase; if (!timebase.denom) mach_timebase_info( &timebase ); +#ifdef HAVE_MACH_CONTINUOUS_TIME + if (&mach_continuous_time != NULL) + return mach_continuous_time() * timebase.numer / timebase.denom / 1000000; +#endif return mach_absolute_time() * timebase.numer / timebase.denom / 1000000; #elif defined(HAVE_CLOCK_GETTIME) struct timespec ts; -- 2.21.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=60930 Your paranoid android. === debian10 (32 bit WoW report) === ntdll: pipe.c:1557: Test failed: pipe is not signaled pipe.c:1557: Test failed: pipe is not signaled === debian10 (64 bit WoW report) === kernel32: comm.c:918: Test failed: OutQueue should not be empty
Chip Davis <cdavis(a)codeweavers.com> wrote:
+#ifdef HAVE_MACH_CONTINUOUS_TIME + if (&mach_continuous_time != NULL) + return mach_continuous_time() * timebase.numer / timebase.denom / 100; +#endif
While the compiler will fix it for you it would still be better to use 'if (mach_continuous_time != NULL)' IMO, so that even if the type of the 'mach_continuous_time' is a real function pointer it still works. Same applies to the next patch. That's a pure cosmetics though. -- Dmitry.
November 26, 2019 10:40 PM, "Dmitry Timoshkov" <dmitry(a)baikal.ru> wrote:
Chip Davis <cdavis(a)codeweavers.com> wrote:
+#ifdef HAVE_MACH_CONTINUOUS_TIME + if (&mach_continuous_time != NULL) + return mach_continuous_time() * timebase.numer / timebase.denom / 100; +#endif
While the compiler will fix it for you it would still be better to use 'if (mach_continuous_time != NULL)' IMO, so that even if the type of the 'mach_continuous_time' is a real function pointer it still works. Same applies to the next patch. That's a pure cosmetics though.
In the case that mach_continuous_time is linked strong, which it will be if -mmacosx-version-min=10.12 or higher, the compiler will warn if I *don't* use '&'. Chip
"Chip Davis" <cdavis(a)codeweavers.com> wrote:
+#ifdef HAVE_MACH_CONTINUOUS_TIME + if (&mach_continuous_time != NULL) + return mach_continuous_time() * timebase.numer / timebase.denom / 100; +#endif
While the compiler will fix it for you it would still be better to use 'if (mach_continuous_time != NULL)' IMO, so that even if the type of the 'mach_continuous_time' is a real function pointer it still works. Same applies to the next patch. That's a pure cosmetics though.
In the case that mach_continuous_time is linked strong, which it will be if -mmacosx-version-min=10.12 or higher, the compiler will warn if I *don't* use '&'.
I see, thanks for the explanation. -- Dmitry.
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=60929 Your paranoid android. === debian10 (32 bit WoW report) === ntdll: pipe.c:1557: Test failed: pipe is not signaled pipe.c:1557: Test failed: pipe is not signaled === debian10 (64 bit WoW report) === kernel32: debugger: Timeout
On Tue, Nov 26, 2019 at 10:29:58PM -0600, Chip Davis wrote:
Signed-off-by: Chip Davis <cdavis(a)codeweavers.com> --- configure.ac | 1 + dlls/ntdll/time.c | 4 ++++ 2 files changed, 5 insertions(+)
diff --git a/configure.ac b/configure.ac index 754dbe8b36e..2a049909a65 100644 --- a/configure.ac +++ b/configure.ac @@ -2206,6 +2206,7 @@ AC_CHECK_FUNCS(\ getopt_long_only \ kqueue \ lstat \ + mach_continuous_time \ pipe2 \ poll \ port_create \ diff --git a/dlls/ntdll/time.c b/dlls/ntdll/time.c index 91e5887b879..4e3d5583c7b 100644 --- a/dlls/ntdll/time.c +++ b/dlls/ntdll/time.c @@ -111,6 +111,10 @@ static inline ULONGLONG monotonic_counter(void) static mach_timebase_info_data_t timebase;
if (!timebase.denom) mach_timebase_info( &timebase ); +#ifdef HAVE_MACH_CONTINUOUS_TIME + if (&mach_continuous_time != NULL) + return mach_continuous_time() * timebase.numer / timebase.denom / 100; +#endif return mach_absolute_time() * timebase.numer / timebase.denom / 100; #elif defined(HAVE_CLOCK_GETTIME) struct timespec ts;
This series should really be squashed into one patch. We don't want commits where, for example, GetTickCount() and NtGetCountTick() differ Huw.
participants (4)
-
Chip Davis -
Dmitry Timoshkov -
Huw Davies -
Marvin