gh-118527: Intern code consts in free-threaded build (#118667)

We already intern and immortalize most string constants. In the
free-threaded build, other constants can be a source of reference count
contention because they are shared by all threads running the same code
objects.
This commit is contained in:
Sam Gross 2024-05-06 20:12:39 -04:00 committed by GitHub
parent 8d8275b0cf
commit 723d4d2fe8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 375 additions and 18 deletions

View File

@ -8,6 +8,8 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define" # error "this header requires Py_BUILD_CORE define"
#endif #endif
#include "pycore_lock.h" // PyMutex
// We hide some of the newer PyCodeObject fields behind macros. // We hide some of the newer PyCodeObject fields behind macros.
// This helps with backporting certain changes to 3.12. // This helps with backporting certain changes to 3.12.
@ -16,6 +18,14 @@ extern "C" {
#define _PyCode_HAS_INSTRUMENTATION(CODE) \ #define _PyCode_HAS_INSTRUMENTATION(CODE) \
(CODE->_co_instrumentation_version > 0) (CODE->_co_instrumentation_version > 0)
struct _py_code_state {
PyMutex mutex;
// Interned constants from code objects. Used by the free-threaded build.
struct _Py_hashtable_t *constants;
};
extern PyStatus _PyCode_Init(PyInterpreterState *interp);
extern void _PyCode_Fini(PyInterpreterState *interp);
#define CODE_MAX_WATCHERS 8 #define CODE_MAX_WATCHERS 8

View File

@ -245,6 +245,7 @@ struct _is {
struct _Py_long_state long_state; struct _Py_long_state long_state;
struct _dtoa_state dtoa; struct _dtoa_state dtoa;
struct _py_func_state func_state; struct _py_func_state func_state;
struct _py_code_state code_state;
struct _Py_dict_state dict_state; struct _Py_dict_state dict_state;
struct _Py_exc_state exc_state; struct _Py_exc_state exc_state;

View File

@ -30,6 +30,9 @@ PyAPI_DATA(PyObject *) _PySet_Dummy;
PyAPI_FUNC(int) _PySet_Contains(PySetObject *so, PyObject *key); PyAPI_FUNC(int) _PySet_Contains(PySetObject *so, PyObject *key);
// Clears the set without acquiring locks. Used by _PyCode_Fini.
extern void _PySet_ClearInternal(PySetObject *so);
#ifdef __cplusplus #ifdef __cplusplus
} }
#endif #endif

View File

@ -535,6 +535,15 @@ def suppress_immortalization(suppress=True):
finally: finally:
_testinternalcapi.set_immortalize_deferred(*old_values) _testinternalcapi.set_immortalize_deferred(*old_values)
def skip_if_suppress_immortalization():
try:
import _testinternalcapi
except ImportError:
return
return unittest.skipUnless(_testinternalcapi.get_immortalize_deferred(),
"requires immortalization of deferred objects")
MS_WINDOWS = (sys.platform == 'win32') MS_WINDOWS = (sys.platform == 'win32')
# Is not actually used in tests, but is kept for compatibility. # Is not actually used in tests, but is kept for compatibility.

View File

@ -142,7 +142,8 @@ except ImportError:
from test.support import (cpython_only, from test.support import (cpython_only,
check_impl_detail, requires_debug_ranges, check_impl_detail, requires_debug_ranges,
gc_collect, Py_GIL_DISABLED, gc_collect, Py_GIL_DISABLED,
suppress_immortalization) suppress_immortalization,
skip_if_suppress_immortalization)
from test.support.script_helper import assert_python_ok from test.support.script_helper import assert_python_ok
from test.support import threading_helper, import_helper from test.support import threading_helper, import_helper
from test.support.bytecode_helper import instructions_with_positions from test.support.bytecode_helper import instructions_with_positions
@ -570,11 +571,31 @@ class CodeConstsTest(unittest.TestCase):
self.assertIsInterned(f()) self.assertIsInterned(f())
@cpython_only @cpython_only
@unittest.skipIf(Py_GIL_DISABLED, "free-threaded build interns all string constants")
def test_interned_string_with_null(self): def test_interned_string_with_null(self):
co = compile(r'res = "str\0value!"', '?', 'exec') co = compile(r'res = "str\0value!"', '?', 'exec')
v = self.find_const(co.co_consts, 'str\0value!') v = self.find_const(co.co_consts, 'str\0value!')
self.assertIsNotInterned(v) self.assertIsNotInterned(v)
@cpython_only
@unittest.skipUnless(Py_GIL_DISABLED, "does not intern all constants")
@skip_if_suppress_immortalization()
def test_interned_constants(self):
# compile separately to avoid compile time de-duping
globals = {}
exec(textwrap.dedent("""
def func1():
return (0.0, (1, 2, "hello"))
"""), globals)
exec(textwrap.dedent("""
def func2():
return (0.0, (1, 2, "hello"))
"""), globals)
self.assertTrue(globals["func1"]() is globals["func2"]())
class CodeWeakRefTest(unittest.TestCase): class CodeWeakRefTest(unittest.TestCase):

View File

@ -28,7 +28,7 @@ class ObjectsTestCase(unittest.TestCase):
self.assertEqual(ci._objects, None) self.assertEqual(ci._objects, None)
def test_c_char_p(self): def test_c_char_p(self):
s = b"Hello, World" s = "Hello, World".encode("ascii")
refcnt = sys.getrefcount(s) refcnt = sys.getrefcount(s)
cs = c_char_p(s) cs = c_char_p(s)
self.assertEqual(refcnt + 1, sys.getrefcount(s)) self.assertEqual(refcnt + 1, sys.getrefcount(s))

View File

@ -47,7 +47,7 @@ class PythonAPITestCase(unittest.TestCase):
@support.refcount_test @support.refcount_test
def test_PyObj_FromPtr(self): def test_PyObj_FromPtr(self):
s = "abc def ghi jkl" s = object()
ref = sys.getrefcount(s) ref = sys.getrefcount(s)
# id(python-object) is the address # id(python-object) is the address
pyobj = _ctypes.PyObj_FromPtr(id(s)) pyobj = _ctypes.PyObj_FromPtr(id(s))

View File

@ -801,7 +801,7 @@ class CBytesIOTest(PyBytesIOTest):
def _test_cow_mutation(self, mutation): def _test_cow_mutation(self, mutation):
# Common code for all BytesIO copy-on-write mutation tests. # Common code for all BytesIO copy-on-write mutation tests.
imm = b' ' * 1024 imm = (' ' * 1024).encode("ascii")
old_rc = sys.getrefcount(imm) old_rc = sys.getrefcount(imm)
memio = self.ioclass(imm) memio = self.ioclass(imm)
self.assertEqual(sys.getrefcount(imm), old_rc + 1) self.assertEqual(sys.getrefcount(imm), old_rc + 1)

View File

@ -1985,6 +1985,17 @@ set_immortalize_deferred(PyObject *self, PyObject *value)
#endif #endif
} }
static PyObject *
get_immortalize_deferred(PyObject *self, PyObject *Py_UNUSED(ignored))
{
#ifdef Py_GIL_DISABLED
PyInterpreterState *interp = PyInterpreterState_Get();
return PyBool_FromLong(interp->gc.immortalize.enable_on_thread_created);
#else
Py_RETURN_FALSE;
#endif
}
static PyObject * static PyObject *
has_inline_values(PyObject *self, PyObject *obj) has_inline_values(PyObject *self, PyObject *obj)
{ {
@ -2081,6 +2092,7 @@ static PyMethodDef module_functions[] = {
{"py_thread_id", get_py_thread_id, METH_NOARGS}, {"py_thread_id", get_py_thread_id, METH_NOARGS},
#endif #endif
{"set_immortalize_deferred", set_immortalize_deferred, METH_VARARGS}, {"set_immortalize_deferred", set_immortalize_deferred, METH_VARARGS},
{"get_immortalize_deferred", get_immortalize_deferred, METH_NOARGS},
#ifdef _Py_TIER2 #ifdef _Py_TIER2
{"uop_symbols_test", _Py_uop_symbols_test, METH_NOARGS}, {"uop_symbols_test", _Py_uop_symbols_test, METH_NOARGS},
#endif #endif

View File

@ -5,6 +5,8 @@
#include "pycore_code.h" // _PyCodeConstructor #include "pycore_code.h" // _PyCodeConstructor
#include "pycore_frame.h" // FRAME_SPECIALS_SIZE #include "pycore_frame.h" // FRAME_SPECIALS_SIZE
#include "pycore_hashtable.h" // _Py_hashtable_t
#include "pycore_initconfig.h" // _PyStatus_OK()
#include "pycore_interp.h" // PyInterpreterState.co_extra_freefuncs #include "pycore_interp.h" // PyInterpreterState.co_extra_freefuncs
#include "pycore_object.h" // _PyObject_SetDeferredRefcount #include "pycore_object.h" // _PyObject_SetDeferredRefcount
#include "pycore_opcode_metadata.h" // _PyOpcode_Deopt, _PyOpcode_Caches #include "pycore_opcode_metadata.h" // _PyOpcode_Deopt, _PyOpcode_Caches
@ -100,10 +102,20 @@ PyCode_ClearWatcher(int watcher_id)
* generic helpers * generic helpers
******************/ ******************/
/* all_name_chars(s): true iff s matches [a-zA-Z0-9_]* */
static int static int
all_name_chars(PyObject *o) should_intern_string(PyObject *o)
{ {
#ifdef Py_GIL_DISABLED
// The free-threaded build interns (and immortalizes) all string constants
// unless we've disabled immortalizing objects that use deferred reference
// counting.
PyInterpreterState *interp = _PyInterpreterState_GET();
if (interp->gc.immortalize.enable_on_thread_created) {
return 1;
}
#endif
// compute if s matches [a-zA-Z0-9_]
const unsigned char *s, *e; const unsigned char *s, *e;
if (!PyUnicode_IS_ASCII(o)) if (!PyUnicode_IS_ASCII(o))
@ -118,6 +130,10 @@ all_name_chars(PyObject *o)
return 1; return 1;
} }
#ifdef Py_GIL_DISABLED
static PyObject *intern_one_constant(PyObject *op);
#endif
static int static int
intern_strings(PyObject *tuple) intern_strings(PyObject *tuple)
{ {
@ -135,14 +151,16 @@ intern_strings(PyObject *tuple)
return 0; return 0;
} }
/* Intern selected string constants */ /* Intern constants. In the default build, this interns selected string
constants. In the free-threaded build, this also interns non-string
constants. */
static int static int
intern_string_constants(PyObject *tuple, int *modified) intern_constants(PyObject *tuple, int *modified)
{ {
for (Py_ssize_t i = PyTuple_GET_SIZE(tuple); --i >= 0; ) { for (Py_ssize_t i = PyTuple_GET_SIZE(tuple); --i >= 0; ) {
PyObject *v = PyTuple_GET_ITEM(tuple, i); PyObject *v = PyTuple_GET_ITEM(tuple, i);
if (PyUnicode_CheckExact(v)) { if (PyUnicode_CheckExact(v)) {
if (all_name_chars(v)) { if (should_intern_string(v)) {
PyObject *w = v; PyObject *w = v;
PyUnicode_InternInPlace(&v); PyUnicode_InternInPlace(&v);
if (w != v) { if (w != v) {
@ -154,7 +172,7 @@ intern_string_constants(PyObject *tuple, int *modified)
} }
} }
else if (PyTuple_CheckExact(v)) { else if (PyTuple_CheckExact(v)) {
if (intern_string_constants(v, NULL) < 0) { if (intern_constants(v, NULL) < 0) {
return -1; return -1;
} }
} }
@ -165,7 +183,7 @@ intern_string_constants(PyObject *tuple, int *modified)
return -1; return -1;
} }
int tmp_modified = 0; int tmp_modified = 0;
if (intern_string_constants(tmp, &tmp_modified) < 0) { if (intern_constants(tmp, &tmp_modified) < 0) {
Py_DECREF(tmp); Py_DECREF(tmp);
return -1; return -1;
} }
@ -184,6 +202,59 @@ intern_string_constants(PyObject *tuple, int *modified)
} }
Py_DECREF(tmp); Py_DECREF(tmp);
} }
#ifdef Py_GIL_DISABLED
else if (PySlice_Check(v)) {
PySliceObject *slice = (PySliceObject *)v;
PyObject *tmp = PyTuple_New(3);
if (tmp == NULL) {
return -1;
}
PyTuple_SET_ITEM(tmp, 0, Py_NewRef(slice->start));
PyTuple_SET_ITEM(tmp, 1, Py_NewRef(slice->stop));
PyTuple_SET_ITEM(tmp, 2, Py_NewRef(slice->step));
int tmp_modified = 0;
if (intern_constants(tmp, &tmp_modified) < 0) {
Py_DECREF(tmp);
return -1;
}
if (tmp_modified) {
v = PySlice_New(PyTuple_GET_ITEM(tmp, 0),
PyTuple_GET_ITEM(tmp, 1),
PyTuple_GET_ITEM(tmp, 2));
if (v == NULL) {
Py_DECREF(tmp);
return -1;
}
PyTuple_SET_ITEM(tuple, i, v);
Py_DECREF(slice);
if (modified) {
*modified = 1;
}
}
Py_DECREF(tmp);
}
// Intern non-string consants in the free-threaded build, but only if
// we are also immortalizing objects that use deferred reference
// counting.
PyThreadState *tstate = PyThreadState_GET();
if (!_Py_IsImmortal(v) && !PyCode_Check(v) &&
!PyUnicode_CheckExact(v) &&
tstate->interp->gc.immortalize.enable_on_thread_created)
{
PyObject *interned = intern_one_constant(v);
if (interned == NULL) {
return -1;
}
else if (interned != v) {
PyTuple_SET_ITEM(tuple, i, interned);
Py_SETREF(v, interned);
if (modified) {
*modified = 1;
}
}
}
#endif
} }
return 0; return 0;
} }
@ -540,18 +611,41 @@ remove_column_info(PyObject *locations)
return res; return res;
} }
static int
intern_code_constants(struct _PyCodeConstructor *con)
{
#ifdef Py_GIL_DISABLED
PyInterpreterState *interp = _PyInterpreterState_GET();
struct _py_code_state *state = &interp->code_state;
PyMutex_Lock(&state->mutex);
#endif
if (intern_strings(con->names) < 0) {
goto error;
}
if (intern_constants(con->consts, NULL) < 0) {
goto error;
}
if (intern_strings(con->localsplusnames) < 0) {
goto error;
}
#ifdef Py_GIL_DISABLED
PyMutex_Unlock(&state->mutex);
#endif
return 0;
error:
#ifdef Py_GIL_DISABLED
PyMutex_Unlock(&state->mutex);
#endif
return -1;
}
/* The caller is responsible for ensuring that the given data is valid. */ /* The caller is responsible for ensuring that the given data is valid. */
PyCodeObject * PyCodeObject *
_PyCode_New(struct _PyCodeConstructor *con) _PyCode_New(struct _PyCodeConstructor *con)
{ {
if (intern_strings(con->names) < 0) { if (intern_code_constants(con) < 0) {
return NULL;
}
if (intern_string_constants(con->consts, NULL) < 0) {
return NULL;
}
if (intern_strings(con->localsplusnames) < 0) {
return NULL; return NULL;
} }
@ -2397,3 +2491,183 @@ _PyCode_ConstantKey(PyObject *op)
} }
return key; return key;
} }
#ifdef Py_GIL_DISABLED
static PyObject *
intern_one_constant(PyObject *op)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
_Py_hashtable_t *consts = interp->code_state.constants;
assert(!PyUnicode_CheckExact(op)); // strings are interned separately
_Py_hashtable_entry_t *entry = _Py_hashtable_get_entry(consts, op);
if (entry == NULL) {
if (_Py_hashtable_set(consts, op, op) != 0) {
return NULL;
}
#ifdef Py_REF_DEBUG
Py_ssize_t refcnt = Py_REFCNT(op);
if (refcnt != 1) {
// Adjust the reftotal to account for the fact that we only
// restore a single reference in _PyCode_Fini.
_Py_AddRefTotal(_PyThreadState_GET(), -(refcnt - 1));
}
#endif
_Py_SetImmortal(op);
return op;
}
assert(_Py_IsImmortal(entry->value));
return (PyObject *)entry->value;
}
static int
compare_constants(const void *key1, const void *key2) {
PyObject *op1 = (PyObject *)key1;
PyObject *op2 = (PyObject *)key2;
if (op1 == op2) {
return 1;
}
if (Py_TYPE(op1) != Py_TYPE(op2)) {
return 0;
}
// We compare container contents by identity because we have already
// internalized the items.
if (PyTuple_CheckExact(op1)) {
Py_ssize_t size = PyTuple_GET_SIZE(op1);
if (size != PyTuple_GET_SIZE(op2)) {
return 0;
}
for (Py_ssize_t i = 0; i < size; i++) {
if (PyTuple_GET_ITEM(op1, i) != PyTuple_GET_ITEM(op2, i)) {
return 0;
}
}
return 1;
}
else if (PyFrozenSet_CheckExact(op1)) {
if (PySet_GET_SIZE(op1) != PySet_GET_SIZE(op2)) {
return 0;
}
Py_ssize_t pos1 = 0, pos2 = 0;
PyObject *obj1, *obj2;
Py_hash_t hash1, hash2;
while ((_PySet_NextEntry(op1, &pos1, &obj1, &hash1)) &&
(_PySet_NextEntry(op2, &pos2, &obj2, &hash2)))
{
if (obj1 != obj2) {
return 0;
}
}
return 1;
}
else if (PySlice_Check(op1)) {
PySliceObject *s1 = (PySliceObject *)op1;
PySliceObject *s2 = (PySliceObject *)op2;
return (s1->start == s2->start &&
s1->stop == s2->stop &&
s1->step == s2->step);
}
else if (PyBytes_CheckExact(op1) || PyLong_CheckExact(op1)) {
return PyObject_RichCompareBool(op1, op2, Py_EQ);
}
else if (PyFloat_CheckExact(op1)) {
// Ensure that, for example, +0.0 and -0.0 are distinct
double f1 = PyFloat_AS_DOUBLE(op1);
double f2 = PyFloat_AS_DOUBLE(op2);
return memcmp(&f1, &f2, sizeof(double)) == 0;
}
else if (PyComplex_CheckExact(op1)) {
Py_complex c1 = ((PyComplexObject *)op1)->cval;
Py_complex c2 = ((PyComplexObject *)op2)->cval;
return memcmp(&c1, &c2, sizeof(Py_complex)) == 0;
}
_Py_FatalErrorFormat("unexpected type in compare_constants: %s",
Py_TYPE(op1)->tp_name);
return 0;
}
static Py_uhash_t
hash_const(const void *key)
{
PyObject *op = (PyObject *)key;
if (PySlice_Check(op)) {
PySliceObject *s = (PySliceObject *)op;
PyObject *data[3] = { s->start, s->stop, s->step };
return _Py_HashBytes(&data, sizeof(data));
}
else if (PyTuple_CheckExact(op)) {
Py_ssize_t size = PyTuple_GET_SIZE(op);
PyObject **data = _PyTuple_ITEMS(op);
return _Py_HashBytes(data, sizeof(PyObject *) * size);
}
Py_hash_t h = PyObject_Hash(op);
if (h == -1) {
// This should never happen: all the constants we support have
// infallible hash functions.
Py_FatalError("code: hash failed");
}
return (Py_uhash_t)h;
}
static int
clear_containers(_Py_hashtable_t *ht, const void *key, const void *value,
void *user_data)
{
// First clear containers to avoid recursive deallocation later on in
// destroy_key.
PyObject *op = (PyObject *)key;
if (PyTuple_CheckExact(op)) {
for (Py_ssize_t i = 0; i < PyTuple_GET_SIZE(op); i++) {
Py_CLEAR(_PyTuple_ITEMS(op)[i]);
}
}
else if (PySlice_Check(op)) {
PySliceObject *slice = (PySliceObject *)op;
Py_SETREF(slice->start, Py_None);
Py_SETREF(slice->stop, Py_None);
Py_SETREF(slice->step, Py_None);
}
else if (PyFrozenSet_CheckExact(op)) {
_PySet_ClearInternal((PySetObject *)op);
}
return 0;
}
static void
destroy_key(void *key)
{
_Py_ClearImmortal(key);
}
#endif
PyStatus
_PyCode_Init(PyInterpreterState *interp)
{
#ifdef Py_GIL_DISABLED
struct _py_code_state *state = &interp->code_state;
state->constants = _Py_hashtable_new_full(&hash_const, &compare_constants,
&destroy_key, NULL, NULL);
if (state->constants == NULL) {
return _PyStatus_NO_MEMORY();
}
#endif
return _PyStatus_OK();
}
void
_PyCode_Fini(PyInterpreterState *interp)
{
#ifdef Py_GIL_DISABLED
// Free interned constants
struct _py_code_state *state = &interp->code_state;
if (state->constants) {
_Py_hashtable_foreach(state->constants, &clear_containers, NULL);
_Py_hashtable_destroy(state->constants);
state->constants = NULL;
}
#endif
}

