Skip to content
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.

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

gh-117139: Convert the evaluation stack to stack refs #118450

Open
wants to merge 136 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
136 commits
Select commit Hold shift + click to select a range
a6275f1
merge from upstream
Fidget-Spinner Apr 30, 2024
7e7627f
remove test tags
Fidget-Spinner Apr 30, 2024
00dde8e
add test for immortal
Fidget-Spinner Apr 30, 2024
eadc4fc
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Apr 30, 2024
64e2800
minor cleanups
Fidget-Spinner Apr 30, 2024
bf6b7e4
Create 2024-05-01-05-09-16.gh-issue-117139.t41w_D.rst
Fidget-Spinner Apr 30, 2024
865ff8d
remove bad assert
Fidget-Spinner Apr 30, 2024
b1777c4
rename tagged -> stackref
Fidget-Spinner May 1, 2024
50cfefa
trivial fixes first
Fidget-Spinner May 2, 2024
39e9057
fix anti-patterns (use deferred everywhere)
Fidget-Spinner May 2, 2024
1bb3c51
Delete 2024-03-26-20-31-34.gh-issue-117139.eELvoZ.rst
Fidget-Spinner May 2, 2024
18ef669
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner May 2, 2024
3fedc6f
replace and use macro
Fidget-Spinner May 2, 2024
1942d78
fix refcounting issues
Fidget-Spinner May 4, 2024
5a1c46f
Fix decref inputs problem
Fidget-Spinner May 4, 2024
dcc55b0
fix cases generator
Fidget-Spinner May 4, 2024
ec70b3b
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner May 4, 2024
542cfbb
fix problems from upstream
Fidget-Spinner May 4, 2024
3af1ef1
cleanup
Fidget-Spinner May 4, 2024
6bc29a1
formatting
Fidget-Spinner May 4, 2024
9052526
change used tags to 0b01
Fidget-Spinner May 5, 2024
c38de34
alignment
Fidget-Spinner May 5, 2024
106058c
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner May 6, 2024
f545b2f
fix merge problems
Fidget-Spinner May 6, 2024
0d1b38f
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner May 8, 2024
a597536
use PyStackRef_IsNull
Fidget-Spinner May 8, 2024
9d43727
tagged ->stackref
Fidget-Spinner May 8, 2024
bc96ae6
use Mark's naming scheme
Fidget-Spinner May 8, 2024
1d71dd1
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner May 8, 2024
e12337a
Update pycore_object_deferred.h
Fidget-Spinner May 8, 2024
91a8c81
tag all pointers, address some comments
Fidget-Spinner May 9, 2024
64bdfe2
fix free threaded, address more reviews
Fidget-Spinner May 9, 2024
88e0ea8
remove GC changes
Fidget-Spinner May 9, 2024
64b237c
address all of Mark's review
Fidget-Spinner May 10, 2024
97f6e14
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner May 10, 2024
834cdf0
fix refcounting issue
Fidget-Spinner May 10, 2024
99611cb
fix gdb failures
Fidget-Spinner May 10, 2024
512cd2e
fix specialization
Fidget-Spinner May 10, 2024
df176bf
fix tier 2
Fidget-Spinner May 10, 2024
576909d
fix optimizer builds
Fidget-Spinner May 10, 2024
adb54d8
don't check immortal when tagging
Fidget-Spinner May 10, 2024
d86652f
remove immortal check for tagging
Fidget-Spinner May 10, 2024
bca14a8
remove unsafe ptr conversions
Fidget-Spinner May 10, 2024
4eaece7
Revert "remove immortal check for tagging"
Fidget-Spinner May 10, 2024
4930f6f
Revert "don't check immortal when tagging"
Fidget-Spinner May 10, 2024
8670ee4
tag immortal objects
Fidget-Spinner May 10, 2024
144a6fa
Remove PyObject_To_StackRef_Steal
Fidget-Spinner May 10, 2024
c2bbe17
minor clenaups
Fidget-Spinner May 11, 2024
a6bfc97
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner May 27, 2024
e21a76a
Use Sam's and Mark's naming scheme
Fidget-Spinner May 28, 2024
e12942d
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner May 28, 2024
64c070d
regen
Fidget-Spinner May 28, 2024
4a103fb
fix test_generated_cases
Fidget-Spinner May 28, 2024
0bd7435
address partially reviews
Fidget-Spinner Jun 4, 2024
51908e9
convert the rest to stackrefs
Fidget-Spinner Jun 4, 2024
e99f019
fix tests
Fidget-Spinner Jun 4, 2024
fde830d
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 4, 2024
c73f8ac
ifdef out on default builds
Fidget-Spinner Jun 4, 2024
6a6bae2
add back XCLOSE and XDUP for perf reasons
Fidget-Spinner Jun 4, 2024
3d23f56
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 5, 2024
99fa382
Low the C recursion limit in WASI, it's failing
Fidget-Spinner Jun 5, 2024
5a5e329
Revert "Low the C recursion limit in WASI, it's failing"
Fidget-Spinner Jun 5, 2024
4a5ae8c
use macros instead of static inline for most things
Fidget-Spinner Jun 5, 2024
4e9740d
Revert "use macros instead of static inline for most things"
Fidget-Spinner Jun 5, 2024
c49cf1c
double WASI stack size of pydebug
Fidget-Spinner Jun 5, 2024
1b45992
Revert "Revert "use macros instead of static inline for most things""
Fidget-Spinner Jun 5, 2024
e5130a9
Revert "double WASI stack size of pydebug"
Fidget-Spinner Jun 5, 2024
505d640
inline functions directly for non-gil to pass on WASI debug
Fidget-Spinner Jun 5, 2024
5053536
Update configure
Fidget-Spinner Jun 5, 2024
0242212
convert more stuff to macros to try fix wasi
Fidget-Spinner Jun 5, 2024
12f8c0e
Increase wasm limit to 32kb
Fidget-Spinner Jun 5, 2024
db89f64
Revert "Increase wasm limit to 32kb"
Fidget-Spinner Jun 5, 2024
79714c2
skip test on WASI
Fidget-Spinner Jun 5, 2024
9a33b7f
fix uses of dup
Fidget-Spinner Jun 6, 2024
18894ca
refactor calls according to Mark
Fidget-Spinner Jun 6, 2024
67e0d25
fix bug
Fidget-Spinner Jun 6, 2024
4423794
fix macro
Fidget-Spinner Jun 6, 2024
66eb526
regen frozemain?
Fidget-Spinner Jun 6, 2024
2eda2f9
add PyStackRef_IsFalse/True
Fidget-Spinner Jun 6, 2024
8f97ae2
skip bad test
Fidget-Spinner Jun 6, 2024
58c3c55
fix tier 2 and JIT builds
Fidget-Spinner Jun 6, 2024
ca92d4a
fix macro
Fidget-Spinner Jun 7, 2024
3356eda
fix error handling
Fidget-Spinner Jun 7, 2024
8274425
revert unicode
Fidget-Spinner Jun 7, 2024
4405a5c
DCE hack
Fidget-Spinner Jun 7, 2024
2d69426
revert dictionary
Fidget-Spinner Jun 7, 2024
4f4107a
revert slice
Fidget-Spinner Jun 7, 2024
65be3fd
revert lists
Fidget-Spinner Jun 7, 2024
78dbf36
revert tuple
Fidget-Spinner Jun 7, 2024
a21eddd
Address review
Fidget-Spinner Jun 10, 2024
d744c8a
address changes (woops)
Fidget-Spinner Jun 10, 2024
6c76fe3
Address half of Mark's review
Fidget-Spinner Jun 11, 2024
8dc4fc6
Address rest of review
Fidget-Spinner Jun 11, 2024
8044c7e
Address rest of review
Fidget-Spinner Jun 11, 2024
90ebfc4
rename for clarity
Fidget-Spinner Jun 11, 2024
e4ccb65
rename PyStackRef_AsPyObjectDeferredToNew -> PyStackRef_AsPyObjectDef…
Fidget-Spinner Jun 12, 2024
2ff4f36
Drop defer, just steal
Fidget-Spinner Jun 12, 2024
992731d
Address half of Mark's review
Fidget-Spinner Jun 13, 2024
12f19ce
revert test frozenmain changes
Fidget-Spinner Jun 13, 2024
268eb0e
remove istrue
Fidget-Spinner Jun 13, 2024
d3b9e51
remove isfalse
Fidget-Spinner Jun 13, 2024
680e828
remove isnone
Fidget-Spinner Jun 13, 2024
27eda47
rename gen_frame_o
Fidget-Spinner Jun 13, 2024
be25a04
Fixup
Fidget-Spinner Jun 13, 2024
ae8c050
rename
Fidget-Spinner Jun 13, 2024
6e84d69
rename true and false to static const
Fidget-Spinner Jun 13, 2024
c523386
try fix on msvc
Fidget-Spinner Jun 13, 2024
d565b0d
try fix
Fidget-Spinner Jun 13, 2024
983764c
fix codegen
Fidget-Spinner Jun 14, 2024
7f3c02d
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 14, 2024
656f35c
Use macro
Fidget-Spinner Jun 14, 2024
92ada68
rename to PyStackRef_none
Fidget-Spinner Jun 14, 2024
24bd03f
Fix tier 2 generation
Fidget-Spinner Jun 14, 2024
fd1140c
cleanup code generator
Fidget-Spinner Jun 14, 2024
fc66d1c
Change is_abstract to extract_bits
Fidget-Spinner Jun 14, 2024
9dda41f
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 17, 2024
233f9e2
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 17, 2024
ef5a057
address review: use macro for conversion
Fidget-Spinner Jun 17, 2024
d2c14f7
fix space issue
Fidget-Spinner Jun 17, 2024
09a3848
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 18, 2024
40e72e1
(test) remove skips
Fidget-Spinner Jun 18, 2024
b33d161
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 18, 2024
b160409
fix upstream changes
Fidget-Spinner Jun 18, 2024
8ffd7ff
Convert functions back to static inline
Fidget-Spinner Jun 19, 2024
cbc1b6f
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 19, 2024
6adbcbf
Update test_frozenmain.h
Fidget-Spinner Jun 19, 2024
fdd24d6
Tighten diff
Fidget-Spinner Jun 20, 2024
9acd39d
Apply all suggestions and reviews by Mark
Fidget-Spinner Jun 21, 2024
e600102
Apply comment from line 1112
Fidget-Spinner Jun 21, 2024
add5f75
borrow, not steal
Fidget-Spinner Jun 21, 2024
f811462
Add documentation and guide
Fidget-Spinner Jun 22, 2024
c4dd69b
Merge remote-tracking branch 'upstream/main' into stackref_all
Fidget-Spinner Jun 22, 2024
60e2bb4
Address review by Mark
Fidget-Spinner Jun 22, 2024
c7eac22
fix for free-threaded builds
Fidget-Spinner Jun 22, 2024
8bf685a
fix typo
Fidget-Spinner Jun 22, 2024
7cf0369
Rename Py_STACKREF_NULL to PyStackRef_NULL
Fidget-Spinner Jun 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions Include/internal/pycore_call.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,24 @@ extern void _PyStack_UnpackDict_FreeNoDecRef(
PyObject *const *stack,
PyObject *kwnames);

PyAPI_FUNC(PyObject *)
Fidget-Spinner marked this conversation as resolved.
Show resolved Hide resolved
PyObject_Vectorcall_Tagged(PyObject *callable,
const _PyStackRef *tagged, size_t nargs, PyObject *kwnames);

PyAPI_FUNC(PyObject *)
PyObject_TypeVectorcall_Tagged(PyTypeObject *callable,
const _PyStackRef *tagged, size_t nargs, PyObject *kwnames);

PyAPI_FUNC(PyObject *)
PyObject_PyCFunctionFastCall_Tagged(PyCFunctionFast cfunc,
PyObject *self,
const _PyStackRef *tagged, Py_ssize_t nargs);
PyAPI_FUNC(PyObject *)
PyObject_PyCFunctionFastWithKeywordsCall_Tagged(PyCFunctionFastWithKeywords cfunc,
PyObject *self,
const _PyStackRef *tagged, Py_ssize_t nargs,
PyObject *kwds);

#ifdef __cplusplus
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ PyAPI_FUNC(void) _PyEval_FormatExcUnbound(PyThreadState *tstate, PyCodeObject *c
PyAPI_FUNC(void) _PyEval_FormatKwargsError(PyThreadState *tstate, PyObject *func, PyObject *kwargs);
PyAPI_FUNC(PyObject *)_PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, Py_ssize_t nargs, PyObject *kwargs);
PyAPI_FUNC(PyObject *)_PyEval_MatchKeys(PyThreadState *tstate, PyObject *map, PyObject *keys);
PyAPI_FUNC(int) _PyEval_UnpackIterable(PyThreadState *tstate, PyObject *v, int argcnt, int argcntafter, PyObject **sp);
PyAPI_FUNC(int) _PyEval_UnpackTaggedIterable(PyThreadState *tstate, PyObject *v, int argcnt, int argcntafter, _PyStackRef *sp);
Fidget-Spinner marked this conversation as resolved.
Show resolved Hide resolved
PyAPI_FUNC(void) _PyEval_FrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame *frame);


