GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-100982: Break up `COMPARE_AND_BRANCH` by brandtbucher · Pull Request #102801 · python/cpython

Conversation

@brandtbucher

@brandtbucher brandtbucher commented Mar 17, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.)

  • Issue: Specialization of some instructions does not conform to PEP 659, and prevents PEP 669 #100982

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
brandtbucher added 11 commits February 1, 2023 12:58
@brandtbucher
Get rid of COMPARE_AND_BRANCH
53e50c4
@brandtbucher
Embed the mask in the oparg again
20b59ac
@brandtbucher
Catch up with main
781c986
@brandtbucher
fixup
49083fa
@brandtbucher
Catch up with main
39ace64
@brandtbucher
Catch up with main
0754813
@brandtbucher
Bump magic number
e93b6fe
@brandtbucher
Fixup bytecodes.c
fd42c63
@brandtbucher
Catch up with main
d42ac00
@brandtbucher
blurb add
eaf0fc6
@brandtbucher
fixup
e911d60
@brandtbucher brandtbucher added performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 17, 2023
@brandtbucher
brandtbucher requested a review from markshannon as a code owner March 17, 2023 23:50
@brandtbucher brandtbucher self-assigned this Mar 17, 2023
@brandtbucher
brandtbucher requested a review from iritkatriel as a code owner March 17, 2023 23:50
@bedevere-bot bedevere-bot mentioned this pull request Mar 17, 2023
Specialization of some instructions does not conform to PEP 659, and prevents PEP 669 #100982
Closed
@bedevere-bot bedevere-bot added the awaiting core review label Mar 17, 2023
brandtbucher added 2 commits March 19, 2023 20:27
@brandtbucher
Catch up with main
29d3966
@brandtbucher
Sorry, Eric!
94b3836
markshannon
markshannon approved these changes Mar 20, 2023
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Mar 20, 2023
brandtbucher added 2 commits March 23, 2023 12:16
@brandtbucher
Catch up with main
31a13a4
@brandtbucher
Catch up with main (again)
ac5a8e5
@brandtbucher
brandtbucher merged commit 0444ae2 into python:main Mar 23, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 23, 2023
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
@brandtbucher @Fidget-Spinner
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
@brandtbucher @warsaw
pythonGH-100982: Break up COMPARE_AND_BRANCH (pythonGH-102801)
73558f2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@brandtbucher 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.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@brandtbucher @markshannon @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.