8000 gh-112529: Implement GC for free-threaded builds by colesbury · Pull Request #114262 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-112529: Implement GC for free-threaded builds #114262

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

Merged
merged 7 commits into from
Jan 25, 2024
Merged

gh-112529: Implement GC for free-threaded builds #114262

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1eb8a0c
gh-112529: Implement GC for free-threaded builds
colesbury Jan 5, 2024
27199cc
Merge branch 'main' into gh-112529-gc
colesbury Jan 23, 2024
baf8081
Add stop-the-world calls to gc_free_threading.c
colesbury Jan 23, 2024
8a18a7a
Merge branch 'main' into gh-112529-gc
colesbury Jan 23, 2024
5676983
Rename "from_block" and add comment to ob_tid definition
colesbury Jan 24, 2024
d13a206
Add helper functions gc_restore_refs and gc_maybe_init_refs.
colesbury Jan 24, 2024
8814b34
Stop the world before calling gc_visit_heaps.
colesbury Jan 24, 2024
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
Prev Previous commit
Stop the world before calling gc_visit_heaps.
  • Loading branch information
@colesbury
colesbury committed Jan 24, 2024
commit 8814b344aa41e0f928df38f2583d4a52cd19239f
40 changes: 38 additions & 2 deletions Python/gc_free_threading.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,16 +148,20 @@ gc_decref(PyObject *op)
op->ob_tid -= 1;
}

// Merge refcounts while the world is stopped.
static void
merge_refcount(PyObject *op, Py_ssize_t extra)
{
assert(_PyInterpreterState_GET()->stoptheworld.world_stopped);

Py_ssize_t refcount = Py_REFCNT(op);
refcount += extra;

#ifdef Py_REF_DEBUG
_Py_AddRefTotal(_PyInterpreterState_GET(), extra);
#endif

// No atomics necessary; all other threads in this interpreter are paused.
op->ob_tid = 0;
op->ob_ref_local = 0;
op->ob_ref_shared = _Py_REF_SHARED(refcount, _Py_REF_MERGED);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this needs to be an atomic operation? Can this just be a call to _Py_ExplicitMergeRefcount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other threads in the interpreter must be paused, so no atomics necessary. I'll add an assert here.

We could probably use _Py_ExplicitMergeRefcount(), but it seems convenient to avoid the atomic operations.

Expand Down Expand Up @@ -263,6 +267,10 @@ static int
gc_visit_heaps(PyInterpreterState *interp, mi_block_visit_fun *visitor,
struct visitor_args *arg)
{
// Other threads in the interpreter must be paused so that we can safely
// traverse their heaps.
assert(interp->stoptheworld.world_stopped);

int err;
HEAD_LOCK(&_PyRuntime);
err = gc_visit_heaps_lock_held(interp, visitor, arg);
Expand Down Expand Up @@ -1167,18 +1175,21 @@ _PyGC_GetReferrers(PyInterpreterState *interp, PyObject *objs)
return NULL;
}

_PyEval_StopTheWorld(interp);

// Append all objects to a worklist. This abuses ob_tid. We will restore
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth stopping the world here? It seems like we're fine to race with ob_tid from a correctness stand point, but it seems like a lot of objects are going to end up with merged ref counts in a multithreaded environment!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to stop the world for the heap traversal (gc_visit_heaps) to be thread-safe... which I forgot to add here. I'll also add an assert to gc_visit_heaps() that the world is stopped.

This doesn't merge most refcounts. They get restored from the mimalloc data structure. They'll only get merged if the owning thread has already exited.

// it later. NOTE: We can't append to the PyListObject during
// gc_visit_heaps() because PyList_Append() may reclaim an abandoned
// mimalloc segments while we are traversing them.
struct get_referrers_args args = { .objs = objs };
gc_visit_heaps(interp, &visit_get_referrers, &args.base);

bool error = false;
PyObject *op;
while ((op = worklist_pop(&args.results)) != NULL) {
gc_restore_tid(op);
if (op != objs && PyList_Append(result, op) < 0) {
Py_CLEAR(result);
error = true;
break;
}
}
Expand All @@ -1188,6 +1199,13 @@ _PyGC_GetReferrers(PyInterpreterState *interp, PyObject *objs)
gc_restore_tid(op);
}

_PyEval_StartTheWorld(interp);

if (error) {
Py_DECREF(result);
return NULL;
}

return result;
}

Expand Down Expand Up @@ -1220,18 +1238,21 @@ _PyGC_GetObjects(PyInterpreterState *interp, Py_ssize_t generation)
return NULL;
}

_PyEval_StopTheWorld(interp);

// Append all objects to a worklist. This abuses ob_tid. We will restore
// it later. NOTE: We can't append to the list during gc_visit_heaps()
// because PyList_Append() may reclaim an abandoned mimalloc segment
// while we are traversing it.
struct get_objects_args args = { 0 };
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto to get referrers

gc_visit_heaps(interp, &visit_get_objects, &args.base);

bool error = false;
PyObject *op;
while ((op = worklist_pop(&args.objects)) != NULL) {
gc_restore_tid(op);
if (op != result && PyList_Append(result, op) < 0) {
Py_CLEAR(result);
error = true;
break;
}
}
Expand All @@ -1241,6 +1262,13 @@ _PyGC_GetObjects(PyInterpreterState *interp, Py_ssize_t generation)
gc_restore_tid(op);
}

_PyEval_StartTheWorld(interp);

if (error) {
Py_DECREF(result);
return NULL;
}

return result;
}

Expand All @@ -1259,7 +1287,9 @@ void
_PyGC_Freeze(PyInterpreterState *interp)
{
struct visitor_args args;
_PyEval_StopTheWorld(interp);
gc_visit_heaps(interp, &visit_freeze, &args);
_PyEval_StartTheWorld(interp);
}

static bool
Expand All @@ -1277,7 +1307,9 @@ void
_PyGC_Unfreeze(PyInterpreterState *interp)
{
struct visitor_args args;
_PyEval_StopTheWorld(interp);
gc_visit_heaps(interp, &visit_unfreeze, &args);
_PyEval_StartTheWorld(interp);
}

struct count_frozen_args {
Expand All @@ -1301,7 +1333,9 @@ Py_ssize_t
_PyGC_GetFreezeCount(PyInterpreterState *interp)
{
struct count_frozen_args args = { .count = 0 };
_PyEval_StopTheWorld(interp);
gc_visit_heaps(interp, &visit_count_frozen, &args.base);
_PyEval_StartTheWorld(interp);
return args.count;
}

Expand Down Expand Up @@ -1660,7 +1694,9 @@ PyUnstable_GC_VisitObjects(gcvisitobjects_t callback, void *arg)
.arg = arg,
};

_PyEval_StopTheWorld(interp);
gc_visit_heaps(interp, &custom_visitor_wrapper, &wrapper.base);
_PyEval_StartTheWorld(interp);
}

/* Clear all free lists
Expand Down
0