gh-100239: specialize bitwise logical binary ops on ints by iritkatriel · Pull Request #128927 · python/cpython · GitHub

This adds specialisations for bitwise |, &, ^ on non-negative ints.

I'm not adding more in the same PR so we can more easily bisect in the future if we need to.

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

…e int

Why restrict to nonnegative longs here? If we do restrict, then we can replace the calls _PyLong_CompactValue with direct access to op->long_value.ob_digit[0]

Because negative ints need more work at runtime and I don't think they're common with bitwise logical ops.

The extra work is already done by _PyLong_CompactValue or am missing something? The output of ls OP rhs might not be a compact int, but there are no guards for the output type.

The extra work is already done by _PyLong_CompactValue or am missing something?

No, I think you're right. Good point.

The output of ls OP rhs might not be a compact int, but there are no guards for the output type.

For bitwise logical operators we should expect the results to have the same size as the inputs.

…e-100239.7_HpBU.rst Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>

The stats are weird with this PR, lot of misses: https://github.com/faster-cpython/benchmarking-public/blob/main/results/bm-20250116-3.14.0a4%2B-c57fe46/bm-20250116-azure-x86_64-iritkatriel-binaryops-3.14.0a4%2B-c57fe46-pystats-vs-base.md

I think non-negative should be dropped in the news entry now?

I think non-negative should be dropped in the news entry now?

And maybe in the title of this pull request, too?

It looks like we are seeing a high proportion of specialization failures for non-compact ints with the & operator.
I don't know why that would be. My guess is that some of the benchmarks are using ints as bit vectors and using more than one digit.

Why three tables, rather than one?

A single table would need to have a list of (guard, action) pairs for each OP. So it's a table of tables. Same thing basically.

It looks like we are seeing a high proportion of specialization failures for non-compact ints with the & operator. I don't know why that would be. My guess is that some of the benchmarks are using ints as bit vectors and using more than one digit.

Just a wild guess: could those be from enums, which derive from int? Especially flag enums?

Just a wild guess: could those be from enums, which derive from int? Especially flag enums?

I don't think so. We check for int with PyLong_CheckExact.

I suspect the misses might be bm_pyflate:

https://github.com/python/pyperformance/blob/1d9261a7da8fcaa642a36181db8e7c4a306a1303/pyperformance/data-files/benchmarks/bm_pyflate/run_benchmark.py#L153-L163

Other python constructions that would causes misses (but are not in pyperformance afaics) are uuid.uuid4 (and variants) and bitwise logical operations on the hash of python objects.

I suspect the misses might be bm_pyflate:

https://github.com/python/pyperformance/blob/1d9261a7da8fcaa642a36181db8e7c4a306a1303/pyperformance/data-files/benchmarks/bm_pyflate/run_benchmark.py#L153-L163

Could be.

https://github.com/faster-cpython/benchmarking/blob/main/results/bm-20250121-3.14.0a4%2B-6476205/bm-20250121-azure-x86_64-iritkatriel-binaryops-3.14.0a4%2B-6476205-pystats-pyflate.md