Expand Down
3 changes: 2 additions & 1 deletion Include/internal/pycore_code.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_stackref.h" // _PyStackRef

// We hide some of the newer PyCodeObject fields behind macros.
// This helps with backporting certain changes to 3.12.
Expand Down Expand Up @@ -278,7 +279,7 @@ extern void _Py_Specialize_StoreSubscr(PyObject *container, PyObject *sub,
extern void _Py_Specialize_Call(PyObject *callable, _Py_CODEUNIT *instr,
int nargs);
extern void _Py_Specialize_BinaryOp(PyObject *lhs, PyObject *rhs, _Py_CODEUNIT *instr,
int oparg, PyObject **locals);
int oparg, _PyStackRef *locals);
extern void _Py_Specialize_CompareOp(PyObject *lhs, PyObject *rhs,
_Py_CODEUNIT *instr, int oparg);
extern void _Py_Specialize_UnpackSequence(PyObject *seq, _Py_CODEUNIT *instr,
Expand Down
8 changes: 8 additions & 0 deletions Include/internal/pycore_dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,14 @@ PyAPI_FUNC(PyObject *)_PyDict_FromItems(
PyObject *const *keys, Py_ssize_t keys_offset,
PyObject *const *values, Py_ssize_t values_offset,
Py_ssize_t length);
PyAPI_FUNC(PyObject *)_PyDict_FromStackItems(
_PyStackRef const *keys, Py_ssize_t keys_offset,
_PyStackRef const *values, Py_ssize_t values_offset,
Py_ssize_t length);
PyAPI_FUNC(PyObject *)_PyDict_FromStackItemsUntaggedKeys(
PyObject *const *keys, Py_ssize_t keys_offset,
_PyStackRef const *values, Py_ssize_t values_offset,
Py_ssize_t length);

static inline uint8_t *
get_insertion_order_array(PyDictValues *values)
Expand Down
47 changes: 35 additions & 12 deletions Include/internal/pycore_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ extern "C" {
#include <stdbool.h>
#include <stddef.h> // offsetof()
#include "pycore_code.h" // STATS
#include "pycore_stackref.h" // _PyStackRef

/* See Objects/frame_layout.md for an explanation of the frame stack
* including explanation of the PyFrameObject and _PyInterpreterFrame
Expand Down Expand Up @@ -67,7 +68,7 @@ typedef struct _PyInterpreterFrame {
uint16_t return_offset; /* Only relevant during a function call */
char owner;
/* Locals and stack */
PyObject *localsplus[1];
_PyStackRef localsplus[1];
} _PyInterpreterFrame;

#define _PyInterpreterFrame_LASTI(IF) \
Expand All @@ -78,23 +79,23 @@ static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) {
return (PyCodeObject *)f->f_executable;
}

static inline PyObject **_PyFrame_Stackbase(_PyInterpreterFrame *f) {
return f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus;
static inline _PyStackRef *_PyFrame_Stackbase(_PyInterpreterFrame *f) {
return (f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus);
}

static inline PyObject *_PyFrame_StackPeek(_PyInterpreterFrame *f) {
static inline _PyStackRef _PyFrame_StackPeek(_PyInterpreterFrame *f) {
assert(f->stacktop > _PyFrame_GetCode(f)->co_nlocalsplus);
assert(f->localsplus[f->stacktop-1] != NULL);
assert(PyStackRef_Get(f->localsplus[f->stacktop-1]) != NULL);
return f->localsplus[f->stacktop-1];
}

static inline PyObject *_PyFrame_StackPop(_PyInterpreterFrame *f) {
static inline _PyStackRef _PyFrame_StackPop(_PyInterpreterFrame *f) {
assert(f->stacktop > _PyFrame_GetCode(f)->co_nlocalsplus);
f->stacktop--;
return f->localsplus[f->stacktop];
}

static inline void _PyFrame_StackPush(_PyInterpreterFrame *f, PyObject *value) {
static inline void _PyFrame_StackPush(_PyInterpreterFrame *f, _PyStackRef value) {
f->localsplus[f->stacktop] = value;
f->stacktop++;
}
Expand All @@ -120,6 +121,12 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *
// Don't leave a dangling pointer to the old frame when creating generators
// and coroutines:
dest->previous = NULL;
#ifdef Py_GIL_DISABLED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary. localsplus[i] where i >= stacktop should not be visible.
If they are visible somewhere, then that's a bug and should be fixed.

Copy link
Contributor

@colesbury colesbury May 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The top-most frame doesn't necessarily have an up-to-date stacktop so you need to scan up to the stack size.

You could try to keep stacktop up to date, but that would require either updating it at every opcode or at every call that might trigger a GC, including every Py_DECREF. (I did this in the nogil fork, but if you're not careful you can introduce a lot of bugs due to incorrect stacktop and it's harder to retrofit efficiently into the current interpreter.) EDIT: I misremembered how the nogil fork worked.

It's okay for the GC to look at objects that are marked as deferred because only the GC can free them, even if they are above stacktop.

PyCodeObject *co = (PyCodeObject *)dest->f_executable;
Fidget-Spinner marked this conversation as resolved.
Show resolved Hide resolved
for (int i = src->stacktop; i < co->co_nlocalsplus + co->co_stacksize; i++) {
dest->localsplus[i] = PyStackRef_StealRef(NULL);
Fidget-Spinner marked this conversation as resolved.
Show resolved Hide resolved
}
#endif
}

/* Consumes reference to func and locals.
Expand All @@ -143,14 +150,24 @@ _PyFrame_Initialize(
frame->owner = FRAME_OWNED_BY_THREAD;

for (int i = null_locals_from; i < code->co_nlocalsplus; i++) {
frame->localsplus[i] = NULL;
frame->localsplus[i] = PyStackRef_StealRef(NULL);
}

#ifdef Py_GIL_DISABLED
// On GIL disabled, we walk the entire stack in GC. Since stacktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like adding this sort of workaround. I would much rather fix the underlying issue.

Why can't we make sure that stacktop is in sync with the real stack pointer anywhere that GC could occur?
We don't do that now, because we don't need to, but I don't see a fundamental reason why we can't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A GC can occur at any Py_DECREF and most other C API calls -- that's pretty much every opcode except LOAD_FAST.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created #118926 for discussion on this.

// is not always in sync with the real stack pointer, we have
// no choice but to traverse the entire stack.
// This just makes sure we don't pass the GC invalid stack values.
for (int i = code->co_nlocalsplus; i < code->co_nlocalsplus + code->co_stacksize; i++) {
frame->localsplus[i] = PyStackRef_StealRef(NULL);
}
#endif
}

/* Gets the pointer to the locals array
* that precedes this frame.
*/
static inline PyObject**
static inline _PyStackRef*
_PyFrame_GetLocalsArray(_PyInterpreterFrame *frame)
{
return frame->localsplus;
Expand All @@ -160,16 +177,16 @@ _PyFrame_GetLocalsArray(_PyInterpreterFrame *frame)
Having stacktop <= 0 ensures that invalid
values are not visible to the cycle GC.
We choose -1 rather than 0 to assist debugging. */
static inline PyObject**
static inline _PyStackRef*
_PyFrame_GetStackPointer(_PyInterpreterFrame *frame)
{
PyObject **sp = frame->localsplus + frame->stacktop;
_PyStackRef *sp = frame->localsplus + frame->stacktop;
frame->stacktop = -1;
return sp;
}

static inline void
_PyFrame_SetStackPointer(_PyInterpreterFrame *frame, PyObject **stack_pointer)
_PyFrame_SetStackPointer(_PyInterpreterFrame *frame, _PyStackRef *stack_pointer)
{
frame->stacktop = (int)(stack_pointer - frame->localsplus);
}
Expand Down Expand Up @@ -307,6 +324,12 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
frame->instr_ptr = _PyCode_CODE(code);
frame->owner = FRAME_OWNED_BY_THREAD;
frame->return_offset = 0;
#ifdef Py_GIL_DISABLED
Fidget-Spinner marked this conversation as resolved.
Show resolved Hide resolved
assert(code->co_nlocalsplus == 0);
for (int i = 0; i < code->co_stacksize; i++) {
frame->localsplus[i] = PyStackRef_StealRef(NULL);
}
#endif
return frame;
}

Expand Down
3 changes: 3 additions & 0 deletions Include/internal/pycore_gc.h
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,9 @@ extern void _PyGC_ClearAllFreeLists(PyInterpreterState *interp);
extern void _Py_ScheduleGC(PyThreadState *tstate);
extern void _Py_RunGC(PyThreadState *tstate);

extern int _Py_visit_decref(PyObject *op, void *arg);
extern int _Py_visit_decref_unreachable(PyObject *op, void *data);

#ifdef Py_GIL_DISABLED
// gh-117783: Immortalize objects that use deferred reference counting
extern void _PyGC_ImmortalizeDeferredObjects(PyInterpreterState *interp);
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_jit.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ extern "C" {

#ifdef _Py_JIT

typedef _Py_CODEUNIT *(*jit_func)(_PyInterpreterFrame *frame, PyObject **stack_pointer, PyThreadState *tstate);
typedef _Py_CODEUNIT *(*jit_func)(_PyInterpreterFrame *frame, _PyStackRef *stack_pointer, PyThreadState *tstate);

int _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction *trace, size_t length);
void _PyJIT_Free(_PyExecutorObject *executor);
Expand Down
3 changes: 2 additions & 1 deletion Include/internal/pycore_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ extern "C" {
#endif

#include "pycore_freelist.h" // _PyFreeListState
#include "pycore_stackref.h" // _PyStackRef

PyAPI_FUNC(PyObject*) _PyList_Extend(PyListObject *, PyObject *);
extern void _PyList_DebugMallocStats(FILE *out);
Expand Down Expand Up @@ -54,7 +55,7 @@ typedef struct {
PyListObject *it_seq; /* Set to NULL when iterator is exhausted */
} _PyListIterObject;

PyAPI_FUNC(PyObject *)_PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n);
PyAPI_FUNC(PyObject *)_PyList_FromStackSteal(_PyStackRef const *src, Py_ssize_t n);

#ifdef __cplusplus
}
Expand Down
15 changes: 1 addition & 14 deletions Include/internal/pycore_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,6 @@ static inline void _Py_ClearImmortal(PyObject *op)
op = NULL; \
} while (0)

// Mark an object as supporting deferred reference counting. This is a no-op
// in the default (with GIL) build. Objects that use deferred reference
// counting should be tracked by the GC so that they are eventually collected.
extern void _PyObject_SetDeferredRefcount(PyObject *op);

static inline int
_PyObject_HasDeferredRefcount(PyObject *op)
{
#ifdef Py_GIL_DISABLED
return (op->ob_gc_bits & _PyGC_BITS_DEFERRED) != 0;
#else
return 0;
#endif
}

#if !defined(Py_GIL_DISABLED)
static inline void
Expand Down Expand Up @@ -709,6 +695,7 @@ PyAPI_FUNC(PyObject*) _PyObject_LookupSpecial(PyObject *, PyObject *);
extern int _PyObject_IsAbstract(PyObject *);

PyAPI_FUNC(int) _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method);
PyAPI_FUNC(int) _PyObject_GetMethodStackRef(PyObject *obj, PyObject *name, _PyStackRef *method);
extern PyObject* _PyObject_NextNotImplemented(PyObject *);

// Pickle support.
Expand Down
32 changes: 32 additions & 0 deletions Include/internal/pycore_object_deferred.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#ifndef Py_INTERNAL_OBJECT_DEFERRED_H
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of moving _PyObject_HasDeferredRefcount to it's own header, we should move the definition of the union _PyStackRef to it's own header (e.g., pycore_stackref_type.h). In other words, I think the better pattern is to isolate the definition of types rather than ad-hoc moving some inline functions. To summarize:

  • Keep pycore_object.h as it is in main
  • Move _PyStackRef to pycore_stackref_type.h or similar
  • Include pycore_stackref_type.h in pycore_stackref.h
  • Include pycore_stackref_type.h in the other headers that use _PyStackRef instead of pycore_stackref.h

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't seem to solve the problem where that pycore_stackref needs pycore_object.h (due to the _PyObject_HasDeferredRefcount dependency).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pycore_stackref.h including pycore_object.h is not a problem as long as pycore_object.h (and other headers that include it) do not include pycore_stackref.h.

pycore_stackref.h should only be included from .c files
pycore_stackref_type.h can be included in .h files that need the _PyStackRef type

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pycore_frame.h header needs to include pycore_stackref.h due to its static inline functions. By some long chain somewhere else, pycore_frame.h will eventually include pycore_object.h. Including pycore_object.h in pycore_stackref.h thus seems to cause a circular header.

