Skip to content

GH-96421: Introduce cleanup contexts for inlined frames #98795

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

GH-96421: Introduce cleanup contexts for inlined frames #98795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24bbfa1
First pass at handling various cleanup contexts
brandtbucher Oct 17, 2022
661ccd8
Rip out the generator and class cleanup code
brandtbucher Oct 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions Include/internal/pycore_frame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ enum _frameowner {
FRAME_OWNED_BY_FRAME_OBJECT = 2
};

enum _frame_cleanup {
FRAME_CLEANUP_INVALID = 0,
FRAME_CLEANUP_ENTRY = 1,
FRAME_CLEANUP_INLINED_FUNCTION = 2,
};

typedef struct _PyInterpreterFrame {
/* "Specials" section */
PyObject *f_funcobj; /* Strong reference */
Expand All @@ -61,7 +67,7 @@ typedef struct _PyInterpreterFrame {
// over, or (in the case of a newly-created frame) a totally invalid value:
_Py_CODEUNIT *prev_instr;
int stacktop; /* Offset of TOS from localsplus */
bool is_entry; // Whether this is the "root" frame for the current _PyCFrame.
char cleanup;
char owner;
/* Locals and stack */
PyObject *localsplus[1];
Expand Down Expand Up @@ -109,7 +115,7 @@ _PyFrame_InitializeSpecials(
frame->stacktop = code->co_nlocalsplus;
frame->frame_obj = NULL;
frame->prev_instr = _PyCode_CODE(code) - 1;
frame->is_entry = false;
frame->cleanup = FRAME_CLEANUP_INVALID;
frame->owner = FRAME_OWNED_BY_THREAD;
}

Expand Down
2 changes: 1 addition & 1 deletion Objects/frameobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1342,7 +1342,7 @@ int _PyFrame_IsEntryFrame(PyFrameObject *frame)
{
assert(frame != NULL);
assert(!_PyFrame_IsIncomplete(frame->f_frame));
return frame->f_frame->is_entry;
return frame->f_frame->cleanup = FRAME_CLEANUP_ENTRY;
}


Expand Down
114 changes: 61 additions & 53 deletions Python/ceval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ GETITEM(PyObject *v, Py_ssize_t i) {
#define TRACE_FUNCTION_EXIT() \
if (cframe.use_tracing) { \
if (trace_function_exit(tstate, frame, retval)) { \
Py_DECREF(retval); \
Py_CLEAR(retval); \
goto exit_unwind; \
} \
}
Expand Down Expand Up @@ -1057,11 +1057,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
cframe.previous = prev_cframe;
tstate->cframe = &cframe;

frame->is_entry = true;
frame->cleanup = FRAME_CLEANUP_ENTRY;
/* Push frame */
frame->previous = prev_cframe->current_frame;
cframe.current_frame = frame;

PyObject *retval = NULL;

if (_Py_EnterRecursiveCallTstate(tstate, "")) {
tstate->c_recursion_remaining--;
tstate->py_recursion_remaining--;
Expand Down Expand Up @@ -1105,7 +1107,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
values are not visible to the cycle GC. \
We choose -1 rather than 0 to assist debugging. \
*/ \
frame->stacktop = -1;
frame->stacktop = -1; \
retval = NULL;


start_frame:
Expand Down Expand Up @@ -1702,6 +1705,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
new_frame->previous = frame;
frame = cframe.current_frame = new_frame;
CALL_STAT_INC(inlined_py_calls);
frame->cleanup = FRAME_CLEANUP_INLINED_FUNCTION;
goto start_frame;
}

Expand Down Expand Up @@ -1861,24 +1865,12 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
}

TARGET(RETURN_VALUE) {
PyObject *retval = POP();
retval = POP();
assert(EMPTY());
_PyFrame_SetStackPointer(frame, stack_pointer);
TRACE_FUNCTION_EXIT();
DTRACE_FUNCTION_EXIT();
_Py_LeaveRecursiveCallPy(tstate);
if (!frame->is_entry) {
frame = cframe.current_frame = pop_frame(tstate, frame);
_PyFrame_StackPush(frame, retval);
goto resume_frame;
}
_Py_LeaveRecursiveCallTstate(tstate);
/* Restore previous cframe and return. */
tstate->cframe = cframe.previous;
tstate->cframe->use_tracing = cframe.use_tracing;
assert(tstate->cframe->current_frame == frame->previous);
assert(!_PyErr_Occurred(tstate));
return retval;
goto cleanup;
}

TARGET(GET_AITER) {
Expand Down Expand Up @@ -2012,7 +2004,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
}

TARGET(SEND) {
assert(frame->is_entry);
assert(STACK_LEVEL() >= 2);
PyObject *v = POP();
PyObject *receiver = TOP();
Expand Down Expand Up @@ -2077,20 +2068,12 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
// The compiler treats any exception raised here as a failed close()
// or throw() call.
assert(oparg == STACK_LEVEL());
assert(frame->is_entry);
PyObject *retval = POP();
retval = POP();
_PyFrame_GetGenerator(frame)->gi_frame_state = FRAME_SUSPENDED;
_PyFrame_SetStackPointer(frame, stack_pointer);
TRACE_FUNCTION_EXIT();
DTRACE_FUNCTION_EXIT();
_Py_LeaveRecursiveCallPy(tstate);
_Py_LeaveRecursiveCallTstate(tstate);
/* Restore previous cframe and return. */
tstate->cframe = cframe.previous;
tstate->cframe->use_tracing = cframe.use_tracing;
assert(tstate->cframe->current_frame == frame->previous);
assert(!_PyErr_Occurred(tstate));
return retval;
goto cleanup;
}

TARGET(POP_EXCEPT) {
Expand Down Expand Up @@ -3149,6 +3132,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
new_frame->previous = frame;
frame = cframe.current_frame = new_frame;
CALL_STAT_INC(inlined_py_calls);
frame->cleanup = FRAME_CLEANUP_INLINED_FUNCTION;
goto start_frame;
}

Expand Down Expand Up @@ -3190,6 +3174,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
new_frame->previous = frame;
frame = cframe.current_frame = new_frame;
CALL_STAT_INC(inlined_py_calls);
frame->cleanup = FRAME_CLEANUP_INLINED_FUNCTION;
goto start_frame;
}

Expand Down Expand Up @@ -4203,6 +4188,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
new_frame->previous = frame;
cframe.current_frame = frame = new_frame;
CALL_STAT_INC(inlined_py_calls);
frame->cleanup = FRAME_CLEANUP_INLINED_FUNCTION;
goto start_frame;
}
/* Callable is not a normal Python function */
Expand Down Expand Up @@ -4287,6 +4273,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
frame->prev_instr = next_instr - 1;
new_frame->previous = frame;
frame = cframe.current_frame = new_frame;
frame->cleanup = FRAME_CLEANUP_INLINED_FUNCTION;
goto start_frame;
}

