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

time.perf_counter() is not system-wide on Windows, in disagreement with documentation #81386

Closed
kh90909 mannequin opened this issue Jun 8, 2019 · 12 comments
Closed

time.perf_counter() is not system-wide on Windows, in disagreement with documentation #81386

kh90909 mannequin opened this issue Jun 8, 2019 · 12 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@kh90909
Copy link
Mannequin

kh90909 mannequin commented Jun 8, 2019

BPO 37205
Nosy @pfmoore, @vstinner, @tjguk, @zware, @eryksun, @zooba, @Mariatta, @danielhrisca, @kh90909
PRs
  • bpo-37205: time.perf_counter() and time.monotonic() are system-wide #23284
  • bpo-37205: time.time() cannot fail with fatal error #23314
  • 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 2020-11-16.15:13:02.153>
    created_at = <Date 2019-06-08.18:15:00.046>
    labels = ['type-bug', '3.8', '3.9', '3.7', 'library', 'OS-windows', 'docs']
    title = 'time.perf_counter() is not system-wide on Windows, in disagreement with documentation'
    updated_at = <Date 2021-03-21.21:11:01.645>
    user = 'https://github.com/kh90909'

    bugs.python.org fields:

    activity = <Date 2021-03-21.21:11:01.645>
    actor = 'eryksun'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2020-11-16.15:13:02.153>
    closer = 'vstinner'
    components = ['Documentation', 'Library (Lib)', 'Windows']
    creation = <Date 2019-06-08.18:15:00.046>
    creator = 'kh90909'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 37205
    keywords = ['patch']
    message_count = 12.0
    messages = ['345057', '345059', '346095', '346097', '380930', '380933', '380934', '380957', '380977', '381092', '381105', '381107']
    nosy_count = 10.0
    nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'docs@python', 'zach.ware', 'eryksun', 'steve.dower', 'Mariatta', 'danielhrisca', 'kh90909']
    pr_nums = ['23284', '23314']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue37205'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    @kh90909
    Copy link
    Mannequin Author

    kh90909 mannequin commented Jun 8, 2019

    The documentation for time.perf_counter() indicates it should return a system-wide value: https://docs.python.org/3/library/time.html#time.perf_counter

    This is not true on Windows, as a process-specific offset is subtracted from the underlying QueryPerformanceCounter() value. The code comments indicate this is to reduce precision loss: https://github.com/python/cpython/blob/master/Python/pytime.c#L995-L997

    This is relevant in multiprocess applications, where accurate timing is required across multiple processes. Here is a simple test case:

    -----------------------------------------------------------

    import concurrent.futures
    import time
    
    def worker():
        return time.perf_counter()
    
    if __name__ == '__main__':
        pool = concurrent.futures.ProcessPoolExecutor()
        futures = []
        for i in range(3):
            print('Submitting worker {:d} at time.perf_counter() == {:.3f}'.format(i, time.perf_counter()))
            futures.append(pool.submit(worker))
            time.sleep(1)
    
        for i, f in enumerate(futures):
            print('Worker {:d} started at time.perf_counter() == {:.3f}'.format(i, f.result()))

    Output:
    -----------------------------------------------------------
    C:\...>Python37\python.exe -VV
    Python 3.7.3 (v3.7.3:ef4ec6ed12, Mar 25 2019, 22:22:05)
    [MSC v.1916 64 bit (AMD64)]

    C:\...>Python37\python.exe perf_counter_across_processes.py
    Submitting worker 0 at time.perf_counter() == 0.376
    Submitting worker 1 at time.perf_counter() == 1.527
    Submitting worker 2 at time.perf_counter() == 2.529
    Worker 0 started at time.perf_counter() == 0.380
    Worker 1 started at time.perf_counter() == 0.956
    Worker 2 started at time.perf_counter() == 1.963
    -----------------------------------------------------------

    See my stackoverflow question for a comparison with Linux: https://stackoverflow.com/questions/56502111/should-time-perf-counter-be-consistent-across-processes-in-python-on-windows

    @kh90909 kh90909 mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes labels Jun 8, 2019
    @kh90909 kh90909 mannequin assigned docspython Jun 8, 2019
    @kh90909 kh90909 mannequin added docs Documentation in the Doc dir stdlib Python modules in the Lib dir OS-windows type-bug An unexpected behavior, bug, or error labels Jun 8, 2019
    @kh90909
    Copy link
    Mannequin Author

    kh90909 mannequin commented Jun 8, 2019

    Note that this offset subtraction behavior appears to be inherited from time.clock(), which did not make any guarantees about returning a system-wide value:

    ec89539#diff-4a575e94d6ac98a0d82fd93509b6bfd3L91

    @kh90909
    Copy link
    Mannequin Author

    kh90909 mannequin commented Jun 20, 2019

    Hi Terry,

    I'm new to this so apologies in advance if this is a silly question...can you explain why you removed 3.5 and 3.6 from the versions list? I have tested that the issue is present in 3.6 and the offending code has been present since time.perf_counter() was introduced in 3.3.

    It it because these versions are in maintenance-only status or similar, such that this type of bug fix would not be considered?

    Thanks,
    Ken

    @Mariatta
    Copy link
    Member

    can you explain why you removed 3.5 and 3.6 from the versions list?

    3.5 and 3.6 are closed for regular bug fix maintenance. We're only fixing issues in 3.7 and 3.8 now.

    Only security fixes will be applied to 3.5 or 3.6, and this issue is not considered a security issue.

    More details in https://devguide.python.org/#status-of-python-branches

    @danielhrisca
    Copy link
    Mannequin

    danielhrisca mannequin commented Nov 13, 2020

    I'm actually surprised that this problem gets so little interest. It's probably so obscure that most people don't even realize it.

    Why isn't it implemented using winapi calls for the windows platform?

    I took inspiration from this SO thread https://stackoverflow.com/questions/56502111/should-time-perf-counter-be-consistent-across-processes-in-python-on-windows

    and came up with this function for 64 bit Python (a few more lines would be needed for error checking)

    #include <windows.h>
    static PyObject* perf_counter(PyObject* self, PyObject* args)
    {
    	PyObject* ret;
    
    	QueryPerformanceFrequency(lpFrequency);
    	QueryPerformanceCounter(lpPerformanceCount);
    	
    	ret = PyFloat_FromDouble(((double) lpPerformanceCount->QuadPart ) / lpFrequency->QuadPart);
    	
    	return ret;
    }

    @vstinner
    Copy link
    Member

    Since I added time.perf_counter_ns() to Python 3.7, it would be acceptable to reduce the "t0" variable and suggest to use time.perf_counter_ns() instead of time.perf_counter() for best precision.

    @danielhrisca
    Copy link
    Mannequin

    danielhrisca mannequin commented Nov 13, 2020

    That would be perfect and would help a lot with timings/synchronization across multiple processes.

    Which Pyhton version will get this fix?

    @danielhrisca
    Copy link
    Mannequin

    danielhrisca mannequin commented Nov 14, 2020

    Under the hood perf_counter_ns already uses the two winapi calls (see

    if (!QueryPerformanceFrequency(&freq)) {
    ) just that it also sets up a static variable as a time origin which makes it process-wide instead of system-wide

    @vstinner
    Copy link
    Member

    I wrote PR 23284 to make time.perf_counter() on Windows and time.monotonic() on macOS system-wide (remove the offset computed at startup). I also suggested the usage of the _ns() variant functions in the documentation.

    @vstinner
    Copy link
    Member

    New changeset 3df5c68 by Victor Stinner in branch 'master':
    bpo-37205: time.perf_counter() and time.monotonic() are system-wide (GH-23284)
    3df5c68

    @vstinner
    Copy link
    Member

    New changeset ae6cd7c by Victor Stinner in branch 'master':
    bpo-37205: time.time() cannot fail with fatal error (GH-23314)
    ae6cd7c

    @vstinner
    Copy link
    Member

    I close the issue, it's now fixed in master. Thanks for the bug report Ken Healy.

    While Python 3.8 and 3.9 are also affected, I prefer to leave them unchanged to avoid bad surprises during a minor update (3.x.y bugfix release). If you disagree, please speak up!

    The regression was introduced in Python 3.7 by:

    commit bdaeb7d
    Author: Victor Stinner <victor.stinner@gmail.com>
    Date: Mon Oct 16 08:44:31 2017 -0700

    bpo-31773: _PyTime_GetPerfCounter() uses _PyTime_t (GH-3983)
    
    * Rewrite win_perf_counter() to only use integers internally.
    * Add _PyTime_MulDiv() which compute "ticks * mul / div"
      in two parts (int part and remaining) to prevent integer overflow.
    * Clock frequency is checked at initialization for integer overflow.
    * Enhance also pymonotonic() to reduce the precision loss on macOS
      (mach_absolute_time() clock).
    

    This change was related to the implementation of the PEP-564.

    If you are curious about this work, I wrote two articles:

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants