GH-119866: Don't clear frame->stackpointer on release builds
#131750
Conversation
brandtbucher
added
performance
| @@ -171,7 +171,9 @@ _PyFrame_GetStackPointer(_PyInterpreterFrame *frame) | |||
| { | |||
| assert(frame->stackpointer != NULL); | |||
| _PyStackRef *sp = frame->stackpointer; | |||
| #ifndef NDEBUG | |||
There was a problem hiding this comment.
Maybe then update the comment
Having stackpointer == NULL ensures that invalid
values are not visible to the cycle GC.Seems that this was done for the cycle GC - if this is no longer needed, just remove the comment?
Out of curiousity: is it safe in release builds to let the cycle GC see invalid values?
There was a problem hiding this comment.
We want to have "deferred" references on the stack, which means that the GC must be able to see all stack values, and only valid stack values.
Setting the stack pointer to NULL is only useful for our asserts, which are sprinkled throughout to make sure that we're spilling and restoring in the correct places. Basically, the GC should never see an invalid stack pointer. On debug builds, we make this easier to detect by setting invalid stack pointers to NULL and checking for this.
Either way, I'll update the comment.
This is only useful for asserts to make sure that we're spilling and restoring the stack pointer in all the right places. Removing these frequent stores makes the interpreter 0.7% faster and the JIT 0.4% faster.