Skip to content

Commit

Permalink
pythonGH-108035: Remove the _PyCFrame struct as it is no longer nee…
Browse files Browse the repository at this point in the history
…ded for performance. (pythonGH-108036)
  • Loading branch information
markshannon authored Aug 17, 2023
1 parent 33e6e3f commit 006e44f
Show file tree
Hide file tree
Showing 21 changed files with 66 additions and 103 deletions.
25 changes: 2 additions & 23 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,6 @@ typedef int (*Py_tracefunc)(PyObject *, PyFrameObject *, int, PyObject *);
#define PyTrace_C_RETURN 6
#define PyTrace_OPCODE 7

// Internal structure: you should not use it directly, but use public functions
// like PyThreadState_EnterTracing() and PyThreadState_LeaveTracing().
typedef struct _PyCFrame {
/* This struct will be threaded through the C stack
* allowing fast access to per-thread state that needs
* to be accessed quickly by the interpreter, but can
* be modified outside of the interpreter.
*
* WARNING: This makes data on the C stack accessible from
* heap objects. Care must be taken to maintain stack
* discipline and make sure that instances of this struct cannot
* accessed outside of their lifetime.
*/
/* Pointer to the currently executing frame (it can be NULL) */
struct _PyInterpreterFrame *current_frame;
struct _PyCFrame *previous;
} _PyCFrame;