(Would be nice if the report was rendered so that it's easy to see which benchmark contributed to a stat.)

Do we have benchmarking numbers for this?

Do we have benchmarking numbers for this?

https://github.com/faster-cpython/benchmarking-public/tree/main/results/bm-20250121-3.14.0a4%2B-6476205

(mdboom edited URL to the public one)

Performance looks neutral within the noise.

Were we expecting a speedup on any particular benchmark, or is there a micro-benchmark that shows a speedup?

Performance looks neutral within the noise.

Were we expecting a speedup on any particular benchmark, or is there a micro-benchmark that shows a speedup?

Here are some microbenchmark numbers:

Old:

>>> for i in range(5):
...     timeit("for i in range(10000):\n\tb = a&i", number=10000, setup="a = 1")
...     
2.517115215305239
2.476386756170541
2.484560175333172
2.4904085099697113
2.4953759447671473

New:

>>> for i in range(5):
...     timeit("for i in range(10000):\n\tb = a&i", number=10000, setup="a = 1")
...     
2.2215038347058
2.1580611928366125
2.1611814140342176
2.154480631928891
2.1730910362675786

My interpretation of the results is:

Shows a measurable speedup for the operators and classes it targets
Doesn't show an overall slowdown
This specialization's effectiveness is limited by the small range of compact ints. Something like faster-cpython/ideas#548 should mostly fix that.

@iritkatriel do you agree?

If so, let's merge this.

pythongh-100239 : specialize bitwise logical binary ops on non-negativ…

c6f2deb

…e int

bedevere-app Bot mentioned this pull request Jan 16, 2025

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

Open

bedevere-app Bot added the awaiting core review label Jan 16, 2025

Merge branch 'main' into binaryops

c57fe46

iritkatriel requested review from markshannon and mdboom January 16, 2025 22:57

eendebakpt reviewed Jan 16, 2025

View reviewed changes

eendebakpt reviewed Jan 17, 2025

View reviewed changes

Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-01-16-22-54-12.gh-issue-100239.7_HpBU.rst Outdated

the globals thing

862bc67

iritkatriel requested a review from ericsnowcurrently as a code owner January 17, 2025 09:03

Update Misc/NEWS.d/next/Core_and_Builtins/2025-01-16-22-54-12.gh-issu…

69e66f5

…e-100239.7_HpBU.rst Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>

iritkatriel added 4 commits January 17, 2025 18:09

add checks in test

0173413

no need to exclude negative

61d8bb3

EXIT_IF --> DEOPT_IF

1f48300

add specialization failure kind stats

e43ac58

kumaraditya303 reviewed Jan 18, 2025

View reviewed changes

Comment thread Python/specialize.c

chris-eibl reviewed Jan 18, 2025

View reviewed changes

iritkatriel commented Jan 18, 2025

View reviewed changes

Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-01-16-22-54-12.gh-issue-100239.7_HpBU.rst Outdated

remove non-negative

6916df4

chris-eibl reviewed Jan 19, 2025

View reviewed changes

Comment thread Python/specialize.c Outdated

add inplace ops

6476205

iritkatriel changed the title gh-100239: specialize bitwise logical binary ops on non-negative int gh-100239: specialize bitwise logical binary ops on ints Jan 21, 2025

iritkatriel added 2 commits January 21, 2025 17:10

Merge remote-tracking branch 'upstream/main' into binaryops

8448b4b

PyLong_FromLong --> PyLong_FromSsize_t

634b388

chris-eibl mentioned this pull request Jan 21, 2025

Missing fast path in PyLong_From*() functions for compact integers #129149

Closed

markshannon reviewed Jan 22, 2025

View reviewed changes

chris-eibl mentioned this pull request Jan 22, 2025

gh-129149: Add Missing fast path in PYLONG_FROM_UINT macro for compact integers #129168

Merged

Merge branch 'main' into binaryops

6041025

markshannon self-requested a review January 29, 2025 09:24

markshannon approved these changes Jan 29, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels Jan 29, 2025

iritkatriel merged commit 4815131 into python:main Jan 29, 2025

bedevere-app Bot removed the awaiting merge label Jan 29, 2025

+                  return (is_nonnegative_compactlong(lhs) && is_nonnegative_compactlong(rhs));
+              }
+              #define NONNEGATIVE_LONGS_ACTION(NAME, OP) \

                   LOOKUP_SPEC(compactlong_float_specs, oparg);
                   LOOKUP_SPEC(float_compactlong_specs, oparg);
+                  LOOKUP_SPEC(compactlongs_specs, oparg);

gh-100239: specialize bitwise logical binary ops on ints by iritkatriel · Pull Request #128927 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-100239: specialize bitwise logical binary ops on ints - #128927

gh-100239: specialize bitwise logical binary ops on ints#128927iritkatriel merged 13 commits intopython:mainpython/cpython:mainfrom iritkatriel:binaryopsiritkatriel/cpython:binaryopsCopy head branch name to clipboard

Conversation

iritkatriel commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eendebakpt Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

iritkatriel Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

eendebakpt Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

iritkatriel Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iritkatriel commented Jan 17, 2025

Uh oh!

Uh oh!

chris-eibl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chris-eibl commented Jan 21, 2025

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

markshannon Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

iritkatriel Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

chris-eibl commented Jan 22, 2025

Uh oh!

iritkatriel commented Jan 22, 2025

Uh oh!

eendebakpt commented Jan 22, 2025

Uh oh!

iritkatriel commented Jan 22, 2025

Uh oh!

iritkatriel commented Jan 22, 2025

Uh oh!

markshannon commented Jan 27, 2025

Uh oh!

iritkatriel commented Jan 28, 2025 • edited by mdboom Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markshannon commented Jan 28, 2025

Uh oh!

iritkatriel commented Jan 29, 2025

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gh-100239: specialize bitwise logical binary ops on ints#128927
iritkatriel merged 13 commits into
python:mainfrom
iritkatriel:binaryops

iritkatriel commented Jan 16, 2025 •
edited
Loading

iritkatriel commented Jan 28, 2025 •
edited by mdboom
Loading