A.M. Kuchling added the comment:
From reading the discussion thread, it looks like the consensus is to not
apply this set of patches because the speed-up is unfortunately small.
Closing as won't-fix; please re-open if someone wishes to pursue this again.
--
nosy: +akuchling
Stefan Behnel added the comment:
One more comment: I also benchmarked the change in long_true_div() now and
found that it's only a minor improvement for large numbers and a
*pessimisation* for small numbers:
Before:
$ ./python -m timeit -s 'x = 5' 'x / -1'
1000 loops, best of 3: 0.0313
New submission from Stefan Behnel:
The attached patch adds fast paths for PyLong division by 1 and -1, as well as
dividing 0 by something. This was found helpful for fractions normalisation, as
the GCD that is divided by can often be |1|, but firing up the whole division
machinery for this
Serhiy Storchaka added the comment:
Perhaps it would be worth to special case multiplying on 0, 1 and -1 and adding
0, 1 and -1 too.
--
stage: - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
STINNER Victor added the comment:
Any optimization requires a benchmark. What is the speedup?
--
nosy: +haypo
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
___
STINNER Victor added the comment:
I proposed an optimization for x 0 (as part of a larger patch to optimize
2 ** x) but the issue was rejected:
http://bugs.python.org/issue21420#msg217802
Mark Dickson wrote (msg217863):
There are many, many tiny optimisations we *could* be making in
Stefan Behnel added the comment:
Attaching a similar patch for long_mul().
--
Added file: http://bugs.python.org/file36730/mul_by_1_fast_path.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
Stefan Behnel added the comment:
Any optimization requires a benchmark. What is the speedup?
I gave numbers in ticket #22464.
Since many Fraction input values can already be normalised for some reason, the
following change shaves off almost 30% of the calls to
PyNumber_InPlaceFloorDivide()
Stefan Behnel added the comment:
@Serhiy: moving the fast path into l_divmod() has the disadvantage of making it
even more complex because we'd then also want to determine the modulus, but
only if requested, and it can be 1, 0 or -1, depending on the second value.
Sounds like a lot more if's.
Stefan Behnel added the comment:
Combined patch for both mul and div that fixes the return value of
long_true_div(), as found by Serhiy, and removes the useless change in
long_divrem(), as found by Antoine. Thanks!
All test_long.py tests pass now.
--
Added file:
Stefan Behnel added the comment:
@Serhiy: please ignore my comment in msg227599. I'll submit a patch that moves
the specialisation to l_divmod().
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
Stefan Behnel added the comment:
Thanks for the reviews, here's a new patch.
--
Added file: http://bugs.python.org/file36732/mul_div_by_1_fast_path_2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
Changes by Stefan Behnel sco...@users.sourceforge.net:
Removed file: http://bugs.python.org/file36732/mul_div_by_1_fast_path_2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
___
Stefan Behnel added the comment:
Sorry, last patch version contained a use before type check bug.
--
Added file: http://bugs.python.org/file36733/mul_div_by_1_fast_path_3.patch
___
Python tracker rep...@bugs.python.org
Stefan Behnel added the comment:
Here is an incremental patch that adds fast paths for adding and subtracting 0.
Question: the module calls long_long() in some places (e.g. long_abs()) and
thus forces the return type to be exactly a PyLong and not a subtype. My
changes use a plain
Antoine Pitrou added the comment:
Le 26/09/2014 12:57, Stefan Behnel a écrit :
Question: the module calls long_long() in some places (e.g.
long_abs()) and thus forces the return type to be exactly a PyLong and
not a subtype. My changes use a plain incref+return input value in
some places.
Changes by Stefan Behnel sco...@users.sourceforge.net:
Added file: http://bugs.python.org/file36736/mul_div_by_1_fast_path_3.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
___
Stefan Behnel added the comment:
Ok, updating both patches.
--
Added file: http://bugs.python.org/file36735/add_sub_0_fast_path_2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
Stefan Behnel added the comment:
I reran the fractions benchmark over the final result and the overall gain
turned out to be, well, small. It's a clearly reproducible 2-3% faster. That's
not bad for the macro impact of a micro-optimisation, but it's not a clear
argument for throwing more code
Serhiy Storchaka added the comment:
Oh, such small gain and only on one specific benchmark not included still in
standard benchmark suite, looks discourage. May be other benchmarks have gain
from these changes?
--
___
Python tracker
STINNER Victor added the comment:
2-3% faster
3% is not enough to justify the change.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22501
___
Stefan Behnel added the comment:
Since Serhiy gave another round of valid feedback, here's an updated patch.
--
Added file: http://bugs.python.org/file36739/mul_div_by_1_fast_path_3.patch
___
Python tracker rep...@bugs.python.org
Stefan Behnel added the comment:
I callgrinded it again and it confirmed that the gain when doing this inside of
long_div() and friends is way lower than doing it right in Fraction.__new__().
It's not safe to do there, though, as is tests on integers are generally not
a good idea in Python
23 matches
Mail list logo
