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

Jump to bottom

Clumsy dispatching on interpreter entry. #89041

Closed
markshannon opened this issue Aug 10, 2021 · 6 comments
Closed

Clumsy dispatching on interpreter entry. #89041

markshannon opened this issue Aug 10, 2021 · 6 comments
Assignees
@markshannon
Labels
3.11 only security fixes performance Performance or resource usage

Comments

@markshannon
Copy link
Member

BPO 44878
Nosy @markshannon, @neonene
PRs
  • bpo-44878: Add missing DISPATCH() #27715
  • bpo-44878: Further ceval entry cleanup #27725
  • bpo-44878: Remove the switch from the main interpreter loop when using computed gotos. #27726
  • bpo-44878: Remove loop from interpreter. All dispatching is done by gotos. #27727
    		•

    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 = 'https://github.com/markshannon'
closed_at = <Date 2021-08-12.10:54:43.596>
created_at = <Date 2021-08-10.13:42:38.715>
labels = ['3.11', 'performance']
title = 'Clumsy dispatching on interpreter entry.'
updated_at = <Date 2021-08-18.09:37:17.120>
user = 'https://github.com/markshannon'

    bugs.python.org fields:

    activity = <Date 2021-08-18.09:37:17.120>
actor = 'Mark.Shannon'
assignee = 'Mark.Shannon'
closed = True
closed_date = <Date 2021-08-12.10:54:43.596>
closer = 'Mark.Shannon'
components = []
creation = <Date 2021-08-10.13:42:38.715>
creator = 'Mark.Shannon'
dependencies = []
files = []
hgrepos = []
issue_num = 44878
keywords = ['patch']
message_count = 6.0
messages = ['399324', '399383', '399391', '399440', '399823', '399826']
nosy_count = 2.0
nosy_names = ['Mark.Shannon', 'neonene']
pr_nums = ['27715', '27725', '27726', '27727']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue44878'
versions = ['Python 3.11']
    @markshannon
    Copy link
    Member Author

    On entering the interpreter (_PyEval_EvalFrameDefault) we need to check for tracing in order to record the call.
    However, we don't do this cleanly resulting in slow dispatch to the non-quickened instruction on every call/next.

    @markshannon markshannon added the 3.11 only security fixes label Aug 10, 2021
    @markshannon markshannon self-assigned this Aug 10, 2021
    @markshannon markshannon added performance Performance or resource usage 3.11 only security fixes labels Aug 10, 2021
    @markshannon markshannon self-assigned this Aug 10, 2021
    @markshannon markshannon added the performance Performance or resource usage label Aug 10, 2021
    @markshannon
    Copy link
    Member Author

    New changeset 3f3d5dc by Mark Shannon in branch 'main':
    bpo-44878: _PyEval_EvalFrameDefault readability improvements (GH-27725)
    3f3d5dc

    @markshannon
    Copy link
    Member Author

    New changeset f66d00f by Mark Shannon in branch 'main':
    bpo-44878: Remove the switch from the main interpreter loop when using computed gotos. (GH-27726)
    f66d00f

    @markshannon
    Copy link
    Member Author

    New changeset a530a95 by Mark Shannon in branch 'main':
    bpo-44878: Remove loop from interpreter. All dispatching is done by gotos. (GH-27727)
    a530a95

    @neonene
    Copy link
    Mannequin

    neonene mannequin commented Aug 18, 2021

    FYI, PR27727 ("Remove loop...") seems to be a bit slower than the previous commit (f08e6d1) on my Windows build (msvc14.29.16.10). pyperformance shows that

    Windows x64 PGO: 34 slower, 11 faster, 13 not significant, Geometric mean: 1.02x slower
    Windows x86 PGO: 28 slower, 17 faster, 13 not significant, Geometric mean: 1.02x slower

    Undoing PR27727 on current cpython-main branch also get speed-ups by 1-2% on average.

    @markshannon
    Copy link
    Member Author

    I'm somewhat surprised by that. After all, the only change in control flow was the change from a break to a goto in exception handling.
    I would have expected PR27726 to have made much more difference.

    There are a few possibilities, including:

    1. It's just a random fluctuation from tiny changes in alignment.
    2. MSVC aligns loops, but not switches, or vice-versa, and that makes a systematic difference.

    I suspect that it is (1), but this is a bit worrying nonetheless.

    @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
    Assignees

    @markshannon markshannon

    Labels
    3.11 only security fixes performance Performance or resource usage
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @markshannon