FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-100982: Break up `COMPARE_AND_BRANCH` by brandtbucher · Pull Request #102801 · python/cpython · GitHub

/ cpython Public

GH-100982: Break up COMPARE_AND_BRANCH - #102801

Merged
brandtbucher merged 15 commits into
python:mainfrom
brandtbucher:compare-and-not-branch
Mar 23, 2023
ConversationCommits (15)ChecksFiles changed
Merged

GH-100982: Break up COMPARE_AND_BRANCH#102801
brandtbucher merged 15 commits into
python:mainfrom
brandtbucher:compare-and-not-branch

Conversation

brandtbucher commented Mar 17, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

COMPARE_AND_BRANCH (and its specializations) are a bit weird. It's basically an "adaptive superinstruction" that's present in co_code, and its semantics and lifecycle are different from that of any other instruction. Not only can it be harder to reason about, but it also makes changes (like quickening in the compiler) more awkward.

However, it appears to be only a small improvement over COMPARE_OP. This branch replaces the COMPARE_AND_BRANCH family with a "normal" COMPARE_OP one; here are some microbenchmarks for specializations with and without conditional jumps (non-debug, non-PGO):

main:

$ ./python -m timeit -s "x = 0.0" "x == x"
20000000 loops, best of 5: 12.7 nsec per loop
$ ./python -m timeit -s "x = 0" "x == x"
20000000 loops, best of 5: 11 nsec per loop
$ ./python -m timeit -s "x = '0'" "x == x"
20000000 loops, best of 5: 11.2 nsec per loop
$ ./python -m timeit -s "x = 0.0" "None if x == x else None"
20000000 loops, best of 5: 10.8 nsec per loop
$ ./python -m timeit -s "x = 0" "None if x == x else None"
20000000 loops, best of 5: 11.4 nsec per loop
$ ./python -m timeit -s "x = '0'" "None if x == x else None"
20000000 loops, best of 5: 11.3 nsec per loop

This branch:

$ ./python -m timeit -s "x = 0.0" "x == x"
50000000 loops, best of 5: 8.76 nsec per loop
$ ./python -m timeit -s "x = 0" "x == x"
50000000 loops, best of 5: 8.89 nsec per loop
$ ./python -m timeit -s "x = '0'" "x == x"
50000000 loops, best of 5: 8.83 nsec per loop
$ ./python -m timeit -s "x = 0.0" "None if x == x else None"
20000000 loops, best of 5: 11.6 nsec per loop
$ ./python -m timeit -s "x = 0" "None if x == x else None"
20000000 loops, best of 5: 11.8 nsec per loop
$ ./python -m timeit -s "x = '0'" "None if x == x else None"
20000000 loops, best of 5: 12 nsec per loop

So COMPARE_AND_BRANCH only gets us a <1ns improvement in the (common) branching case, and it costs us about 2-4ns in the (uncommon) non-branching case. Honestly, I don't think that's enough of a win.

(Benchmarks are slightly slower, but in the noise.)

brandtbucher added performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 17, 2023
brandtbucher self-assigned this Mar 17, 2023
bedevere-bot mentioned this pull request Mar 17, 2023
Closed
markshannon approved these changes Mar 20, 2023
View reviewed changes

markshannon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good.

brandtbucher merged commit 0444ae2 into python:main Mar 23, 2023
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
pythonGH-100982: Break up COMPARE_AND_BRANCH (pythonGH-102801)
6183cbd
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
pythonGH-100982: Break up COMPARE_AND_BRANCH (pythonGH-102801)
73558f2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

markshannon markshannon approved these changes
iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner

Assignees

brandtbucher

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL