Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible #29203

Merged
merged 9 commits into from Nov 16, 2021

Conversation

Projects
None yet
5 participants
@corona10
Copy link
Member

@corona10 corona10 commented Oct 24, 2021

https://bugs.python.org/issue45429

Modules/timemodule.c Outdated Show resolved Hide resolved
Loading
Modules/timemodule.c Outdated Show resolved Hide resolved
Loading
Modules/timemodule.c Outdated Show resolved Hide resolved
Loading
@corona10 corona10 force-pushed the bpo-45429 branch 3 times, most recently from aa8d2ef to eabd853 Oct 24, 2021
@corona10 corona10 changed the title bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible [WIP] bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible Oct 24, 2021
@corona10 corona10 changed the title [WIP] bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible Oct 24, 2021
Modules/timemodule.c Outdated Show resolved Hide resolved
Loading
@corona10
Copy link
Member Author

@corona10 corona10 commented Nov 9, 2021

@vstinner gentle ping

Loading

@@ -2017,6 +2024,23 @@ time_exec(PyObject *module)
utc_string = tm.tm_zone;
#endif

#if defined(MS_WINDOWS)
if ((LONG)timer_flags == -1) {
Copy link
Member

@vstinner vstinner Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ((LONG)timer_flags == -1) {
if (timer_flags == (DWORD)-1) {

Loading

#define CREATE_WAITABLE_TIMER_HIGH_RESOLUTION 0x00000002
#endif

static DWORD timer_flags = -1;
Copy link
Member

@vstinner vstinner Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static DWORD timer_flags = -1;
static DWORD timer_flags = (DWORD)-1;

Loading

@@ -0,0 +1,3 @@
On Windows, :func:`time.sleep` now uses a waitable timer which supports
high-resolution timers without increasing the timer frequency if possible.
Copy link
Member

@vstinner vstinner Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to omit "without increasing the timer frequency if possible":

Suggested change
high-resolution timers without increasing the timer frequency if possible.
high-resolution timers.

(same change where you copied this doc)

Loading

Copy link
Member

@vstinner vstinner Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can explain that in Python 3.10, the best resolution was 1 ms, whereas it's now better (smaller) than 1 ms.

Loading

@vstinner
Copy link
Member

@vstinner vstinner commented Nov 16, 2021

The change works as expected on my Windows 10 "version 21H1" VM ("OS Build 19043.1348").

On a debug build (worse performance than a release build), bench.py of https://bugs.python.org/issue21302 says:

time.sleep(1e-10) benchmark...
Mean +- std dev: 20.0 ms +- 6.8 ms (250 values)

With this PR, I get:

time.sleep(1e-10) benchmark...
Mean +- std dev: 17.2 us +- 10.6 us (249964 values)

It's ~1000x more accurate: ms (10^-3 sec) => us (10^-6 sec)!

Loading

@corona10 corona10 requested a review from vstinner Nov 16, 2021
@@ -272,6 +272,10 @@ time
a resolution of 1 millisecond (10\ :sup:`-3` seconds).
(Contributed by Benjamin Szőke and Victor Stinner in :issue:`21302`.)

* On Windows, :func:`time.sleep` now uses a waitable timer which supports high-resolution timers.
In Python 3.10, the best resolution was 1 ms, from Python 3.11 it's now smaller than 1 ms.
Copy link
Member

@vstinner vstinner Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe try to merge this this item with the previous one.

Loading

Copy link
Member

@vstinner vstinner left a comment

LGTM. The code is correct, there is maybe a way to enhance the What's New In Python 3.11 doc.

Loading

@corona10
Copy link
Member Author

@corona10 corona10 commented Nov 16, 2021

there is maybe a way to enhance the What's New In Python 3.11 doc.

Will do it at separated PR (maybe tomorrow)

Loading

@corona10 corona10 merged commit 55868f1 into python:main Nov 16, 2021
12 checks passed
Loading
@corona10 corona10 deleted the bpo-45429 branch Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment