Skip to content

Commit f2b5131

Browse files
authored
[3.14] gh-146041: Avoid lock in sys.intern() for already interned strings (gh-146072) (#146390)
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. (cherry picked from commit 6009309)
1 parent 58c5eda commit f2b5131

File tree

5 files changed

+40
-20
lines changed

5 files changed

+40
-20
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
@@ -1954,7 +1954,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
19541954
}
19551955

19561956
Py_INCREF(name);
1957-
Py_INCREF(tp);
1957+
_Py_INCREF_TYPE(tp);
19581958

19591959
PyThreadState *tstate = _PyThreadState_GET();
19601960
_PyCStackRef cref;
@@ -2029,7 +2029,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
20292029
}
20302030
done:
20312031
_PyThreadState_PopCStackRef(tstate, &cref);
2032-
Py_DECREF(tp);
2032+
_Py_DECREF_TYPE(tp);
20332033
Py_DECREF(name);
20342034
return res;
20352035
}
@@ -2678,13 +2678,6 @@ _Py_NewReferenceNoTotal(PyObject *op)
26782678
void
26792679
_Py_SetImmortalUntracked(PyObject *op)
26802680
{
2681-
#ifdef Py_DEBUG
2682-
// For strings, use _PyUnicode_InternImmortal instead.
2683-
if (PyUnicode_CheckExact(op)) {
2684-
assert(PyUnicode_CHECK_INTERNED(op) == SSTATE_INTERNED_IMMORTAL
2685-
|| PyUnicode_CHECK_INTERNED(op) == SSTATE_INTERNED_IMMORTAL_STATIC);
2686-
}
2687-
#endif
26882681
// Check if already immortal to avoid degrading from static immortal to plain immortal
26892682
if (_Py_IsImmortal(op)) {
26902683
return;

Objects/unicodeobject.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15954,8 +15954,11 @@ immortalize_interned(PyObject *s)
1595415954
_Py_DecRefTotal(_PyThreadState_GET());
1595515955
}
1595615956
#endif
15957-
FT_ATOMIC_STORE_UINT8_RELAXED(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_IMMORTAL);
1595815957
_Py_SetImmortal(s);
15958+
// The switch to SSTATE_INTERNED_IMMORTAL must be the last thing done here
15959+
// to synchronize with the check in intern_common() that avoids locking if
15960+
// the string is already immortal.
15961+
FT_ATOMIC_STORE_UINT8(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_IMMORTAL);
1595915962
}
1596015963

1596115964
static /* non-null */ PyObject*
@@ -16035,7 +16038,25 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */,
1603516038
/* Do a setdefault on the per-interpreter cache. */
1603616039
PyObject *interned = get_interned_dict(interp);
1603716040
assert(interned != NULL);
16038-
16041+
#ifdef Py_GIL_DISABLED
16042+
// Lock-free fast path: check if there's already an interned copy that
16043+
// is in its final immortal state.
16044+
PyObject *r;
16045+
int res = PyDict_GetItemRef(interned, s, &r);
16046+
if (res < 0) {
16047+
PyErr_Clear();
16048+
return s;
16049+
}
16050+
if (res > 0) {
16051+
unsigned int state = _Py_atomic_load_uint8(&_PyUnicode_STATE(r).interned);
16052+
if (state == SSTATE_INTERNED_IMMORTAL) {
16053+
Py_DECREF(s);
16054+
return r;
16055+
}
16056+
// Not yet fully interned; fall through to the locking path.
16057+
Py_DECREF(r);
16058+
}
16059+
#endif
1603916060
LOCK_INTERNED(interp);
1604016061
PyObject *t;
1604116062
{
@@ -16072,7 +16093,7 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */,
1607216093
Py_DECREF(s);
1607316094
Py_DECREF(s);
1607416095
}
16075-
FT_ATOMIC_STORE_UINT8_RELAXED(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_MORTAL);
16096+
FT_ATOMIC_STORE_UINT8(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_MORTAL);
1607616097

1607716098
/* INTERNED_MORTAL -> INTERNED_IMMORTAL (if needed) */
1607816099

Tools/ftscalingbench/ftscalingbench.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,15 @@ def staticmethod_call():
239239
for _ in range(1000 * WORK_SCALE):
240240
obj.my_staticmethod()
241241

242+
@register_benchmark
243+
def setattr_non_interned():
244+
prefix = "prefix"
245+
obj = MyObject()
246+
for _ in range(1000 * WORK_SCALE):
247+
setattr(obj, f"{prefix}_a", None)
248+
setattr(obj, f"{prefix}_b", None)
249+
setattr(obj, f"{prefix}_c", None)
250+
242251

243252
def bench_one_thread(func):
244253
t0 = time.perf_counter_ns()

0 commit comments

Comments
 (0)