Uh oh!
There was an error while loading. Please reload this page.
GH-127705: Use _PyStackRefs in the default build. - #127875
Conversation
…xes to immortal objects.
…e-127705.Qad2hx.rst Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
ericsnowcurrently
left a comment
There was a problem hiding this comment.
This mostly seems okay. FYI, I didn't do a deep dive into the code to verifying correctness, like I normally do. I did read through all the files except for Include/internal/pycore_stackref.h. The changes make sense and I can see how they simplify things.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
mpage
commented
Mar 6, 2025
@markshannon - Have you measured the effect of the mortality optimizations independently from the changes to use macros? The tag bits are a precious commodity and the mortality optimizations require two bits. |
@mpage. I changed the PR to use a single bit, using
|
| inst(RETURN_VALUE, (retval -- res)) { | ||
| assert(frame->owner != FRAME_OWNED_BY_INTERPRETER); | ||
| _PyStackRef temp = retval; | ||
| assert(PyStackRef_IsHeapSafe(temp)); |
There was a problem hiding this comment.
Why the assert here? This blocks stackrefs from passing through to the host frame, which blocks a lot of unboxed and deferred refcounting optimizations.
There was a problem hiding this comment.
It blocks a lot of unsafe operations. Passing references towards the caller is unsafe as it is possible that the only immediate reference count (stored in ob_refcnt) for an object is stored in the callee, and will be deleted during the return.
bedevere-bot
commented
Mar 10, 2025
|
bedevere-bot
commented
Mar 10, 2025
|
bedevere-bot
commented
Mar 10, 2025
|
bedevere-bot
commented
Mar 10, 2025
|
bedevere-bot
commented
Mar 10, 2025
|
bedevere-bot
commented
Mar 10, 2025
|
bedevere-bot
commented
Mar 10, 2025
|
bedevere-bot
commented
Mar 10, 2025
|
bedevere-bot
commented
Mar 11, 2025
|
We already have
_PyStackRefs in the default build, but they are justPyObject *pointers cast to a pointer-sized int.This PR adds tags according to the tagging scheme in #127705.