Skip to content

Commit 440713c

Browse files
cielavenircielavenir
committed
Revert "pythonGH-108035: Remove the _PyCFrame struct as it is no longer needed for performance. (pythonGH-108036)"
This reverts commit 006e44f.
1 parent 006e44f commit 440713c

21 files changed

Lines changed: 103 additions & 66 deletions

‎Include/cpython/pystate.h‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,24 @@ typedef int (*Py_tracefunc)(PyObject *, PyFrameObject *, int, PyObject *);
2929
#define PyTrace_C_RETURN 6
3030
#define PyTrace_OPCODE 7
3131

32+
// Internal structure: you should not use it directly, but use public functions
33+
// like PyThreadState_EnterTracing() and PyThreadState_LeaveTracing().
34+
typedef struct _PyCFrame {
35+
/* This struct will be threaded through the C stack
36+
* allowing fast access to per-thread state that needs
37+
* to be accessed quickly by the interpreter, but can
38+
* be modified outside of the interpreter.
39+
*
40+
* WARNING: This makes data on the C stack accessible from
41+
* heap objects. Care must be taken to maintain stack
42+
* discipline and make sure that instances of this struct cannot
43+
* accessed outside of their lifetime.
44+
*/
45+
/* Pointer to the currently executing frame (it can be NULL) */
46+
struct _PyInterpreterFrame *current_frame;
47+
struct _PyCFrame *previous;
48+
} _PyCFrame;
49+
3250
typedef struct _err_stackitem {
3351
/* This struct represents a single execution context where we might
3452
* be currently handling an exception. It is a per-coroutine state
@@ -105,8 +123,9 @@ struct _ts {
105123
int tracing;
106124
int what_event; /* The event currently being monitored, if any. */
107125

108-
/* Pointer to currently executing frame. */
109-
struct _PyInterpreterFrame *current_frame;
126+
/* Pointer to current _PyCFrame in the C stack frame of the currently,
127+
* or most recently, executing _PyEval_EvalFrameDefault. */
128+
_PyCFrame *cframe;
110129

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

214+
/* The bottom-most frame on the stack. */
215+
_PyCFrame root_cframe;
195216
};
196217

