Skip to content

gh-130851: Only intern constants of types generated by the compiler#130901

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
colesbury merged 2 commits into from
Mar 7, 2025
Merged

gh-130851: Only intern constants of types generated by the compiler #130901

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c3dd1c
gh-130851: Only intern constants of types generated by compiler
colesbury Mar 5, 2025
f6670ce
Use _Py_IsImmortal instead of recursive call in should_immortalize_co…
colesbury Mar 7, 2025
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
43 changes: 41 additions & 2 deletions Objects/codeobject.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,44 @@ should_intern_string(PyObject *o)

#ifdef Py_GIL_DISABLED
static PyObject *intern_one_constant(PyObject *op);

// gh-130851: In the free threading build, we intern and immortalize most
// constants, except code objects. However, users can generate code objects
// with arbitrary co_consts. We don't want to immortalize or intern unexpected
// constants or tuples/sets containing unexpected constants.
static int
should_immortalize_constant(PyObject *v)
{
// Only immortalize containers if we've already immortalized all their
// elements.
if (PyTuple_CheckExact(v)) {
for (Py_ssize_t i = PyTuple_GET_SIZE(v); --i >= 0; ) {
if (!_Py_IsImmortal(PyTuple_GET_ITEM(v, i))) {
return 0;
}
}
return 1;
}
else if (PyFrozenSet_CheckExact(v)) {
PyObject *item;
Py_hash_t hash;
Py_ssize_t pos = 0;
while (_PySet_NextEntry(v, &pos, &item, &hash)) {
if (!_Py_IsImmortal(item)) {
return 0;
}
}
return 1;
}
else if (PySlice_Check(v)) {
PySliceObject *slice = (PySliceObject *)v;
return (_Py_IsImmortal(slice->start) &&
_Py_IsImmortal(slice->stop) &&
_Py_IsImmortal(slice->step));
}
return (PyLong_CheckExact(v) || PyFloat_CheckExact(v) ||
PyComplex_Check(v) || PyBytes_CheckExact(v));
}
#endif

static int
Expand DownExpand Up@@ -241,8 +279,9 @@ intern_constants(PyObject *tuple, int *modified)

// Intern non-string constants in the free-threaded build
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET();
if (!_Py_IsImmortal(v) && !PyCode_Check(v) &&
!PyUnicode_CheckExact(v) && !tstate->suppress_co_const_immortalization)
if (!_Py_IsImmortal(v) && !PyUnicode_CheckExact(v) &&

@Yhg1sYhg1sMar 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the explicit check for unicode necessary, given that should_immortalize_constant also checks that? (For performance, perhaps?)

@colesburycolesburyMar 7, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The checks were in both places because I wrote should_immortalize_constant recursively for tuples, sets, and slices.

I've changed should_immortalize_constant so that it's no longer recursive and instead interns and immortalizes tuples, set, and slices if we've already immortalized their elements. That should still end up interning and immortalizing the same objects, but avoids the seemingly redundant checks.

should_immortalize_constant(v) &&
!tstate->suppress_co_const_immortalization)
{
PyObject *interned = intern_one_constant(v);
if (interned == NULL) {
Expand Down