Skip to content

Commit 6009309

Browse files
authored
gh-146041: Avoid lock in sys.intern() for already interned strings (gh-146072)
Fix free-threading scaling bottleneck in sys.intern and `PyObject_SetAttr` by avoiding the interpreter-wide lock when the string is already interned and immortalized.
1 parent 535b09c commit 6009309

File tree

5 files changed

+38
-19
lines changed

5 files changed

+38
-19
lines changed

InternalDocs/string_interning.md

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,9 @@ The key and value of each entry in this dict reference the same object.
5252

5353
## Immortality and reference counting
5454

55-
Invariant: Every immortal string is interned.
55+
In the GIL-enabled build interned strings may be mortal or immortal. In the
56+
free-threaded build, interned strings are always immortal.
5657

57-
In practice, this means that you must not use `_Py_SetImmortal` on
58-
a string. (If you know it's already immortal, don't immortalize it;
59-
if you know it's not interned you might be immortalizing a redundant copy;
60-
if it's interned and mortal it needs extra processing in
61-
`_PyUnicode_InternImmortal`.)
62-
63-
The converse is not true: interned strings can be mortal.
6458
For mortal interned strings:
6559

6660
- the 2 references from the interned dict (key & value) are excluded from
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix free-threading scaling bottleneck in :func:`sys.intern` and
2+
:c:func:`PyObject_SetAttr` by avoiding the interpreter-wide lock when the string
3+
is already interned and immortalized.

Objects/object.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,7 +2032,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
20322032
}
20332033

20342034
Py_INCREF(name);
2035-
Py_INCREF(tp);
2035+
_Py_INCREF_TYPE(tp);
20362036

20372037
PyThreadState *tstate = _PyThreadState_GET();
20382038
_PyCStackRef cref;
@@ -2107,7 +2107,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
21072107
}
21082108
done:
21092109
_PyThreadState_PopCStackRef(tstate, &cref);
2110-
Py_DECREF(tp);
2110+
_Py_DECREF_TYPE(tp);
21112111
Py_DECREF(name);
21122112
return res;
21132113
}
@@ -2761,13 +2761,6 @@ _Py_NewReferenceNoTotal(PyObject *op)
27612761
void
27622762
_Py_SetImmortalUntracked(PyObject *op)
27632763
{
2764-
#ifdef Py_DEBUG
2765-
// For strings, use _PyUnicode_InternImmortal instead.
2766-
if (PyUnicode_CheckExact(op)) {
2767-
assert(PyUnicode_CHECK_INTERNED(op) == SSTATE_INTERNED_IMMORTAL
2768-
|| PyUnicode_CHECK_INTERNED(op) == SSTATE_INTERNED_IMMORTAL_STATIC);
2769-
}
2770-
#endif
27712764
// Check if already immortal to avoid degrading from static immortal to plain immortal
27722765
if (_Py_IsImmortal(op)) {
27732766
return;

Objects/unicodeobject.c

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14187,8 +14187,11 @@ immortalize_interned(PyObject *s)
1418714187
_Py_DecRefTotal(_PyThreadState_GET());
1418814188
}
1418914189
#endif
14190-
FT_ATOMIC_STORE_UINT8_RELAXED(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_IMMORTAL);
1419114190
_Py_SetImmortal(s);
14191+
// The switch to SSTATE_INTERNED_IMMORTAL must be the last thing done here
14192+
// to synchronize with the check in intern_common() that avoids locking if
14193+
// the string is already immortal.
14194+
FT_ATOMIC_STORE_UINT8(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_IMMORTAL);
1419214195
}
1419314196

1419414197
static /* non-null */ PyObject*
@@ -14270,6 +14273,23 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */,
1427014273
assert(interned != NULL);
1427114274
#ifdef Py_GIL_DISABLED
1427214275
# define INTERN_MUTEX &_Py_INTERP_CACHED_OBJECT(interp, interned_mutex)
14276+
// Lock-free fast path: check if there's already an interned copy that
14277+
// is in its final immortal state.
14278+
PyObject *r;
14279+
int res = PyDict_GetItemRef(interned, s, &r);
14280+
if (res < 0) {
14281+
PyErr_Clear();
14282+
return s;
14283+
}
14284+
if (res > 0) {
14285+
unsigned int state = _Py_atomic_load_uint8(&_PyUnicode_STATE(r).interned);
14286+
if (state == SSTATE_INTERNED_IMMORTAL) {
14287+
Py_DECREF(s);
14288+
return r;
14289+
}
14290+
// Not yet fully interned; fall through to the locking path.
14291+
Py_DECREF(r);
14292+
}
1427314293
#endif
1427414294
FT_MUTEX_LOCK(INTERN_MUTEX);
1427514295
PyObject *t;
@@ -14307,7 +14327,7 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */,
1430714327
Py_DECREF(s);
1430814328
Py_DECREF(s);
1430914329
}
14310-
FT_ATOMIC_STORE_UINT8_RELAXED(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_MORTAL);
14330+
FT_ATOMIC_STORE_UINT8(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_MORTAL);
1431114331

1431214332
/* INTERNED_MORTAL -> INTERNED_IMMORTAL (if needed) */
1431314333

Tools/ftscalingbench/ftscalingbench.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,15 @@ def deepcopy():
285285
for i in range(40 * WORK_SCALE):
286286
copy.deepcopy(x)
287287

288+
@register_benchmark
289+
def setattr_non_interned():
290+
prefix = "prefix"
291+
obj = MyObject()
292+
for _ in range(1000 * WORK_SCALE):
293+
setattr(obj, f"{prefix}_a", None)
294+
setattr(obj, f"{prefix}_b", None)
295+
setattr(obj, f"{prefix}_c", None)
296+
288297

289298
def bench_one_thread(func):
290299
t0 = time.perf_counter_ns()

0 commit comments

Comments
 (0)