197218
/* WASI has limited call stack. Python's recursion limit depends on code

‎Include/internal/pycore_frame.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ _PyFrame_GetFirstComplete(_PyInterpreterFrame *frame)
196196
static inline _PyInterpreterFrame *
197197
_PyThreadState_GetFrame(PyThreadState *tstate)
198198
{
199-
return _PyFrame_GetFirstComplete(tstate->current_frame);
199+
return _PyFrame_GetFirstComplete(tstate->cframe->current_frame);
200200
}
201201

202202
/* For use by _PyFrame_GetFrameObject

‎Include/internal/pycore_runtime.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ typedef struct _Py_DebugOffsets {
8080
off_t prev;
8181
off_t next;
8282
off_t interp;
83-
off_t current_frame;
83+
off_t cframe;
8484
off_t thread_id;
8585
off_t native_thread_id;
8686
} thread_state;

‎Include/internal/pycore_runtime_init.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ extern PyTypeObject _PyExc_MemoryError;
4545
.prev = offsetof(PyThreadState, prev), \
4646
.next = offsetof(PyThreadState, next), \
4747
.interp = offsetof(PyThreadState, interp), \
48-
.current_frame = offsetof(PyThreadState, current_frame), \
48+
.cframe = offsetof(PyThreadState, cframe), \
4949
.thread_id = offsetof(PyThreadState, thread_id), \
5050
.native_thread_id = offsetof(PyThreadState, native_thread_id), \
5151
}, \
@@ -56,6 +56,10 @@ extern PyTypeObject _PyExc_MemoryError;
5656
.localsplus = offsetof(_PyInterpreterFrame, localsplus), \
5757
.owner = offsetof(_PyInterpreterFrame, owner), \
5858
}, \
59+
.cframe = { \
60+
.current_frame = offsetof(_PyCFrame, current_frame), \
61+
.previous = offsetof(_PyCFrame, previous), \
62+
}, \
5963
.code_object = { \
6064
.filename = offsetof(PyCodeObject, co_filename), \
6165
.name = offsetof(PyCodeObject, co_name), \

‎Misc/NEWS.d/next/Core and Builtins/2023-08-11-16-18-19.gh-issue-108035.e2msOD.rst‎

Lines changed: 0 additions & 4 deletions
This file was deleted.

‎Modules/_xxsubinterpretersmodule.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ _is_running(PyInterpreterState *interp)
369369
}
370370

371371
assert(!PyErr_Occurred());
372-
struct _PyInterpreterFrame *frame = tstate->current_frame;
372+
struct _PyInterpreterFrame *frame = tstate->cframe->current_frame;
373373
if (frame == NULL) {
374374
return 0;
375375
}

‎Objects/genobject.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -476,17 +476,17 @@ _gen_throw(PyGenObject *gen, int close_on_genexit,
476476
will be reported correctly to the user. */
477477
/* XXX We should probably be updating the current frame
478478
somewhere in ceval.c. */
479-
_PyInterpreterFrame *prev = tstate->current_frame;
479+
_PyInterpreterFrame *prev = tstate->cframe->current_frame;
480480
frame->previous = prev;
481-
tstate->current_frame = frame;
481+
tstate->cframe->current_frame = frame;
482482
/* Close the generator that we are currently iterating with
483483
'yield from' or awaiting on with 'await'. */
484484
PyFrameState state = gen->gi_frame_state;
485485
gen->gi_frame_state = FRAME_EXECUTING;
486486
ret = _gen_throw((PyGenObject *)yf, close_on_genexit,
487487
typ, val, tb);
488488
gen->gi_frame_state = state;
489-
tstate->current_frame = prev;
489+
tstate->cframe->current_frame = prev;
490490
frame->previous = NULL;
491491
} else {
492492
/* `yf` is an iterator or a coroutine-like object. */
@@ -938,7 +938,7 @@ _Py_MakeCoro(PyFunctionObject *func)
938938
if (origin_depth == 0) {
939939
((PyCoroObject *)coro)->cr_origin_or_finalizer = NULL;
940940
} else {
941-
_PyInterpreterFrame *frame = tstate->current_frame;
941+
_PyInterpreterFrame *frame = tstate->cframe->current_frame;
942942
assert(frame);
943943
assert(_PyFrame_IsIncomplete(frame));
944944
frame = _PyFrame_GetFirstComplete(frame->previous);

‎Objects/typevarobject.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ make_union(PyObject *self, PyObject *other)
107107
static PyObject *
108108
caller(void)
109109
{
110-
_PyInterpreterFrame *f = _PyThreadState_GET()->current_frame;
110+
_PyInterpreterFrame *f = _PyThreadState_GET()->cframe->current_frame;
111111
if (f == NULL) {
112112
Py_RETURN_NONE;
113113
}

‎Python/bytecodes.c‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ dummy_func(
7272
_PyInterpreterFrame *frame,
7373
unsigned char opcode,
7474
unsigned int oparg,
75+
_PyCFrame cframe,
7576
_Py_CODEUNIT *next_instr,
7677
PyObject **stack_pointer,
7778
PyObject *kwnames,
@@ -133,7 +134,8 @@ dummy_func(
133134
}
134135

135136
inst(RESUME, (--)) {
136-
assert(frame == tstate->current_frame);
137+
assert(tstate->cframe == &cframe);
138+
assert(frame == cframe.current_frame);
137139
/* Possibly combine this with eval breaker */
138140
if (_PyFrame_GetCode(frame)->_co_instrumentation_version != tstate->interp->monitoring_version) {
139141
int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp);
@@ -750,8 +752,9 @@ dummy_func(
750752
inst(INTERPRETER_EXIT, (retval --)) {
751753
assert(frame == &entry_frame);
752754
assert(_PyFrame_IsIncomplete(frame));
753-
/* Restore previous frame and return. */
754-
tstate->current_frame = frame->previous;
755+
/* Restore previous cframe and return. */
756+
tstate->cframe = cframe.previous;
757+
assert(tstate->cframe->current_frame == frame->previous);
755758
assert(!_PyErr_Occurred(tstate));
756759
tstate->c_recursion_remaining += PY_EVAL_C_STACK_UNITS;
757760
return retval;
@@ -765,7 +768,7 @@ dummy_func(
765768
assert(frame != &entry_frame);
766769
// GH-99729: We need to unlink the frame *before* clearing it:
767770
_PyInterpreterFrame *dying = frame;
768-
frame = tstate->current_frame = dying->previous;
771+
frame = cframe.current_frame = dying->previous;
769772
_PyEvalFrameClearAndPop(tstate, dying);
770773
frame->prev_instr += frame->return_offset;
771774
_PyFrame_StackPush(frame, retval);
@@ -784,7 +787,7 @@ dummy_func(
784787
assert(frame != &entry_frame);
785788
// GH-99729: We need to unlink the frame *before* clearing it:
786789
_PyInterpreterFrame *dying = frame;
787-
frame = tstate->current_frame = dying->previous;
790+
frame = cframe.current_frame = dying->previous;
788791
_PyEvalFrameClearAndPop(tstate, dying);
789792
frame->prev_instr += frame->return_offset;
790793
_PyFrame_StackPush(frame, retval);
@@ -800,7 +803,7 @@ dummy_func(
800803
assert(frame != &entry_frame);
801804
// GH-99729: We need to unlink the frame *before* clearing it:
802805
_PyInterpreterFrame *dying = frame;
803-
frame = tstate->current_frame = dying->previous;
806+
frame = cframe.current_frame = dying->previous;
804807
_PyEvalFrameClearAndPop(tstate, dying);
805808
frame->prev_instr += frame->return_offset;
806809
_PyFrame_StackPush(frame, retval);
@@ -820,7 +823,7 @@ dummy_func(
820823
assert(frame != &entry_frame);
821824
// GH-99729: We need to unlink the frame *before* clearing it:
822825
_PyInterpreterFrame *dying = frame;
823-
frame = tstate->current_frame = dying->previous;
826+
frame = cframe.current_frame = dying->previous;
824827
_PyEvalFrameClearAndPop(tstate, dying);
825828
frame->prev_instr += frame->return_offset;
826829
_PyFrame_StackPush(frame, retval);
@@ -1016,7 +1019,7 @@ dummy_func(
10161019
gen->gi_exc_state.previous_item = NULL;
10171020
_Py_LeaveRecursiveCallPy(tstate);
10181021
_PyInterpreterFrame *gen_frame = frame;
1019-
frame = tstate->current_frame = frame->previous;
1022+
frame = cframe.current_frame = frame->previous;
10201023
gen_frame->previous = NULL;
10211024
_PyFrame_StackPush(frame, retval);
10221025
goto resume_frame;
@@ -1035,7 +1038,7 @@ dummy_func(
10351038
gen->gi_exc_state.previous_item = NULL;
10361039
_Py_LeaveRecursiveCallPy(tstate);
10371040
_PyInterpreterFrame *gen_frame = frame;
1038-
frame = tstate->current_frame = frame->previous;
1041+
frame = cframe.current_frame = frame->previous;
10391042
gen_frame->previous = NULL;
10401043
_PyFrame_StackPush(frame, retval);
10411044
goto resume_frame;
@@ -2204,10 +2207,10 @@ dummy_func(
22042207
OBJECT_STAT_INC(optimization_attempts);
22052208
frame = _PyOptimizer_BackEdge(frame, here, next_instr, stack_pointer);
22062209
if (frame == NULL) {
2207-
frame = tstate->current_frame;
2210+
frame = cframe.current_frame;
22082211
goto resume_with_error;
22092212
}
2210-
assert(frame == tstate->current_frame);
2213+
assert(frame == cframe.current_frame);
22112214
here[1].cache &= ((1 << OPTIMIZER_BITS_IN_COUNTER) -1);
22122215
goto resume_frame;
22132216
}
@@ -2235,7 +2238,7 @@ dummy_func(
22352238
Py_INCREF(executor);
22362239
frame = executor->execute(executor, frame, stack_pointer);
22372240
if (frame == NULL) {
2238-
frame = tstate->current_frame;
2241+
frame = cframe.current_frame;
22392242
goto resume_with_error;
22402243
}
22412244
goto resume_frame;
@@ -2990,11 +2993,12 @@ dummy_func(
29902993
_PyFrame_SetStackPointer(frame, stack_pointer);
29912994
new_frame->previous = frame;
29922995
CALL_STAT_INC(inlined_py_calls);
2993-
frame = tstate->current_frame = new_frame;
29942996
#if TIER_ONE
2997+
frame = cframe.current_frame = new_frame;
29952998
goto start_frame;
29962999
#endif
29973000
#if TIER_TWO
3001+
frame = tstate->cframe->current_frame = new_frame;
29983002
ERROR_IF(_Py_EnterRecursivePy(tstate), exit_unwind);
29993003
stack_pointer = _PyFrame_GetStackPointer(frame);
30003004
ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive;
@@ -3131,7 +3135,7 @@ dummy_func(
31313135
/* Link frames */
31323136
init_frame->previous = shim;
31333137
shim->previous = frame;
3134-
frame = tstate->current_frame = init_frame;
3138+
frame = cframe.current_frame = init_frame;
31353139
CALL_STAT_INC(inlined_py_calls);
31363140
/* Account for pushing the extra frame.
31373141
* We don't check recursion depth here,
@@ -3594,7 +3598,7 @@ dummy_func(
35943598
assert(frame != &entry_frame);
35953599
_PyInterpreterFrame *prev = frame->previous;
35963600
_PyThreadState_PopFrame(tstate, frame);
3597-
frame = tstate->current_frame = prev;
3601+
frame = cframe.current_frame = prev;
35983602
_PyFrame_StackPush(frame, (PyObject *)gen);
35993603
goto resume_frame;
36003604
}

‎Python/ceval.c‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -656,10 +656,17 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
656656
int lltrace = 0;
657657
#endif
658658

659+
_PyCFrame cframe;
659660
_PyInterpreterFrame entry_frame;
660661
PyObject *kwnames = NULL; // Borrowed reference. Reset by CALL instructions.
661662

662-
663+
/* WARNING: Because the _PyCFrame lives on the C stack,
664+
* but can be accessed from a heap allocated object (tstate)
665+
* strict stack discipline must be maintained.
666+
*/
667+
_PyCFrame *prev_cframe = tstate->cframe;
668+
cframe.previous = prev_cframe;
669+
tstate->cframe = &cframe;
663670

664671
#ifdef Py_DEBUG
665672
/* Set these to invalid but identifiable values for debugging. */
@@ -675,9 +682,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
675682
entry_frame.owner = FRAME_OWNED_BY_CSTACK;
676683
entry_frame.return_offset = 0;
677684
/* Push frame */
678-
entry_frame.previous = tstate->current_frame;
685+
entry_frame.previous = prev_cframe->current_frame;
679686
frame->previous = &entry_frame;
680-
tstate->current_frame = frame;
687+
cframe.current_frame = frame;
681688

682689
tstate->c_recursion_remaining -= (PY_EVAL_C_STACK_UNITS - 1);
683690
if (_Py_EnterRecursiveCallTstate(tstate, "")) {
@@ -917,12 +924,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
917924
assert(frame != &entry_frame);
918925
// GH-99729: We need to unlink the frame *before* clearing it:
919926
_PyInterpreterFrame *dying = frame;
920-
frame = tstate->current_frame = dying->previous;
927+
frame = cframe.current_frame = dying->previous;
921928
_PyEvalFrameClearAndPop(tstate, dying);
922929
frame->return_offset = 0;
923930
if (frame == &entry_frame) {
924-
/* Restore previous frame and exit */
925-
tstate->current_frame = frame->previous;
931+
/* Restore previous cframe and exit */
932+
tstate->cframe = cframe.previous;
933+
assert(tstate->cframe->current_frame == frame->previous);
926934
tstate->c_recursion_remaining += PY_EVAL_C_STACK_UNITS;
927935
return NULL;
928936
}
@@ -2289,7 +2297,7 @@ int
22892297
PyEval_MergeCompilerFlags(PyCompilerFlags *cf)
22902298
{
22912299
PyThreadState *tstate = _PyThreadState_GET();
2292-
_PyInterpreterFrame *current_frame = tstate->current_frame;
2300+
_PyInterpreterFrame *current_frame = tstate->cframe->current_frame;
22932301
int result = cf->cf_flags != 0;
22942302

22952303
if (current_frame != NULL) {

0 commit comments

Comments
 (0)