[Threading][Win32] Fix the wait functions for Win32 to always over-wait.

Rather than bodging the test to make it more robust, fix the functions
in the Threading layer to behave the same as they do on other platforms,
i.e. to guarantee that they always wait *at least* the amount of time
you asked for.

rdar://100236038

Author: al45tair

include/swift/Threading/Impl/Win32.h

@@ -122,9 +122,76 @@ template <class Rep, class Period>
 inline bool cond_wait(cond_handle &handle,
                       std::chrono::duration<Rep, Period> duration) {
   auto ms = chrono_utils::ceil<std::chrono::milliseconds>(duration);
+
+  /* If you are paying attention to the next line, you are now asking
+
+       "Why are you adding 16 - that seems mad?"
+
+     To explain, we need to understand how the Sleep...() APIs in
+     Windows work.
+
+     The Windows kernel runs a timer, the system tick, which is
+     used for scheduling; the timer in question, for historical
+     reasons, by default runs at 64Hz.  Every time the timer fires,
+     Windows updates the tick count, schedules processes and so on.
+
+     When you ask to sleep, there are two cases:
+
+     1. You've asked to sleep for less than a tick, or
+
+     2. You've asked to sleep for at least a tick.
+
+     In case 1, the sleep functions appear to run a delay loop; this
+     is by its very nature inaccurate and you may or may not wait less
+     than the time you requested.  You might also get rescheduled,
+     in which case you'll wait at least a whole tick.
+
+     In case 2, Windows appears to be adding the requested wait to
+     its current tick count to work out when to wake your thread.
+     That *sounds* sensible, until you realise that if it does this
+     towards the end of a tick period, you will wait for that much
+     less time.  i.e. the sleep functions will return *early* by
+     up to one tick period.
+
+     This is especially unfortunate if you ask to wait for exactly
+     one tick period, because you will end up waiting for anything
+     between no time at all and a full tick period.
+
+     To complicate matters, the system tick rate might not be 64Hz;
+     the multimedia timer API can cause it to change to as high as
+     1kHz, and on *some* machines this happens globally for all
+     processes, while on *other* machines the kernel is actually using
+     a separate system tick rate and merely pretending to Win32
+     processes that things still work this way.
+
+     On other platforms, these kinds of functions are guaranteed to
+     wait for at least the time you requested, and we'd like for that
+     to be true for the Threading package's APIs here.  One way to
+     achieve that is to add a whole tick's worth of milliseconds
+     to the time we're requesting to wait for.  In that case, we'll
+     avoid the delay loop from case (1), and we'll also guarantee that
+     we wait for at least the amount of time the caller of this
+     function expected.
+
+     It would be nice if there were a Windows API we could call to
+     obtain the current system tick period.  The Internet suggests
+     that the undocumented call NtQueryTimerResolution() might be of
+     some use for this, but it turns out that that just gives you
+     the kernel's idea of the system tick period, which isn't the
+     same as Win32's idea necessarily.  e.g. on my test system, I
+     can see that the tick period is 15.625ms, while that call tells
+     me 1ms.
+
+     We don't want to change the system tick rate using the multimedia
+     timer API mentioned earlier, because on some machines that is
+     global and will use extra power and CPU cycles.
+
+     The upshot is that the best choice here appears to be to add
+     15.625ms (1/64s) to the time we're asking to wait.  That rounds
+     up to 16ms, which is why there's a +16. */
   return SleepConditionVariableSRW(&handle.condition,
                                    &handle.lock,
-                                   DWORD(ms.count()),
+                                   DWORD(ms.count()) + 16,
                                    0);
 }
 inline bool cond_wait(cond_handle &handle,

unittests/Threading/ConditionVariable.cpp

@@ -19,51 +19,13 @@
 #include "ThreadingHelpers.h"
 #include "LockingHelpers.h"
 
-#ifdef _WIN32
-#include <windows.h>
-#endif
-
 using namespace swift;
 
 template <typename Body>
 std::chrono::duration<double> measureDuration(Body body) {
-#ifdef _WIN32
-  using namespace std::chrono_literals;
-
-  /* On Windows, we use SleepConditionVariableSRW() to implement
-     ConditionVariable.  The way Windows implements the SleepXXX() functions,
-     if you specify a timeout smaller than the current system tick rate,
-     the function may return early.  See
-
-     https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sleep
-
-     This causes flakiness in this test, which is undesirable.
-
-     To combat this, we adjust the system tick rate temporarily here.
-
-     Even *after* doing that, measurement suggests that Windows can return
-     early by an amount not exceeding a system tick. */
-  TIMECAPS tc;
-  UINT uTimerRes;
-
-  ASSERT_NE(timeGetDevCaps(&tc, sizeof(TIMECAPS)), TIMERR_NOERROR);
-
-  uTimerRes = min(max(tc.wPeriodMin, 1/*ms*/), tc.wPeriodMax);
-  timeBeginPeriod(uTimerRes);
-#endif
-
   auto start = std::chrono::steady_clock::now();
   body();
-  std::chrono::duration<double> elapsed = std::chrono::steady_clock::now() - start;
-
-#ifdef _WIN32
-  timeEndPeriod(uTimerRes);
-
-  // Lie about the elapsed time to make the test more robust
-  elapsed += 0.001s;
-#endif
-
-  return elapsed;
+  return std::chrono::steady_clock::now() - start;
 }
 
 // -----------------------------------------------------------------------------