bpo-41902: Add fast path for long_div if the divisor is one #22480
Conversation
|
Can you please run benchmarks? For example, benchmark:
For multiple values of x:
|
| @support.cpython_only | ||
| def test_division_with_one(self): | ||
| for n in range(10000000): | ||
| self.assertEqual(n, n // 1) |
|
|
||
| @support.cpython_only | ||
| def test_division_with_one(self): | ||
| for n in range(10000000): |
There was a problem hiding this comment.
Well, I don't think that it's worth it to test 10000000 numbers. Just test a bunch of various numbers made of a single digit. Like:
(-MASK, -10, -5, 0, 1, 2, 42, MASK)
|
|
||
|
|
||
| @support.cpython_only | ||
| def test_division_with_one(self): |
There was a problem hiding this comment.
If you test assertIs(), please add a comment mentioning bpo-41902 and explain that it's a micro-optimization which returns x for x//1 if x is an int made of a single digit.
| def test_division_with_one(self): | ||
| for n in range(10000000): | ||
| self.assertEqual(n, n // 1) | ||
|
|
There was a problem hiding this comment.
Test also that assertEqual(type(x//1), int) where x is an int subclass.
| long_div(PyObject *a, PyObject *b) | ||
| { | ||
| PyLongObject *div; | ||
| if (b == _PyLong_One && PyLong_CheckExact(a)) { |
There was a problem hiding this comment.
This branch may impact the normal path, we certainly should benchmark this with PGO and cpu isolation.
|
@mdickinson doesn't like this approach, see the discussion on the issue: https://bugs.python.org/issue41902#msg377773 |
https://bugs.python.org/issue41902