GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-100239: specialize bitwise logical binary ops on ints by iritkatriel · Pull Request #128927 · python/cpython Conversation
|
| return (is_nonnegative_compactlong(lhs) && is_nonnegative_compactlong(rhs)); | ||
| } | ||
|
|
||
| #define NONNEGATIVE_LONGS_ACTION(NAME, OP) \ |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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]
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Because negative ints need more work at runtime and I don't think they're common with bitwise logical ops.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The extra work is already done by
_PyLong_CompactValueor am missing something?
No, I think you're right. Good point.
The output of
ls OP rhsmight 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…e-100239.7_HpBU.rst Co-authored-by: Pieter Eendebak <[email protected]>
|
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
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I think non-negative should be dropped in the news entry now?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
And maybe in the title of this pull request, too? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
||
| LOOKUP_SPEC(compactlong_float_specs, oparg); | ||
| LOOKUP_SPEC(float_compactlong_specs, oparg); | ||
| LOOKUP_SPEC(compactlongs_specs, oparg); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Why three tables, rather than one?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Just a wild guess: could those be from enums, which derive from int? Especially flag enums? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
I don't think so. We check for int with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I suspect the misses might be Other python constructions that would causes misses (but are not in pyperformance afaics) are |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
(Would be nice if the report was rendered so that it's easy to see which benchmark contributed to a stat.) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Do we have benchmarking numbers for this? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
(mdboom edited URL to the public one) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Here are some microbenchmark numbers: Old: New: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
My interpretation of the results is:
@iritkatriel do you agree?
If so, let's merge this.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.