Skip to content

Commit

Permalink
pythongh-117783: Immortalize objects that use deferred reference coun…
Browse files Browse the repository at this point in the history
…ting

Deferred reference counting is not fully implemented yet. As a temporary
measure, we immortalize objects that would use deferred reference
counting to avoid multi-threaded scaling bottlenecks.

This is only performed in the free-threaded build once the first
non-main thread is started. Additionally, some tests, including refleak
tests, suppress this behavior.
  • Loading branch information
colesbury committed Apr 19, 2024
1 parent 8f25cc9 commit 9ff8d52
Show file tree
Hide file tree
Showing 13 changed files with 144 additions and 6 deletions.
17 changes: 17 additions & 0 deletions Include/internal/pycore_gc.h
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,18 @@ struct _gc_runtime_state {
collections, and are awaiting to undergo a full collection for
the first time. */
Py_ssize_t long_lived_pending;

/* gh-117783: Deferred reference counting is not fully implemented yet, so
as a temporary measure we treat objects using deferred referenence
counting as immortal. */
struct {
/* Immortalize objects instead of marking them as using deferred
reference counting. */
int enabled;

/* Set enabled=1 when the first background thread is created. */
int enable_on_thread;
} immortalize;
#endif
};

Expand Down Expand Up @@ -343,6 +355,11 @@ extern void _PyGC_ClearAllFreeLists(PyInterpreterState *interp);
extern void _Py_ScheduleGC(PyThreadState *tstate);
extern void _Py_RunGC(PyThreadState *tstate);

#ifdef Py_GIL_DISABLED
// gh-117783: Immortalize objects that use deferred reference counting
extern void _PyGC_ImmortalizeDeferredObjects(PyInterpreterState *interp);
#endif

#ifdef __cplusplus
}
#endif
Expand Down
8 changes: 6 additions & 2 deletions Lib/test/libregrtest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
import time
import trace

from test.support import os_helper, MS_WINDOWS, flush_std_streams
from test.support import (os_helper, MS_WINDOWS, flush_std_streams,
suppress_immortalization)

from .cmdline import _parse_args, Namespace
from .findtests import findtests, split_test_packages, list_cases
Expand Down Expand Up @@ -526,7 +527,10 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
if self.num_workers:
self._run_tests_mp(runtests, self.num_workers)
else:
self.run_tests_sequentially(runtests)
# gh-117783: don't immortalize deferred objects when tracking
# refleaks. Only releveant for the free-threaded build.
with suppress_immortalization(runtests.hunt_refleak):
self.run_tests_sequentially(runtests)

coverage = self.results.get_coverage_results()
self.display_result(runtests)
Expand Down
5 changes: 4 additions & 1 deletion Lib/test/libregrtest/single.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,10 @@ def run_single_test(test_name: TestName, runtests: RunTests) -> TestResult:
result = TestResult(test_name)
pgo = runtests.pgo
try:
_runtest(result, runtests)
# gh-117783: don't immortalize deferred objects when tracking
# refleaks. Only releveant for the free-threaded build.
with support.suppress_immortalization(runtests.hunt_refleak):
_runtest(result, runtests)
except:
if not pgo:
msg = traceback.format_exc()
Expand Down
19 changes: 19 additions & 0 deletions Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,25 @@ def has_no_debug_ranges():
def requires_debug_ranges(reason='requires co_positions / debug_ranges'):
return unittest.skipIf(has_no_debug_ranges(), reason)

@contextlib.contextmanager
def suppress_immortalization(suppress=True):
"""Suppress immortalization of deferred objects."""
try:
import _testinternalcapi
except ImportError:
yield
return

if not suppress:
yield
return

old_values = _testinternalcapi.set_immortalize_deferred(False)
try:
yield
finally:
_testinternalcapi.set_immortalize_deferred(*old_values)

MS_WINDOWS = (sys.platform == 'win32')

# Is not actually used in tests, but is kept for compatibility.
Expand Down
6 changes: 5 additions & 1 deletion Lib/test/test_capi/test_watchers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import unittest

from contextlib import contextmanager, ExitStack
from test.support import catch_unraisable_exception, import_helper, gc_collect
from test.support import (
catch_unraisable_exception, import_helper,
gc_collect, suppress_immortalization)


