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

Rewrite _PyTime_GetWinPerfCounter() for _PyTime_t #75954

Closed
vstinner opened this issue Oct 12, 2017 · 5 comments
Closed

Rewrite _PyTime_GetWinPerfCounter() for _PyTime_t #75954

vstinner opened this issue Oct 12, 2017 · 5 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

BPO 31773
Nosy @vstinner
PRs
  • bpo-31773: time.perf_counter() uses again double #3964
  • bpo-31773: _PyTime_GetPerfCounter() uses _PyTime_t #3983
  • Files
  • round.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 2017-10-24.09:06:08.175>
    created_at = <Date 2017-10-12.14:14:14.674>
    labels = ['interpreter-core', 'type-feature', '3.7']
    title = 'Rewrite _PyTime_GetWinPerfCounter() for _PyTime_t'
    updated_at = <Date 2017-10-24.09:06:08.173>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2017-10-24.09:06:08.173>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-10-24.09:06:08.175>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2017-10-12.14:14:14.674>
    creator = 'vstinner'
    dependencies = []
    files = ['47217']
    hgrepos = []
    issue_num = 31773
    keywords = ['patch']
    message_count = 5.0
    messages = ['304239', '304263', '304327', '304477', '304877']
    nosy_count = 1.0
    nosy_names = ['vstinner']
    pr_nums = ['3964', '3983']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue31773'
    versions = ['Python 3.7']

    @vstinner
    Copy link
    Member Author

    The commit a997c7b of bpo-31415 moved the implementation of time.perf_counter() from Modules/timemodule.c to Python/pytime.c. The change not only moved the code, but also changed the internal type storing time from floatting point number (C double) to integer number (_PyTyime_t = int64_t).

    The drawback of this change is that time.perf_counter() now converts QueryPerformanceCounter() / QueryPerformanceFrequency() double into a _PyTime_t (integer) and then back to double. Two useless conversions required by the _PyTime_t format used in Python/pytime.c. These conversions introduced a loss of precision.

    Try attached round.py script which implements the double <=> _PyTime_t conversions and checks to check for precision loss. The script shows that we loose precision even with a single second for QueryPerformanceFrequency() == 3579545.

    It seems like QueryPerformanceFrequency() now returns 10 ** 7 (10_000_000, resolution of 100 ns) on Windows 8 and newer, but returns 3,579,545 (3.6 MHz, resolution of 279 ns) on Windows 7. It depends maybe on the hardware clock, I don't know. Anyway, whenever possible, we should avoid precision loss of a clock.

    @vstinner vstinner added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Oct 12, 2017
    @vstinner
    Copy link
    Member Author

    New changeset cba9a0c by Victor Stinner in branch 'master':
    bpo-31773: time.perf_counter() uses again double (GH-3964)
    cba9a0c

    @vstinner
    Copy link
    Member Author

    I reopen the issue since I found a solution to only use integer in pytime.c for QueryPerformanceCounter() / QueryPerformanceFrequency() *and* prevent integer overflow.

    @vstinner vstinner reopened this Oct 13, 2017
    @vstinner
    Copy link
    Member Author

    New changeset bdaeb7d by Victor Stinner in branch 'master':
    bpo-31773: _PyTime_GetPerfCounter() uses _PyTime_t (GH-3983)
    bdaeb7d

    @vstinner
    Copy link
    Member Author

    Buildbots seem to be happy, I close the issue.

    See my new PEP-564 for the next chapiter of this story ;-)

    @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 interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant