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

[Windows] time.sleep() should use CREATE_WAITABLE_TIMER_HIGH_RESOLUTION #89592

Closed
vstinner opened this issue Oct 11, 2021 · 10 comments
Closed

[Windows] time.sleep() should use CREATE_WAITABLE_TIMER_HIGH_RESOLUTION #89592

vstinner opened this issue Oct 11, 2021 · 10 comments
Labels
3.11 only security fixes OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

BPO 45429
Nosy @pfmoore, @vstinner, @tjguk, @ambv, @zware, @eryksun, @zooba, @corona10, @Livius90
PRs
  • bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible #29203
  • bpo-45429: Merge whatsnew about time.sleep #29589
  • Files
  • bpo-45429.py
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-11-18.06:26:34.313>
    created_at = <Date 2021-10-11.08:28:27.791>
    labels = ['type-feature', 'library', 'OS-windows', '3.11']
    title = '[Windows] time.sleep() should use CREATE_WAITABLE_TIMER_HIGH_RESOLUTION'
    updated_at = <Date 2021-11-18.06:26:34.312>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-11-18.06:26:34.312>
    actor = 'corona10'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-11-18.06:26:34.313>
    closer = 'corona10'
    components = ['Library (Lib)', 'Windows']
    creation = <Date 2021-10-11.08:28:27.791>
    creator = 'vstinner'
    dependencies = []
    files = ['50392']
    hgrepos = []
    issue_num = 45429
    keywords = ['patch']
    message_count = 9.0
    messages = ['403631', '403634', '403635', '403659', '403703', '404868', '404936', '406402', '406512']
    nosy_count = 9.0
    nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'lukasz.langa', 'zach.ware', 'eryksun', 'steve.dower', 'corona10', 'Livius']
    pr_nums = ['29203', '29589']
    priority = 'normal'
    resolution = None
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue45429'
    versions = ['Python 3.11']

    @vstinner
    Copy link
    Member Author

    In bpo-21302, the Windows implementation of time.sleep() was modified to use a waitable timer:

    New changeset 58f8adf by Victor Stinner in branch 'main':
    bpo-21302: time.sleep() uses waitable timer on Windows (GH-28483)
    58f8adf

    It now calls the functions:

    • CreateWaitableTimerW()
    • SetWaitableTimer()
    • WaitForMultipleObjects()

    While SetWaitableTimer() has a resolution of 100 ns, the timer has a resolution of 15.6 ms in practice.

    We could use the undocumented CREATE_WAITABLE_TIMER_HIGH_RESOLUTION flag with CreateWaitableTimerEx(). See: https://bugs.python.org/issue21302#msg403550

    See also:

    @vstinner vstinner added 3.11 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement OS-windows labels Oct 11, 2021
    @vstinner
    Copy link
    Member Author

    @vstinner
    Copy link
    Member Author

    The Go programming language called timeBeginPeriod(1) to get more accurate timers. With the following change, it can now use a high resolution timer (CREATE_WAITABLE_TIMER_HIGH_RESOLUTION) to sleep:
    https://go-review.googlesource.com/c/go/+/248699/

    @vstinner
    Copy link
    Member Author

    See also bpo-19007: "precise time.time() under Windows 8: use GetSystemTimePreciseAsFileTime".

    @eryksun
    Copy link
    Contributor

    eryksun commented Oct 11, 2021

    It's up to the core devs whether or not Python should try to use a high-resolution timer, which is currently undocumented in the Windows API and implemented only in recent releases of Windows 10 and 11. But if this does get supported, the code should fall back on creating a normal timer if CREATE_WAITABLE_TIMER_HIGH_RESOLUTION makes the call fail. For example:

        #ifndef CREATE_WAITABLE_TIMER_HIGH_RESOLUTION
        #define CREATE_WAITABLE_TIMER_HIGH_RESOLUTION 0x00000002
        #endif
        LARGE_INTEGER relative_timeout;
        // No need to check for integer overflow, both types are signed
        assert(sizeof(relative_timeout) == sizeof(timeout_100ns));
        // SetWaitableTimerEx(): a negative due time is relative
        relative_timeout.QuadPart = -timeout_100ns;
        DWORD flags = CREATE_WAITABLE_TIMER_HIGH_RESOLUTION;
    
    create_timer:
    
            HANDLE timer = CreateWaitableTimerExW(NULL, NULL, flags, TIMER_ALL_ACCESS);
            if (timer == NULL)
            {
                if (flags && GetLastError() == ERROR_INVALID_PARAMETER) {
                    // CREATE_WAITABLE_TIMER_HIGH_RESOLUTION is not supported.
                    flags = 0;
                    goto create_timer;
                }
                PyErr_SetFromWindowsErr(0);
                return -1;
            }
    
            if (!SetWaitableTimerEx(timer, &relative_timeout,
                  0,          // no period; the timer is signaled once
                  NULL, NULL, // no completion routine
                  NULL,       // no wake context; do not resume from suspend
                  0))         // no tolerable delay for timer coalescing
            {
                PyErr_SetFromWindowsErr(0);
                goto error;
            }

    @Livius90
    Copy link
    Mannequin

    Livius90 mannequin commented Oct 23, 2021

    A similar solution was introduced in VirtualBox some months ago. Soon, i could get back my Windows 10 developing PC and i can try this things.

    https://www.virtualbox.org/browser/vbox/trunk/src/VBox/Runtime/r3/win/timer-win.cpp#L312

    @corona10
    Copy link
    Member

    AS-IS:
    average: 0.015609736680984497

    TO-BE:
    average: 2.7387380599975585e-05

    Impressive result :)

    @corona10
    Copy link
    Member

    New changeset 55868f1 by Dong-hee Na in branch 'main':
    bpo-45429: Support CREATE_WAITABLE_TIMER_HIGH_RESOLUTION if possible (GH-29203)
    55868f1

    @ambv
    Copy link
    Contributor

    ambv commented Nov 18, 2021

    New changeset fc4474e by Dong-hee Na in branch 'main':
    bpo-45429: Merge whatsnew about time.sleep (GH-29589)
    fc4474e

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @Livius90
    Copy link
    Contributor

    some test-run:
    #65501 (comment)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants