This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Remove C stack use by specializing BINARY_SUBSCR, STORE_SUBSCR, LOAD_ATTR, and STORE_ATTR
Type: performance Stage: patch review
Components: Interpreter Core Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Mark.Shannon Nosy List: Dennis Sweeney, Mark.Shannon, brandtbucher, gvanrossum, pablogsal, pmpp
Priority: normal Keywords: patch

Created on 2021-11-17 09:50 by Mark.Shannon, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 29592 merged Mark.Shannon, 2021-11-17 10:13
PR 30129 merged brandtbucher, 2021-12-15 23:57
Messages (7)
msg406461 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2021-11-17 09:50
We can remove the C stack use and general overhead of calling special methods implemented in Python for attribute access and indexing.

Each operation has a special method that implements it. When that special method is implemented in Python, we should avoid the `tp_xxx` slot machinery and use the same mechanism we use for normal calls to Python functions. 

* BINARY_SUBSCR: `__getitem__`
* STORE_SUBSCR: `__setitem__`
* LOAD_ATTR: `__getattribute__` (and maybe `__getattr__`)
* STORE_ATTR: `__setattr__`

It probably isn't worth bothering with the deletion forms.

The getters (`__getitem__` and `__getattribute__`) are relatively simple, as the call returns the result.

The setters are a bit more complicated as the return value needs to be discarded, so an additional frame which discards the result of the call needs to be inserted.
msg406473 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-11-17 15:58
Of these, presumably LOAD_GETATTR is by far the most used, so should we try that first?
msg406476 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2021-11-17 16:19
I don't think it matter much which we do first.
I happened to do BINARY_SUBSCR first.
msg406477 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-11-17 16:32
That's a good one too, and perhaps simpler.
msg406530 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2021-11-18 11:03
New changeset 21fa7a3e8f99a1a32467f85c877e40cbdafa9da7 by Mark Shannon in branch 'main':
bpo-45829: Specialize BINARY_SUBSCR for __getitem__ implemented in Python. (GH-29592)
https://github.com/python/cpython/commit/21fa7a3e8f99a1a32467f85c877e40cbdafa9da7
msg406573 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python committer) Date: 2021-11-19 07:21
This snippet occurs a couple of times in ceval.c (BINARY_SUBSCR_GETITEM and CALL_FUNCTION_PY_SIMPLE):

            new_frame->previous = frame;
            frame = cframe.current_frame = new_frame;
            new_frame->depth = frame->depth + 1;

Maybe I'm reading it wrong, but I think the last line is just setting new_frame->depth++, leaving new_frame->depth = 1 instead of frame->previous->depth + 1.

I think the second and third lines should be swapped?
msg408687 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2021-12-16 11:08
New changeset 62a8a0c5223f750e22ee381d3cfbdb718cf1cc93 by Brandt Bucher in branch 'main':
bpo-45829: Check `__getitem__`'s version for overflow before specializing (GH-30129)
https://github.com/python/cpython/commit/62a8a0c5223f750e22ee381d3cfbdb718cf1cc93
History
Date User Action Args
2022-04-11 14:59:52adminsetgithub: 89987
2021-12-16 11:08:35Mark.Shannonsetmessages: + msg408687
2021-12-15 23:57:28brandtbuchersetpull_requests: + pull_request28348
2021-12-06 11:43:48pmppsetnosy: + pmpp
2021-11-19 07:21:37Dennis Sweeneysetnosy: + Dennis Sweeney
messages: + msg406573
2021-11-18 11:03:03Mark.Shannonsetmessages: + msg406530
2021-11-17 16:32:32gvanrossumsetmessages: + msg406477
2021-11-17 16:19:19Mark.Shannonsetmessages: + msg406476
2021-11-17 15:58:20gvanrossumsetmessages: + msg406473
2021-11-17 15:16:10gvanrossumsetnosy: + gvanrossum
2021-11-17 10:13:11Mark.Shannonsetkeywords: + patch
stage: patch review
pull_requests: + pull_request27835
2021-11-17 09:50:11Mark.Shannoncreate