gh-88494: Reduce CLOCK_RES to 1 ms in tests#118395
gh-88494: Reduce CLOCK_RES to 1 ms in tests#118395vstinner wants to merge 2 commits intopython:mainfrom
Conversation
On Windows, time.monotonic() now calls QueryPerformanceCounter() which has a resolution of 100 ns. Reduce CLOCK_RES from 50 or 100 ms to 1 ms.
|
Sadly, my change is too optimistic :-( test_asyncio failed on Windows:
|
gvanrossum
left a comment
There was a problem hiding this comment.
Now I worry that other tests (that still use the 1ms value) will become flaky on Windows, unless you have a clear explanation why that fix is only needed in that one spot.
asyncio.BaseEventLoop.time() is time.monotonic(). On Windows, this clock now has a resolution of 100 ns, since I modified it to use QueryPerformanceCounter().
This test calls RegisterWaitForSingleObject() which has a resolution of 15.6 ms. I scheduled buildbot jobs to see how the change goes on more various machines. |
|
Failure on AMD64 Windows Server 2022 NoGIL PR: |
|
Maybe the problem is that the meaning of the variable CLOCK_RES is ambiguous? It looks like it gives the actual clock resolution, but it actually seems to mean the tolerance for longer (or shorter) delays. Tests depending on close-to-exact delays will always be flaky -- or if we make the tolerance too large, they will be meaningless. |
|
While it's appealing to make the test stricter, CLOCK_RES is used to compare time.monotonic() values (resolution better than 1 us), but also to compare Windows "wait" functions (resolution of 15.6 ms). I prefer to abandon this PR. I don't want to have multiple CLOCK_RES depending on the function and/or the operating system. Let's keep the status quo of 50-100 ms tolerance. |
|
Good call. Sorry it didn't work out, but they can't all be winners... :-) |
On Windows, time.monotonic() now calls QueryPerformanceCounter() which has a resolution of 100 ns. Reduce CLOCK_RES from 50 or 100 ms to 1 ms.