GH-96421: Insert shim frame on entry to interpreter #96319
Conversation
markshannon
changed the title
…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. |
|
|
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 |
Mostly nits on the changes.
Also, I'd like to discuss this vs. something like #98795 again at our stand-up, if that's okay. This still feels pretty heavy/breaky to me compared to that PR (and I'm still not clear what it actually gives us over that approach).
Misc/NEWS.d/next/Core and Builtins/2022-10-19-15-59-08.gh-issue-96421.e22y3r.rst
Outdated
Show resolved
Hide resolved
markshannon
added
the
|
If you want to schedule another build, you need to add the " |
bedevere-bot
removed
the
Just a couple of remaining nits.
I'm not sure what kind of review you're looking for... I don't see any technical issues with the implementation, so I'll hit "approve". :)
But I still have reservations about this change being much deeper (and probably more expensive) than what's really necessary right now. I still feel like I haven't really gotten an answer on why this change is more appropriate than something like #98795. So I don't necessarily approve of this approach, but I also won't block you from merging this if you still feel differently.
Misc/NEWS.d/next/Core and Builtins/2022-10-19-15-59-08.gh-issue-96421.e22y3r.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
…6319) * Adds EXIT_INTERPRETER instruction to exit PyEval_EvalDefault() * Simplifies RETURN_VALUE, YIELD_VALUE and RETURN_GENERATOR instructions as they no longer need to check for entry frames.
Simplifies
RETURN_VALUE,RETURN_GENERATORandYIELD_VALUEas they no longer need to check if the current frame is the entry frame.Should allow specialization of
FOR_ITERandSENDfor generators and coroutines.Needs docs and news.
Performance impact is about zero