Skip to content

bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible #29203

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Nov 16, 2021
Merged

bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible #29203

merged 9 commits into from
Nov 16, 2021

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Oct 24, 2021 •
edited by bedevere-bot
Loading

@corona10 corona10 requested a review from vstinner October 24, 2021 16:01
eryksun
eryksun reviewed Oct 24, 2021
View reviewed changes
corona10
corona10 commented Oct 24, 2021
View reviewed changes
eryksun
eryksun reviewed Oct 24, 2021
View reviewed changes
@corona10 corona10 force-pushed the bpo-45429 branch 3 times, most recently from aa8d2ef to eabd853 Compare October 24, 2021 17:30
@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
eryksun
eryksun reviewed Oct 24, 2021
View reviewed changes
@corona10
Copy link
Member Author

corona10 commented Nov 9, 2021

@vstinner gentle ping

vstinner
vstinner reviewed Nov 16, 2021
View reviewed changes
Modules/timemodule.c Outdated
@@ -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

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) {

Modules/timemodule.c Outdated
#define CREATE_WAITABLE_TIMER_HIGH_RESOLUTION 0x00000002
#endif

static DWORD timer_flags = -1;
Copy link
Member

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;

Misc/NEWS.d/next/Library/2021-10-25-01-22-49.bpo-45429.VaEyN9.rst Outdated
@@ -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

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)

Copy link
Member

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.

@vstinner
Copy link
Member

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)!

@corona10 corona10 requested a review from vstinner November 16, 2021 12:33
vstinner
vstinner reviewed Nov 16, 2021
View reviewed changes
Doc/whatsnew/3.11.rst
@@ -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

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.

vstinner
vstinner approved these changes Nov 16, 2021
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

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

@corona10
Copy link
Member Author

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

Will do it at separated PR (maybe tomorrow)

@corona10 corona10 merged commit 55868f1 into python:main Nov 16, 2021
@corona10 corona10 deleted the bpo-45429 branch November 16, 2021 13:41
remykarem pushed a commit to remykarem/cpython that referenced this pull request Dec 7, 2021
@corona10 @remykarem
bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible (p…
747f0d7 
…ythonGH-29203)
@vstinner vstinner mentioned this pull request Nov 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eryksun eryksun eryksun left review comments

@vstinner vstinner vstinner approved these changes

@abalkin abalkin Awaiting requested review from abalkin abalkin is a code owner

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@corona10 @vstinner @eryksun @the-knights-who-say-ni @bedevere-bot