#define Py_INTERNAL_OBJECT_DEFERRED_H

#ifdef __cplusplus
extern "C" {
#endif

#include "pycore_gc.h"

#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif

// Mark an object as supporting deferred reference counting. This is a no-op
// in the default (with GIL) build. Objects that use deferred reference
// counting should be tracked by the GC so that they are eventually collected.
extern void _PyObject_SetDeferredRefcount(PyObject *op);

static inline int
_PyObject_HasDeferredRefcount(PyObject *op)
{
#ifdef Py_GIL_DISABLED
return (op->ob_gc_bits & _PyGC_BITS_DEFERRED) != 0;
#else
return 0;
#endif
}

#ifdef __cplusplus
}
#endif
#endif // !Py_INTERNAL_OBJECT_DEFERRED_H
2 changes: 1 addition & 1 deletion Include/internal/pycore_opcode_metadata.h

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

2 changes: 1 addition & 1 deletion Include/internal/pycore_optimizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ extern int _Py_uop_frame_pop(_Py_UOpsContext *ctx);

PyAPI_FUNC(PyObject *) _Py_uop_symbols_test(PyObject *self, PyObject *ignored);

PyAPI_FUNC(int) _PyOptimizer_Optimize(_PyInterpreterFrame *frame, _Py_CODEUNIT *start, PyObject **stack_pointer, _PyExecutorObject **exec_ptr);
PyAPI_FUNC(int) _PyOptimizer_Optimize(_PyInterpreterFrame *frame, _Py_CODEUNIT *start, _PyStackRef *stack_pointer, _PyExecutorObject **exec_ptr);