# Skip this test if the _testcapi module isn't available.
Expand Down Expand Up @@ -382,6 +384,7 @@ def assert_event_counts(self, exp_created_0, exp_destroyed_0,
self.assertEqual(
exp_destroyed_1, _testcapi.get_code_watcher_num_destroyed_events(1))

@suppress_immortalization()
def test_code_object_events_dispatched(self):
# verify that all counts are zero before any watchers are registered
self.assert_event_counts(0, 0, 0, 0)
Expand Down Expand Up @@ -428,6 +431,7 @@ def test_error(self):
self.assertIsNone(cm.unraisable.object)
self.assertEqual(str(cm.unraisable.exc_value), "boom!")

@suppress_immortalization()
def test_dealloc_error(self):
co = _testcapi.code_newempty("test_watchers", "dummy0", 0)
with self.code_watcher(2):
Expand Down
6 changes: 5 additions & 1 deletion Lib/test/test_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@
ctypes = None
from test.support import (cpython_only,
check_impl_detail, requires_debug_ranges,
gc_collect, Py_GIL_DISABLED)
gc_collect, Py_GIL_DISABLED,
suppress_immortalization)
from test.support.script_helper import assert_python_ok
from test.support import threading_helper, import_helper
from test.support.bytecode_helper import instructions_with_positions
Expand Down Expand Up @@ -577,6 +578,7 @@ def test_interned_string_with_null(self):

class CodeWeakRefTest(unittest.TestCase):

@suppress_immortalization()
def test_basic(self):
# Create a code object in a clean environment so that we know we have
# the only reference to it left.
Expand Down Expand Up @@ -827,6 +829,7 @@ def test_bad_index(self):
self.assertEqual(GetExtra(f.__code__, FREE_INDEX+100,
ctypes.c_voidp(100)), 0)

@suppress_immortalization()
def test_free_called(self):
# Verify that the provided free function gets invoked
# when the code object is cleaned up.
Expand Down Expand Up @@ -854,6 +857,7 @@ def test_get_set(self):
del f

@threading_helper.requires_working_threading()
@suppress_immortalization()
def test_free_different_thread(self):
# Freeing a code object on a different thread then
# where the co_extra was set should be safe.
Expand Down
1 change: 1 addition & 0 deletions Lib/test/test_functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1833,6 +1833,7 @@ def f():
return 1
self.assertEqual(f.cache_parameters(), {'maxsize': 1000, "typed": True})

@support.suppress_immortalization()
def test_lru_cache_weakrefable(self):
@self.module.lru_cache
def test_function(x):
Expand Down
5 changes: 4 additions & 1 deletion Lib/test/test_weakref.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import random

from test import support
from test.support import script_helper, ALWAYS_EQ
from test.support import script_helper, ALWAYS_EQ, suppress_immortalization
from test.support import gc_collect
from test.support import import_helper
from test.support import threading_helper
Expand Down Expand Up @@ -650,6 +650,7 @@ class C(object):
# deallocation of c2.
del c2

@suppress_immortalization()
def test_callback_in_cycle(self):
import gc

Expand Down Expand Up @@ -742,6 +743,7 @@ class D:
del c1, c2, C, D
gc.collect()

@suppress_immortalization()
def test_callback_in_cycle_resurrection(self):
import gc

Expand Down Expand Up @@ -877,6 +879,7 @@ def test_init(self):
# No exception should be raised here
gc.collect()

@suppress_immortalization()
def test_classes(self):
# Check that classes are weakrefable.
class A(object):
Expand Down
22 changes: 22 additions & 0 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1936,6 +1936,27 @@ get_py_thread_id(PyObject *self, PyObject *Py_UNUSED(ignored))
}
#endif

static PyObject *
set_immortalize_deferred(PyObject *self, PyObject *value)
{
#ifdef Py_GIL_DISABLED
PyInterpreterState *interp = PyInterpreterState_Get();
int old_enabled = interp->gc.immortalize.enabled;
int old_enabled_on_thread = interp->gc.immortalize.enable_on_thread;
int enabled_on_thread = 0;
if (!PyArg_ParseTuple(value, "i|i",
&interp->gc.immortalize.enabled,
&enabled_on_thread))
{
return NULL;
}
interp->gc.immortalize.enable_on_thread = enabled_on_thread;
return Py_BuildValue("ii", old_enabled, old_enabled_on_thread);
#else
Py_RETURN_FALSE;
#endif
}

static PyObject *
has_inline_values(PyObject *self, PyObject *obj)
{
Expand Down Expand Up @@ -2029,6 +2050,7 @@ static PyMethodDef module_functions[] = {
#ifdef Py_GIL_DISABLED
{"py_thread_id", get_py_thread_id, METH_NOARGS},
#endif
{"set_immortalize_deferred", set_immortalize_deferred, METH_VARARGS},
{"uop_symbols_test", _Py_uop_symbols_test, METH_NOARGS},
{NULL, NULL} /* sentinel */
};
Expand Down
7 changes: 7 additions & 0 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -2433,6 +2433,13 @@ _PyObject_SetDeferredRefcount(PyObject *op)
assert(PyType_IS_GC(Py_TYPE(op)));
assert(_Py_IsOwnedByCurrentThread(op));
assert(op->ob_ref_shared == 0);
PyInterpreterState *interp = _PyInterpreterState_GET();
if (interp->gc.immortalize.enabled) {
// gh-117696: immortalize objects instead of using deferred reference
// counting for now.
_Py_SetImmortal(op);
return;
}
op->ob_gc_bits |= _PyGC_BITS_DEFERRED;
op->ob_ref_local += 1;
op->ob_ref_shared = _Py_REF_QUEUED;
Expand Down
28 changes: 28 additions & 0 deletions Python/gc_free_threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,10 @@ _PyGC_Init(PyInterpreterState *interp)
{
GCState *gcstate = &interp->gc;

if (_Py_IsMainInterpreter(interp)) {
gcstate->immortalize.enable_on_thread = 1;
}

gcstate->garbage = PyList_New(0);
if (gcstate->garbage == NULL) {
return _PyStatus_NO_MEMORY();
Expand Down Expand Up @@ -1781,6 +1785,30 @@ custom_visitor_wrapper(const mi_heap_t *heap, const mi_heap_area_t *area,
return true;
}

// gh-117783: Immortalize objects that use deferred reference counting to
// temporarily work around scaling bottlenecks.
static bool
immortalize_visitor(const mi_heap_t *heap, const mi_heap_area_t *area,
void *block, size_t block_size, void *args)
{
PyObject *op = op_from_block(block, args, false);
if (op != NULL && _PyObject_HasDeferredRefcount(op)) {
_Py_SetImmortal(op);
op->ob_gc_bits &= ~_PyGC_BITS_DEFERRED;
}
return true;
}

void
_PyGC_ImmortalizeDeferredObjects(PyInterpreterState *interp)
{
struct visitor_args args;
_PyEval_StopTheWorld(interp);
gc_visit_heaps(interp, &immortalize_visitor, &args);
interp->gc.immortalize.enabled = 1;
_PyEval_StartTheWorld(interp);
}

void
PyUnstable_GC_VisitObjects(gcvisitobjects_t callback, void *arg)
{
Expand Down
17 changes: 17 additions & 0 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,22 @@ finalize_modules_delete_special(PyThreadState *tstate, int verbose)
}
}

static void
swap_module_dict(PyModuleObject *mod)
{
if (_Py_IsImmortal(mod->md_dict)) {
// gh-117783: Immortalizing module dicts can cause some finalizers to
// run much later than typical leading to attribute errors due to
// partially cleared modules. To avoid this, we copy the module dict
// if it was immortalized.
PyObject *copy = PyDict_Copy(mod->md_dict);
if (copy == NULL) {
PyErr_FormatUnraisable("Exception ignored on removing modules");
return;
}
Py_SETREF(mod->md_dict, copy);
}
}

static PyObject*
finalize_remove_modules(PyObject *modules, int verbose)
Expand Down Expand Up @@ -1521,6 +1537,7 @@ finalize_remove_modules(PyObject *modules, int verbose)
if (verbose && PyUnicode_Check(name)) { \
PySys_FormatStderr("# cleanup[2] removing %U\n", name); \
} \
swap_module_dict((PyModuleObject *)mod); \
STORE_MODULE_WEAKREF(name, mod); \
if (PyObject_SetItem(modules, name, Py_None) < 0) { \
PyErr_FormatUnraisable("Exception ignored on removing modules"); \
Expand Down
9 changes: 9 additions & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1567,6 +1567,15 @@ new_threadstate(PyInterpreterState *interp, int whence)
// Must be called with lock unlocked to avoid re-entrancy deadlock.
PyMem_RawFree(new_tstate);
}
else {
#ifdef Py_GIL_DISABLED
if (interp->gc.immortalize.enable_on_thread && !interp->gc.immortalize.enabled) {
// Immortalize objects marked as using deferred reference counting
// the first time a non-main thread is created.
_PyGC_ImmortalizeDeferredObjects(interp);
}
#endif
}

#ifdef Py_GIL_DISABLED
// Must be called with lock unlocked to avoid lock ordering deadlocks.
Expand Down

0 comments on commit 9ff8d52

Please sign in to comment.