classification
Title: Improve arg clinic code generation for cases with type checking
Type: performance Stage: resolved
Components: Argument Clinic Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: 23867 35582 Superseder:
Assigned To: serhiy.storchaka Nosy List: ammar2, eric.smith, larry, rhettinger, serhiy.storchaka
Priority: low Keywords: patch

Created on 2018-09-28 21:15 by rhettinger, last changed 2019-01-12 06:31 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 9659 merged ammar2, 2018-10-01 18:26
Messages (8)
msg326659 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-09-28 21:15
An arg clinic specification such as

    p: object(subclass_of='&PyTuple_Type')

generates slow code using _PyArg_ParseStack() that has to parse a format string like "O!" to decide to make a type check.  Instead, it should directly generate a branch-predictable test for the type check and then call the much quicker function _PyArg_UnpackStack().

See https://github.com/python/cpython/pull/9628 for an example of this change giving a 30% speedup.
msg326819 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2018-10-01 18:26
I've attached a PR that implements this. From the looks of it, there aren't a lot of uses of subclass_of within argument clinic converted functions at the moment. Additionally, most of the places it is used tend to have some non object arguments so a call to _PyArg_ParseStack() gets placed anyway.

Attached as part of separate commit in the PR is the conversion of some files over to use the new code. These are mostly just to serve as examples. 

While converting would still leave a call to ParseStack in most places, I'd like to do some benchmarking and figure out if there is still a benefit. Like you said, the fact that branch-prediction is easier might be useful given that these functions are usually called with the right types of arguments anyway.
msg326980 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-10-03 15:00
Thank you for your PR Ammar. But I work on more general changes. The first step is issue23867. With implementing the second step this issue can be closed.
msg327012 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2018-10-03 20:12
Aah, I didn't know that ticket existed. Thanks for the link Serhiy, I'll review your PR instead. Leaving it up to Raymond if he wants to close the issue and use the other one to track this.
msg333471 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-01-11 14:09
PR 9659 is mostly superseded by issue35582. But it may contain other changes. Ammar, do you mind to create a new PR or merge the old PR with the current master?
msg333494 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2019-01-11 17:45
Sounds good, I'll take a look at the changes and update the PR accordingly.
msg333509 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2019-01-12 03:18
Great job Serhiy, your work on argument clinic has encompassed the previous PR very well. I've updated it for the changes.
msg333512 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-01-12 06:23
New changeset cb08a71c5c534f33d9486677534dafb087c30e8c by Serhiy Storchaka (Ammar Askar) in branch 'master':
bpo-34838: Use subclass_of for math.dist. (GH-9659)
https://github.com/python/cpython/commit/cb08a71c5c534f33d9486677534dafb087c30e8c
History
Date User Action Args
2019-01-12 06:31:09serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: resolved
2019-01-12 06:23:44serhiy.storchakasetmessages: + msg333512
2019-01-12 03:18:04ammar2setmessages: + msg333509
2019-01-11 17:45:27ammar2setmessages: + msg333494
2019-01-11 14:09:16serhiy.storchakasetmessages: + msg333471
2019-01-05 16:43:58serhiy.storchakasetdependencies: + Argument Clinic: inline parsing code for 1-argument functions, Argument Clinic: inline parsing code for functions with only positional parameters
2018-10-03 20:12:55ammar2setmessages: + msg327012
2018-10-03 15:00:40serhiy.storchakasetmessages: + msg326980
2018-10-01 18:26:20ammar2setnosy: + ammar2

messages: + msg326819
stage: patch review -> (no value)
2018-10-01 18:26:11ammar2setkeywords: + patch
stage: patch review
pull_requests: + pull_request9052
2018-09-28 23:26:42eric.smithsetnosy: + eric.smith
2018-09-28 21:46:15vstinnersetnosy: - vstinner
2018-09-28 21:20:13serhiy.storchakasetassignee: serhiy.storchaka
2018-09-28 21:15:58rhettingercreate