New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-96421: Insert shim frame on entry to interpreter #96319
base: main
Are you sure you want to change the base?
Conversation
…nt safety checks but ASAN gets tripped up in some cases.
Does this add much extra CPU/stack/memory overhead for calling Python from C? Or alternating py/c/py/c/py calls? Probably not the most important use-cases, but I would hope that they wouldn't pessimize too much. |
|
The performance impact of this negative but I want to merge anyway because:
|
Thanks for taking the time to do this. There's definitely a lot of subtlety involved.
In general, I think that this adds a fair amount of complexity that wasn't present before (I've recently gained new appreciation for the value of being able to reason about the web of frames that exist in 3.11+, particularly things like visibility and ownership rules). If you're confident that we can recoup the perf cost and complexity with work that builds upon this, though, I think it makes sense as a first step towards that.
Also, I think that the growing set of rules around things like "ownership", "completeness", and "state" are becoming a real maintenance burden. We might consider tightening up those concepts in 3.12, because there are lots of complex relationships and invalid states that are crucial to understand when working on (or with) interpreter internals. I'm thinking not only about us, but also third-party libraries like Greenlet and forks like Cinder.
Misc/NEWS.d/next/Core and Builtins/2022-09-09-10-37-34.gh-issue-96421.cyg33z.rst
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Core and Builtins/2022-10-19-15-59-08.gh-issue-96421.e22y3r.rst
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Core and Builtins/2022-10-19-15-59-08.gh-issue-96421.e22y3r.rst
Outdated
Show resolved
Hide resolved
When you're done making the requested changes, leave the comment: |
I have made the requested changes; please review again |
Thanks for making the requested changes! @brandtbucher: please review the changes made to this pull request. |
I don't know if I'll have time to review again today (still need to prepare my talk for the release stream). Is this able to wait until next week? If not, I can try to make time. |
It can wait until next week |
Simplifies
RETURN_VALUE
,RETURN_GENERATOR
andYIELD_VALUE
as they no longer need to check if the current frame is the entry frame.Should allow specialization of
FOR_ITER
andSEND
for generators and coroutines.Needs docs and news.
Performance impact is about zero