#ifdef __cplusplus
}
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_sliceobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ extern "C" {
/* runtime lifecycle */

PyAPI_FUNC(PyObject *)
_PyBuildSlice_ConsumeRefs(PyObject *start, PyObject *stop);
_PyBuildSlice_ConsumeStackRefs(_PyStackRef start, _PyStackRef stop);

#ifdef __cplusplus
}
Expand Down
9 changes: 6 additions & 3 deletions Include/internal/pycore_stackref.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_object_deferred.h"

#include <stddef.h>

typedef union {
Expand Down Expand Up @@ -52,7 +54,7 @@ _PyStackRef_NewRefDeferred(PyObject *obj)
// Make sure we don't take an already tagged value.
assert(((uintptr_t)obj & Py_TAG_DEFERRED) == 0);
assert(obj != NULL);
if (_PyObject_HasDeferredRefcount(obj)) {
if (_PyObject_HasDeferredRefcount(obj) || _Py_IsImmortal(obj)) {
return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED };
}
else {
Expand Down Expand Up @@ -86,7 +88,8 @@ static inline PyObject *
PyStackRef_StealObject(_PyStackRef tagged)
{
if ((tagged.bits & Py_TAG_DEFERRED) == Py_TAG_DEFERRED) {
assert(_PyObject_HasDeferredRefcount(PyStackRef_Get(tagged)));
assert(_PyObject_HasDeferredRefcount(PyStackRef_Get(tagged)) ||
_Py_IsImmortal(PyStackRef_Get(tagged)));
return Py_NewRef(PyStackRef_Get(tagged));
}
return PyStackRef_Get(tagged);
Expand Down Expand Up @@ -114,7 +117,7 @@ _Py_untag_stack_steal(PyObject **dst, const _PyStackRef *src, size_t length)

#define PyStackRef_XSETREF(dst, src) \
do { \
_PyStackRef *_tmp_dst_ptr = &(dst) \
_PyStackRef *_tmp_dst_ptr = &(dst); \
_PyStackRef _tmp_old_dst = (*_tmp_dst_ptr); \
*_tmp_dst_ptr = (src); \
PyStackRef_XDECREF(_tmp_old_dst); \
Expand Down
3 changes: 3 additions & 0 deletions Include/internal/pycore_tuple.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_stackref.h" // _PyStackRef

extern void _PyTuple_MaybeUntrack(PyObject *);
extern void _PyTuple_DebugMallocStats(FILE *out);

Expand All @@ -22,6 +24,7 @@ extern PyStatus _PyTuple_InitGlobalObjects(PyInterpreterState *);

extern PyObject *_PyTuple_FromArray(PyObject *const *, Py_ssize_t);
PyAPI_FUNC(PyObject *)_PyTuple_FromArraySteal(PyObject *const *, Py_ssize_t);
PyAPI_FUNC(PyObject *)_PyTuple_FromStackSteal(_PyStackRef const *, Py_ssize_t);

typedef struct {
PyObject_HEAD
Expand Down
7 changes: 7 additions & 0 deletions Include/internal/pycore_unicodeobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ extern "C" {
#include "pycore_fileutils.h" // _Py_error_handler
#include "pycore_identifier.h" // _Py_Identifier
#include "pycore_ucnhash.h" // _PyUnicode_Name_CAPI
#include "pycore_stackref.h" // _PyStackRef

/* --- Characters Type APIs ----------------------------------------------- */

Expand Down Expand Up @@ -208,6 +209,12 @@ PyAPI_FUNC(PyObject*) _PyUnicode_JoinArray(
Py_ssize_t seqlen
);

PyAPI_FUNC(PyObject*) _PyUnicode_JoinStack(
PyObject *separator,
_PyStackRef const *items,
Py_ssize_t seqlen
);

/* Test whether a unicode is equal to ASCII identifier. Return 1 if true,
0 otherwise. The right argument must be ASCII identifier.
Any error occurs inside will be cleared before return. */
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_uop_metadata.h

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