typedef struct _err_stackitem {
/* This struct represents a single execution context where we might
* be currently handling an exception. It is a per-coroutine state
Expand Down Expand Up @@ -123,9 +105,8 @@ struct _ts {
int tracing;
int what_event; /* The event currently being monitored, if any. */

/* Pointer to current _PyCFrame in the C stack frame of the currently,
* or most recently, executing _PyEval_EvalFrameDefault. */
_PyCFrame *cframe;
/* Pointer to currently executing frame. */
struct _PyInterpreterFrame *current_frame;

Py_tracefunc c_profilefunc;
Py_tracefunc c_tracefunc;
Expand Down Expand Up @@ -211,8 +192,6 @@ struct _ts {
/* The thread's exception stack entry. (Always the last entry.) */
_PyErr_StackItem exc_state;

/* The bottom-most frame on the stack. */
_PyCFrame root_cframe;
};

/* WASI has limited call stack. Python's recursion limit depends on code
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ _PyFrame_GetFirstComplete(_PyInterpreterFrame *frame)
static inline _PyInterpreterFrame *
_PyThreadState_GetFrame(PyThreadState *tstate)
{
return _PyFrame_GetFirstComplete(tstate->cframe->current_frame);
return _PyFrame_GetFirstComplete(tstate->current_frame);
}

/* For use by _PyFrame_GetFrameObject
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ typedef struct _Py_DebugOffsets {
off_t prev;
off_t next;
off_t interp;
off_t cframe;
off_t current_frame;
off_t thread_id;
off_t native_thread_id;
} thread_state;
Expand Down
6 changes: 1 addition & 5 deletions Include/internal/pycore_runtime_init.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ extern PyTypeObject _PyExc_MemoryError;
.prev = offsetof(PyThreadState, prev), \
.next = offsetof(PyThreadState, next), \
.interp = offsetof(PyThreadState, interp), \
.cframe = offsetof(PyThreadState, cframe), \
.current_frame = offsetof(PyThreadState, current_frame), \
.thread_id = offsetof(PyThreadState, thread_id), \
.native_thread_id = offsetof(PyThreadState, native_thread_id), \
}, \
Expand All @@ -56,10 +56,6 @@ extern PyTypeObject _PyExc_MemoryError;
.localsplus = offsetof(_PyInterpreterFrame, localsplus), \
.owner = offsetof(_PyInterpreterFrame, owner), \
}, \
.cframe = { \
.current_frame = offsetof(_PyCFrame, current_frame), \
.previous = offsetof(_PyCFrame, previous), \
}, \
.code_object = { \
.filename = offsetof(PyCodeObject, co_filename), \
.name = offsetof(PyCodeObject, co_name), \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Remove the ``_PyCFrame`` struct, moving the pointer to the current intepreter frame
back to the threadstate, as it was for 3.10 and earlier. The ``_PyCFrame``
existed as a performance optimization for tracing. Since PEP 669 has been
implemented, this optimization no longer applies.
2 changes: 1 addition & 1 deletion Modules/_xxsubinterpretersmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ _is_running(PyInterpreterState *interp)
}

assert(!PyErr_Occurred());
struct _PyInterpreterFrame *frame = tstate->cframe->current_frame;
struct _PyInterpreterFrame *frame = tstate->current_frame;
if (frame == NULL) {
return 0;
}
Expand Down
8 changes: 4 additions & 4 deletions Objects/genobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -476,17 +476,17 @@ _gen_throw(PyGenObject *gen, int close_on_genexit,
will be reported correctly to the user. */
/* XXX We should probably be updating the current frame
somewhere in ceval.c. */
_PyInterpreterFrame *prev = tstate->cframe->current_frame;
_PyInterpreterFrame *prev = tstate->current_frame;
frame->previous = prev;
tstate->cframe->current_frame = frame;
tstate->current_frame = frame;
/* Close the generator that we are currently iterating with
'yield from' or awaiting on with 'await'. */
PyFrameState state = gen->gi_frame_state;
gen->gi_frame_state = FRAME_EXECUTING;
ret = _gen_throw((PyGenObject *)yf, close_on_genexit,
typ, val, tb);
gen->gi_frame_state = state;
tstate->cframe->current_frame = prev;
tstate->current_frame = prev;
frame->previous = NULL;
} else {
/* `yf` is an iterator or a coroutine-like object. */
Expand Down Expand Up @@ -938,7 +938,7 @@ _Py_MakeCoro(PyFunctionObject *func)
if (origin_depth == 0) {
((PyCoroObject *)coro)->cr_origin_or_finalizer = NULL;
} else {
_PyInterpreterFrame *frame = tstate->cframe->current_frame;
_PyInterpreterFrame *frame = tstate->current_frame;
assert(frame);
assert(_PyFrame_IsIncomplete(frame));
frame = _PyFrame_GetFirstComplete(frame->previous);
Expand Down
2 changes: 1 addition & 1 deletion Objects/typevarobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ make_union(PyObject *self, PyObject *other)
static PyObject *
caller(void)
{
_PyInterpreterFrame *f = _PyThreadState_GET()->cframe->current_frame;
_PyInterpreterFrame *f = _PyThreadState_GET()->current_frame;
if (f == NULL) {
Py_RETURN_NONE;
}
Expand Down
34 changes: 15 additions & 19 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ dummy_func(
_PyInterpreterFrame *frame,
unsigned char opcode,
unsigned int oparg,
_PyCFrame cframe,
_Py_CODEUNIT *next_instr,
PyObject **stack_pointer,
PyObject *kwnames,
Expand Down Expand Up @@ -134,8 +133,7 @@ dummy_func(
}

inst(RESUME, (--)) {
assert(tstate->cframe == &cframe);
assert(frame == cframe.current_frame);
assert(frame == tstate->current_frame);
/* Possibly combine this with eval breaker */
if (_PyFrame_GetCode(frame)->_co_instrumentation_version != tstate->interp->monitoring_version) {
int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp);
Expand Down Expand Up @@ -752,9 +750,8 @@ dummy_func(
inst(INTERPRETER_EXIT, (retval --)) {
assert(frame == &entry_frame);
assert(_PyFrame_IsIncomplete(frame));
/* Restore previous cframe and return. */
tstate->cframe = cframe.previous;
assert(tstate->cframe->current_frame == frame->previous);
/* Restore previous frame and return. */
tstate->current_frame = frame->previous;
assert(!_PyErr_Occurred(tstate));
tstate->c_recursion_remaining += PY_EVAL_C_STACK_UNITS;
return retval;
Expand All @@ -768,7 +765,7 @@ dummy_func(
assert(frame != &entry_frame);
// GH-99729: We need to unlink the frame *before* clearing it:
_PyInterpreterFrame *dying = frame;
frame = cframe.current_frame = dying->previous;
frame = tstate->current_frame = dying->previous;
_PyEvalFrameClearAndPop(tstate, dying);
frame->prev_instr += frame->return_offset;
_PyFrame_StackPush(frame, retval);
Expand All @@ -787,7 +784,7 @@ dummy_func(
assert(frame != &entry_frame);
// GH-99729: We need to unlink the frame *before* clearing it:
_PyInterpreterFrame *dying = frame;
frame = cframe.current_frame = dying->previous;
frame = tstate->current_frame = dying->previous;
_PyEvalFrameClearAndPop(tstate, dying);
frame->prev_instr += frame->return_offset;
_PyFrame_StackPush(frame, retval);
Expand All @@ -803,7 +800,7 @@ dummy_func(
assert(frame != &entry_frame);
// GH-99729: We need to unlink the frame *before* clearing it:
_PyInterpreterFrame *dying = frame;
frame = cframe.current_frame = dying->previous;
frame = tstate->current_frame = dying->previous;
_PyEvalFrameClearAndPop(tstate, dying);
frame->prev_instr += frame->return_offset;
_PyFrame_StackPush(frame, retval);
Expand All @@ -823,7 +820,7 @@ dummy_func(
assert(frame != &entry_frame);
// GH-99729: We need to unlink the frame *before* clearing it:
_PyInterpreterFrame *dying = frame;
frame = cframe.current_frame = dying->previous;
frame = tstate->current_frame = dying->previous;
_PyEvalFrameClearAndPop(tstate, dying);
frame->prev_instr += frame->return_offset;
_PyFrame_StackPush(frame, retval);
Expand Down Expand Up @@ -1019,7 +1016,7 @@ dummy_func(
gen->gi_exc_state.previous_item = NULL;
_Py_LeaveRecursiveCallPy(tstate);
_PyInterpreterFrame *gen_frame = frame;
frame = cframe.current_frame = frame->previous;
frame = tstate->current_frame = frame->previous;
gen_frame->previous = NULL;
_PyFrame_StackPush(frame, retval);
goto resume_frame;
Expand All @@ -1038,7 +1035,7 @@ dummy_func(
gen->gi_exc_state.previous_item = NULL;
_Py_LeaveRecursiveCallPy(tstate);
_PyInterpreterFrame *gen_frame = frame;
frame = cframe.current_frame = frame->previous;
frame = tstate->current_frame = frame->previous;
gen_frame->previous = NULL;
_PyFrame_StackPush(frame, retval);
goto resume_frame;
Expand Down Expand Up @@ -2207,10 +2204,10 @@ dummy_func(
OBJECT_STAT_INC(optimization_attempts);
frame = _PyOptimizer_BackEdge(frame, here, next_instr, stack_pointer);
if (frame == NULL) {
frame = cframe.current_frame;
frame = tstate->current_frame;
goto resume_with_error;
}
assert(frame == cframe.current_frame);
assert(frame == tstate->current_frame);
here[1].cache &= ((1 << OPTIMIZER_BITS_IN_COUNTER) -1);
goto resume_frame;
}
Expand Down Expand Up @@ -2238,7 +2235,7 @@ dummy_func(
Py_INCREF(executor);
frame = executor->execute(executor, frame, stack_pointer);
if (frame == NULL) {
frame = cframe.current_frame;
frame = tstate->current_frame;
goto resume_with_error;
}
goto resume_frame;
Expand Down Expand Up @@ -2993,12 +2990,11 @@ dummy_func(
_PyFrame_SetStackPointer(frame, stack_pointer);
new_frame->previous = frame;
CALL_STAT_INC(inlined_py_calls);
frame = tstate->current_frame = new_frame;
#if TIER_ONE
frame = cframe.current_frame = new_frame;
goto start_frame;
#endif
#if TIER_TWO
frame = tstate->cframe->current_frame = new_frame;
ERROR_IF(_Py_EnterRecursivePy(tstate), exit_unwind);
stack_pointer = _PyFrame_GetStackPointer(frame);
ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive;
Expand Down Expand Up @@ -3135,7 +3131,7 @@ dummy_func(
/* Link frames */
init_frame->previous = shim;
shim->previous = frame;
frame = cframe.current_frame = init_frame;
frame = tstate->current_frame = init_frame;
CALL_STAT_INC(inlined_py_calls);
/* Account for pushing the extra frame.
* We don't check recursion depth here,
Expand Down Expand Up @@ -3598,7 +3594,7 @@ dummy_func(
assert(frame != &entry_frame);
_PyInterpreterFrame *prev = frame->previous;
_PyThreadState_PopFrame(tstate, frame);
frame = cframe.current_frame = prev;
frame = tstate->current_frame = prev;
_PyFrame_StackPush(frame, (PyObject *)gen);
goto resume_frame;
}
Expand Down
22 changes: 7 additions & 15 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -656,17 +656,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
int lltrace = 0;
#endif

_PyCFrame cframe;
_PyInterpreterFrame entry_frame;
PyObject *kwnames = NULL; // Borrowed reference. Reset by CALL instructions.

/* WARNING: Because the _PyCFrame lives on the C stack,
* but can be accessed from a heap allocated object (tstate)
* strict stack discipline must be maintained.
*/
_PyCFrame *prev_cframe = tstate->cframe;
cframe.previous = prev_cframe;
tstate->cframe = &cframe;


#ifdef Py_DEBUG
/* Set these to invalid but identifiable values for debugging. */
Expand All @@ -682,9 +675,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
entry_frame.owner = FRAME_OWNED_BY_CSTACK;
entry_frame.return_offset = 0;
/* Push frame */
entry_frame.previous = prev_cframe->current_frame;
entry_frame.previous = tstate->current_frame;
frame->previous = &entry_frame;
cframe.current_frame = frame;
tstate->current_frame = frame;

tstate->c_recursion_remaining -= (PY_EVAL_C_STACK_UNITS - 1);
if (_Py_EnterRecursiveCallTstate(tstate, "")) {
Expand Down Expand Up @@ -924,13 +917,12 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
assert(frame != &entry_frame);
// GH-99729: We need to unlink the frame *before* clearing it:
_PyInterpreterFrame *dying = frame;
frame = cframe.current_frame = dying->previous;
frame = tstate->current_frame = dying->previous;
_PyEvalFrameClearAndPop(tstate, dying);
frame->return_offset = 0;
if (frame == &entry_frame) {
/* Restore previous cframe and exit */
tstate->cframe = cframe.previous;
assert(tstate->cframe->current_frame == frame->previous);
/* Restore previous frame and exit */
tstate->current_frame = frame->previous;
tstate->c_recursion_remaining += PY_EVAL_C_STACK_UNITS;
return NULL;
}
Expand Down Expand Up @@ -2297,7 +2289,7 @@ int
PyEval_MergeCompilerFlags(PyCompilerFlags *cf)
{
PyThreadState *tstate = _PyThreadState_GET();
_PyInterpreterFrame *current_frame = tstate->cframe->current_frame;
_PyInterpreterFrame *current_frame = tstate->current_frame;
int result = cf->cf_flags != 0;

if (current_frame != NULL) {
Expand Down
2 changes: 1 addition & 1 deletion Python/ceval_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
_PyFrame_SetStackPointer(frame, stack_pointer); \
frame->prev_instr = next_instr - 1; \
(NEW_FRAME)->previous = frame; \
frame = cframe.current_frame = (NEW_FRAME); \
frame = tstate->current_frame = (NEW_FRAME); \
CALL_STAT_INC(inlined_py_calls); \
goto start_frame; \
} while (0)
Expand Down
2 changes: 1 addition & 1 deletion Python/executor.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
STACK_SHRINK(1);
error:
// On ERROR_IF we return NULL as the frame.
// The caller recovers the frame from cframe.current_frame.
// The caller recovers the frame from tstate->current_frame.
DPRINTF(2, "Error: [Opcode %d, operand %" PRIu64 "]\n", opcode, operand);
_PyFrame_SetStackPointer(frame, stack_pointer);
Py_DECREF(self);
Expand Down
3 changes: 1 addition & 2 deletions Python/executor_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Python/frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ _PyFrame_ClearExceptCode(_PyInterpreterFrame *frame)
_PyFrame_GetGenerator(frame)->gi_frame_state == FRAME_CLEARED);
// GH-99729: Clearing this frame can expose the stack (via finalizers). It's
// crucial that this frame has been unlinked, and is no longer visible:
assert(_PyThreadState_GET()->cframe->current_frame != frame);
assert(_PyThreadState_GET()->current_frame != frame);
if (frame->frame_obj) {
PyFrameObject *f = frame->frame_obj;
frame->frame_obj = NULL;
Expand Down
Loading

0 comments on commit 006e44f

Please sign in to comment.