Expand Down Expand Up @@ -4327,6 +4314,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
frame->prev_instr = next_instr - 1;
new_frame->previous = frame;
frame = cframe.current_frame = new_frame;
frame->cleanup = FRAME_CLEANUP_INLINED_FUNCTION;
goto start_frame;
}

Expand Down Expand Up @@ -4843,26 +4831,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
assert(frame->frame_obj == NULL);
gen->gi_frame_state = FRAME_CREATED;
gen_frame->owner = FRAME_OWNED_BY_GENERATOR;
_Py_LeaveRecursiveCallPy(tstate);
if (!frame->is_entry) {
_PyInterpreterFrame *prev = frame->previous;
_PyThreadState_PopFrame(tstate, frame);
frame = cframe.current_frame = prev;
_PyFrame_StackPush(frame, (PyObject *)gen);
goto resume_frame;
}
_Py_LeaveRecursiveCallTstate(tstate);
/* Make sure that frame is in a valid state */
frame->stacktop = 0;
frame->f_locals = NULL;
Py_INCREF(frame->f_funcobj);
Py_INCREF(frame->f_code);
/* Restore previous cframe and return. */
tstate->cframe = cframe.previous;
tstate->cframe->use_tracing = cframe.use_tracing;
assert(tstate->cframe->current_frame == frame->previous);
assert(!_PyErr_Occurred(tstate));
return (PyObject *)gen;
retval = (PyObject *)gen;
goto cleanup;
}

TARGET(BUILD_SLICE) {
Expand Down Expand Up @@ -5221,22 +5196,55 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
}

exit_unwind:
assert(_PyErr_Occurred(tstate));
_Py_LeaveRecursiveCallPy(tstate);
if (frame->is_entry) {
/* Restore previous cframe and exit */
tstate->cframe = cframe.previous;
tstate->cframe->use_tracing = cframe.use_tracing;
assert(tstate->cframe->current_frame == frame->previous);
_Py_LeaveRecursiveCallTstate(tstate);
return NULL;
switch (frame->cleanup) {

case FRAME_CLEANUP_ENTRY:
tstate->cframe = cframe.previous;
tstate->cframe->use_tracing = cframe.use_tracing;
assert(tstate->cframe->current_frame == frame->previous);
_Py_LeaveRecursiveCallTstate(tstate);
_Py_LeaveRecursiveCallPy(tstate);
assert(_PyErr_Occurred(tstate));
assert(retval == NULL);
return NULL;

case FRAME_CLEANUP_INLINED_FUNCTION:
frame = cframe.current_frame = pop_frame(tstate, frame);
_Py_LeaveRecursiveCallPy(tstate);
assert(_PyErr_Occurred(tstate));
assert(retval == NULL);
goto resume_with_error;

}
frame = cframe.current_frame = pop_frame(tstate, frame);
Py_UNREACHABLE();

resume_with_error:
SET_LOCALS_FROM_FRAME();
goto error;

cleanup:
switch (frame->cleanup) {

case FRAME_CLEANUP_ENTRY:
tstate->cframe = cframe.previous;
tstate->cframe->use_tracing = cframe.use_tracing;
assert(tstate->cframe->current_frame == frame->previous);
_Py_LeaveRecursiveCallTstate(tstate);
_Py_LeaveRecursiveCallPy(tstate);
assert(!_PyErr_Occurred(tstate));
assert(retval);
return retval;

case FRAME_CLEANUP_INLINED_FUNCTION:
frame = cframe.current_frame = pop_frame(tstate, frame);
_PyFrame_StackPush(frame, retval);
_Py_LeaveRecursiveCallPy(tstate);
assert(!_PyErr_Occurred(tstate));
assert(retval);
goto resume_frame;

}
Py_UNREACHABLE();
}

static void
Expand Down
2 changes: 1 addition & 1 deletion Tools/gdb/libpython.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ def _f_lasti(self):
return int(prev_instr - first_instr)

def is_entry(self):
return self._f_special("is_entry", bool)
return self._f_special("cleanup", int) == 1

def previous(self):
return self._f_special("previous", PyFramePtr)
Expand Down