View File

@ -2621,6 +2621,12 @@ PySet_Clear(PyObject *set)
return 0; return 0;
} }
void
_PySet_ClearInternal(PySetObject *so)
{
(void)set_clear_internal(so);
}
int int
PySet_Contains(PyObject *anyset, PyObject *key) PySet_Contains(PyObject *anyset, PyObject *key)
{ {

View File

@ -866,8 +866,21 @@ builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename,
if (str == NULL) if (str == NULL)
goto error; goto error;
#ifdef Py_GIL_DISABLED
// gh-118527: Disable immortalization of code constants for explicit
// compile() calls to get consistent frozen outputs between the default
// and free-threaded builds.
PyInterpreterState *interp = _PyInterpreterState_GET();
int old_value = interp->gc.immortalize.enable_on_thread_created;
interp->gc.immortalize.enable_on_thread_created = 0;
#endif
result = Py_CompileStringObject(str, filename, start[compile_mode], &cf, optimize); result = Py_CompileStringObject(str, filename, start[compile_mode], &cf, optimize);
#ifdef Py_GIL_DISABLED
interp->gc.immortalize.enable_on_thread_created = old_value;
#endif
Py_XDECREF(source_copy); Py_XDECREF(source_copy);
goto finally; goto finally;

View File

@ -854,6 +854,11 @@ pycore_interp_init(PyThreadState *tstate)
return status; return status;
} }
status = _PyCode_Init(interp);
if (_PyStatus_EXCEPTION(status)) {
return status;
}
status = _PyDtoa_Init(interp); status = _PyDtoa_Init(interp);
if (_PyStatus_EXCEPTION(status)) { if (_PyStatus_EXCEPTION(status)) {
return status; return status;
@ -1827,6 +1832,8 @@ finalize_interp_types(PyInterpreterState *interp)
_PyTypes_Fini(interp); _PyTypes_Fini(interp);
_PyCode_Fini(interp);
// Call _PyUnicode_ClearInterned() before _PyDict_Fini() since it uses // Call _PyUnicode_ClearInterned() before _PyDict_Fini() since it uses
// a dict internally. // a dict internally.
_PyUnicode_ClearInterned(interp); _PyUnicode_ClearInterned(interp);

View File

@ -404,6 +404,7 @@ _Py_COMP_DIAG_POP
&(runtime)->audit_hooks.mutex, \ &(runtime)->audit_hooks.mutex, \
&(runtime)->allocators.mutex, \ &(runtime)->allocators.mutex, \
&(runtime)->_main_interpreter.types.mutex, \ &(runtime)->_main_interpreter.types.mutex, \
&(runtime)->_main_interpreter.code_state.mutex, \
} }
static void static void