classification
Title: Clean up division fast paths in Objects/longobject.c
Type: performance Stage: commit review
Components: Versions: Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: mark.dickinson, pitrou, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-09-10 10:46 by mark.dickinson, last changed 2017-07-22 10:09 by mark.dickinson.

Files
File name Uploaded Description Edit
divmod_fastpath.patch mark.dickinson, 2016-09-10 10:46 review
Messages (6)
msg275618 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2016-09-10 10:46
We seem to have ended up with redundant fast path checks for division in longobject.c: long_div has a fast path check, but the long_div slow path calls l_divmod, which then does a second, identical, fast path check. long_mod has similar behaviour. long_divmod, however, has no fast path, so relies on the one from l_divmod.

This patch removes the extra fast path from l_divmod, and then adds a top-level fast path check to long_divmod.
msg275619 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2016-09-10 10:47
N.B. The patch also tweaks the fast path condition to *include* the common case of a dividend of 0, and *exclude* the rare case of a negative divisor. (The latter change helps to keep the fast path code simple.)
msg275675 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2016-09-10 19:08
Serhiy: thanks for the review. Replying here, since I get a 500 error every time I try to reply from Rietveld.

I'd rather not rely on either NSMALLPOSINTS > 0 or on digit 0 existing when Py_SIZE is 0. We don't rely on that elsewhere, and the code should stay simple where possible.
msg275679 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-09-10 19:14
The patch LGTM. I'll open separate issue for guaranteeing that a->ob_digit[0] is 0 in case of Py_SIZE(a) == 0 and using this fact for simplifying and optimizing the code.
msg298843 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-07-22 09:32
What's the status of this?
msg298845 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-07-22 10:09
> What's the status of this?

Forgotten, rather than abandoned. :-( I'll unassign so that someone else can pick this up. I believe the patch is ready to go, except that of course now it needs a PR.
History
Date User Action Args
2017-07-22 10:09:56mark.dickinsonsetassignee: mark.dickinson ->
messages: + msg298845
2017-07-22 09:32:24pitrousetnosy: + pitrou
messages: + msg298843
2016-09-10 19:14:38serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg275679
2016-09-10 19:08:50mark.dickinsonsetmessages: + msg275675
2016-09-10 10:47:58mark.dickinsonsetmessages: + msg275619
2016-09-10 10:46:13mark.dickinsoncreate