classification
Title: Optimize PyLong_AsDouble for single-digit longs
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: yselivanov Nosy List: mark.dickinson, pitrou, python-dev, serhiy.storchaka, skrah, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2016-02-05 01:55 by yselivanov, last changed 2016-02-08 21:13 by yselivanov. This issue is now closed.

Files
File name Uploaded Description Edit
as_double.patch yselivanov, 2016-02-05 01:54 review
Messages (15)
msg259615 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-02-05 01:54
The attached patch drastically speeds up PyLong_AsDouble for single digit longs:


-m timeit -s "x=2" "x*2.2 + 2 + x*2.5 + 1.0 - x / 2.0 + (x+0.1)/(x-0.1)*2 + (x+10)*(x-30)"

with patch: 0.414
without: 0.612

spectral_norm: 1.05x faster.    The results are even better when paired with patch from issue #21955.
msg259703 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-02-06 00:40
New changeset 986184c355e8 by Yury Selivanov in branch 'default':
Issue #26288: Optimize PyLong_AsDouble.
https://hg.python.org/cpython/rev/986184c355e8
msg259704 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-02-06 00:40
Thanks a lot for the review, Serhiy!
msg259715 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-02-06 02:24
Nice enhancement.

/* Fast path; single digit will always fit decimal.
   This improves performance of FP/long operations by at
   least 20%. This is even visible on macro-benchmarks.
*/

I'm not sure that "spectral_norm" can be qualified as macro-benchmark. It's more a microbenchmark on numerics, no? I'm just nitpicking, your patch is fine ;-)
msg259723 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2016-02-06 10:57
Actually, please fix the comment. We don't want someone wondering what those "macro-benchmarks" are.
msg259724 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-02-06 11:23
Additionally, "single digit will always fit a double"?
msg259736 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-02-06 17:07
> Actually, please fix the comment. We don't want someone wondering what those "macro-benchmarks" are.

If spectral-norm and nbody aren't good benchmarks then let's remove them from our benchmarks suite.

I'll remove that comment anyways, as it doesn't make a lot of sense :)

> Additionally, "single digit will always fit a double"?

What's wrong about that phrase?  Would this be better: "It's safe to cast a single-digit long (31 bits) to double"?
msg259737 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-02-06 17:18
Sorry, I was a bit brief: The current comment says "decimal" instead of "double". It should be changed to "double".
msg259738 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2016-02-06 17:18
Le 06/02/2016 18:07, Yury Selivanov a écrit :
> 
>> Actually, please fix the comment. We don't want someone wondering what those "macro-benchmarks" are.
> 
> If spectral-norm and nbody aren't good benchmarks then let's remove
> them from our benchmarks suite.

Probably, yes.
msg259739 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2016-02-06 17:19
Actually, let me refine that. nbody and spectral-norm don't make sense
for people running CPython. Perhaps people running PyPy might care about
their performance... (though PyPy is supposed to support a subset of
Numpy too)
msg259740 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-02-06 17:21
New changeset cfb77ccdc23a by Yury Selivanov in branch 'default':
Issue #26288: Fix comment
https://hg.python.org/cpython/rev/cfb77ccdc23a
msg259741 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-02-06 17:23
> Sorry, I was a bit brief: The current comment says "decimal" instead of "double". It should be changed to "double".

Oh, got it now, sorry.  I rephrased the comment a bit, hopefully it's better now.  Please check.  Thanks!
msg259777 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-02-07 10:16
The comment looks good to me -- I'll stay out of the benchmarking issue, I didn't check any of that. :)
msg259785 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-02-07 11:56
Well I *did* run the decimal/float milli-benchmark now and it shows at least 15% improvement for floats consistently.

Given that the official benchmark suite does not seem to be very stable either (#21955), I actually prefer small and well-understood benchmarks.
msg259886 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-02-08 21:13
I'm not sure why this issue is open... Closing it.
History
Date User Action Args
2016-02-08 21:13:53yselivanovsetstatus: open -> closed

messages: + msg259886
2016-02-07 11:56:22skrahsetmessages: + msg259785
2016-02-07 10:16:20skrahsetmessages: + msg259777
2016-02-06 17:23:13yselivanovsetmessages: + msg259741
2016-02-06 17:21:35python-devsetmessages: + msg259740
2016-02-06 17:19:26pitrousetmessages: + msg259739
2016-02-06 17:18:17pitrousetmessages: + msg259738
2016-02-06 17:18:07skrahsetmessages: + msg259737
2016-02-06 17:12:32mark.dickinsonsetnosy: + mark.dickinson
2016-02-06 17:07:13yselivanovsetmessages: + msg259736
2016-02-06 11:23:54skrahsetnosy: + skrah
messages: + msg259724
2016-02-06 10:57:17pitrousetstatus: closed -> open
assignee: yselivanov
messages: + msg259723
2016-02-06 02:24:52vstinnersetmessages: + msg259715
2016-02-06 00:40:58yselivanovsetstatus: open -> closed
type: performance
messages: + msg259704

resolution: fixed
stage: resolved
2016-02-06 00:40:10python-devsetnosy: + python-dev
messages: + msg259703
2016-02-05 01:55:00yselivanovcreate