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-100239: specialize long tail of binary operations by iritkatriel · Pull Request #128722 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Jan 10, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This implements part of #100239: the four arithmetic ops between int, float and float, int.

Microbenchmarks:

Old:

>>> timeit("for i in range(10000):\n\tb = a+i", number=100000, setup="a = 1.0")
37.0931663562078
>>> timeit("for i in range(10000):\n\tb = i+a", number=100000, setup="a = 1.0")
40.84421204589307

New:

>>> timeit("for i in range(10000):\n\tb = a+i", number=100000, setup="a = 1.0")
31.263000949984416
>>> timeit("for i in range(10000):\n\tb = i+a", number=100000, setup="a = 1.0")
31.243564788019285

So performance is 20-30% better, and also more uniform (old is 10% slower for int+float compared to float+int).

Full benchmarks don't show an overall speedup, but they do show better specialisation stats for BINARY_OP:
https://github.com/faster-cpython/benchmarking-public/tree/main/results/bm-20250110-3.14.0a3+-7264e37#readme

  • Issue: Specialize long tail of binary operations using a table. #100239

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner and dg-pb reacted with rocket emoji
All reactions
  • 🚀 2 reactions
@iritkatriel
pythongh-100239: specialize long tail of binary operations
7264e37
@iritkatriel
iritkatriel requested review from ericsnowcurrently and markshannon as code owners January 10, 2025 23:27
@bedevere-app bedevere-app Bot mentioned this pull request Jan 10, 2025
Specialize long tail of binary operations using a table. #100239
Open
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 10, 2025
@iritkatriel
iritkatriel requested review from eendebakpt and mdboom January 10, 2025 23:27
iritkatriel added 2 commits January 10, 2025 23:54
@iritkatriel
add news
89b4679
@iritkatriel
fix fomatting
6dba21a
@Fidget-Spinner

Fidget-Spinner commented Jan 11, 2025

Copy link
Copy Markdown
Member

Nice! The arithmetic benchmarks show a good speedup in the provided link:

spectral_norm	107 ms	96.3 ms: 1.11x faster
pyflate	480 ms	437 ms: 1.10x faster
chaos	61.1 ms	57.9 ms: 1.06x faster
iritkatriel reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
eendebakpt
eendebakpt reviewed Jan 11, 2025
View reviewed changes

@eendebakpt eendebakpt left a comment

Copy link
Copy Markdown
Contributor

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.

This already looks quite good. To get a feeling for the interfaces I added a few more specializations. See iritkatriel/cpython@gh-100239...eendebakpt:cpython:gh-100239-list-tuple-add

Adding more specializations is quite easy, but if we end up adding more we will need some more macros or tooling (such as for example the TRY_BINARY_SPECIALIZATION in the branch above). Fine to leave that to a followup PR though.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
eendebakpt
eendebakpt reviewed Jan 11, 2025
View reviewed changes
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel and others added 3 commits January 11, 2025 23:26
@iritkatriel
assert rather than deopt if no descr/guard
7bf51d6
@iritkatriel @eendebakpt
Apply suggestions from code review
69112e9 
Co-authored-by: Pieter Eendebak <[email protected]>
@iritkatriel
long --> compactlong
fd88c8d
@Fidget-Spinner Fidget-Spinner mentioned this pull request Jan 12, 2025
Need more non-escaping specializations faster-cpython/ideas#704
Closed
mdboom
mdboom approved these changes Jan 13, 2025
View reviewed changes
Comment thread Include/internal/pycore_code.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jan 13, 2025
iritkatriel added 4 commits January 13, 2025 19:59
@iritkatriel
static specs
f29d8be
@iritkatriel
add test and fix multiply
efe8e0c
@iritkatriel
Merge remote-tracking branch 'upstream/main' into pythongh-100239
c74a196
@iritkatriel
Merge remote-tracking branch 'upstream/main' into pythongh-100239
68c4f34
markshannon
markshannon reviewed Jan 14, 2025
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 promising. We might want to add some filtering based on the class before calling the guard function when specializing. Calling a long chain of guard functions could be expensive.

Looking at the stats, it seems that this doesn't make that much difference to the number of BINARY_OPs that are specialized.
Unfortunately the stats don't tell us which class pairs to add, but I think str % str and str % tuple would be worth a look.
Or we could enhance the stats to give us cls/cls/operator triples, at least for those classes with a small version number?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_code.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Include/internal/pycore_code.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Include/internal/pycore_code.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel added 6 commits January 14, 2025 15:39
@iritkatriel
void _Py_Specialize_BinaryOp. PyBinaryOpSpecializationDescr --> _PyBi…
0432b9a 
…naryOpSpecializationDescr
@iritkatriel
let generator deal with fetching descr from the cache
d87946a
@iritkatriel
Merge remote-tracking branch 'upstream/main' into pythongh-100239
cffb3d6
@iritkatriel
make descr arrays const to please the globals check
ede9e8c
@iritkatriel
Revert "make descr arrays const to please the globals check"
a144cdc 
This reverts commit ede9e8c.
@iritkatriel
add globals to ignored.tsv
e526f4e
@iritkatriel
remove unused function
3d6b60f
@iritkatriel
iritkatriel requested a review from markshannon January 16, 2025 00:04
markshannon
markshannon approved these changes Jan 16, 2025
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.

A couple of suggestions, but nothing blocking.

OOI what was causing the earlier test failures?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_code.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@markshannon markshannon mentioned this pull request Jan 16, 2025
GH-117581: Specialize binary operators by refcount as well as type. #117627
Closed
@iritkatriel

iritkatriel commented Jan 16, 2025

Copy link
Copy Markdown
Member Author

OOI what was causing the earlier test failures?

This was missing: #128892

The test assumes some valid opcode is invalid.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel and others added 3 commits January 16, 2025 11:57
@iritkatriel @markshannon
Update Python/bytecodes.c
988369d 
Co-authored-by: Mark Shannon <[email protected]>
@iritkatriel
write_ptr
16c8ed6
@iritkatriel
Merge branch 'main' into pythongh-100239
47395aa
@markshannon markshannon mentioned this pull request Jan 16, 2025
GH-128914: Remove conditional stack effects from bytecodes.c and the code generators #128918
Merged
@iritkatriel
iritkatriel merged commit 3893a92 into python:main Jan 16, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 16, 2025
@iritkatriel

iritkatriel commented Jan 17, 2025

Copy link
Copy Markdown
Member Author

I repeated the benchmarks with the multiply bug (that prevented specialisation) fixed:
https://github.com/faster-cpython/benchmarking-public/tree/main/results/bm-20250116-3.14.0a4+-3893a92
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou encukou mentioned this pull request Jan 23, 2025
Specialize BINARY_OP by refcount and by type of operands #117581
Closed
@chris-eibl chris-eibl mentioned this pull request Feb 14, 2026
gh-100239: use PyFloat_AS_DOUBLE and _PyLong_IsZero in the float / compactlong specializations #144826
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eendebakpt eendebakpt eendebakpt left review comments
@markshannon markshannon markshannon approved these changes
@mdboom mdboom mdboom approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

5 participants

@iritkatriel @Fidget-Spinner @mdboom @eendebakpt @markshannon
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.