Skip to content

Commit 909aff7

Browse files
alexmalyshevmeta-codesync[bot]
authored andcommitted
Backport gh-132657 to 3.14: defer refcount on module values
Summary: Backport of upstream gh-132657, "If we are specializing to LOAD_GLOBAL_MODULE or LOAD_ATTR_MODULE, try to enable deferred reference counting for the value, if the object is owned by a different thread", which landed in CPython 3.15.0a6. Meta Python 3.14 has neither the helper nor the calls; `third-party/python/main` already has it via the upstream import. Stacked on the gh-143469 backport because they touch the same function and only make sense together: gh-143469 lets the module attribute load specialize at all, and this one makes the value it caches cheap to load from many threads. Either alone leaves the common case flat. `maybe_enable_deferred_ref_count` is copied verbatim from upstream. The two call sites needed adapting: upstream reads the value out of `_PyDict_LookupIndexAndValue`, which does not exist in 3.14, so the value comes from the dict entry instead. That can be NULL for a deleted key at a still-valid index, which upstream's helper input cannot be, so the call is NULL-guarded at both sites. Both opcodes already deopt on a NULL entry value at run time, so behaviour is unchanged. Scope note: upstream also calls this helper from the class-attribute descriptor path. That is a different change, and 3.14's version of that code currently *fails* specialization when the descriptor lacks deferred refcount (`SPEC_FAIL_ATTR_DESCR_NOT_DEFERRED`) rather than enabling it, so converting it is not a mechanical backport. Left alone; gh-132657's own NEWS entry names only `LOAD_GLOBAL_MODULE` and `LOAD_ATTR_MODULE`. Measured on 3.14.7t against this stack, three modules differing only in the value they expose and whether they define `__getattr__`, speedup relative to one thread: module value 1t ms 8t 32t mortal, no __getattr__ 10.9 9.0x 19.2x mortal, has __getattr__ 9.1 7.8x 16.8x immortal, has __getattr__ 9.2 5.0x 16.6x untracked, has __getattr__ 11.0 0.5x 0.5x The second row is the one this diff buys: with only the gh-143469 backport below it, a mortal module value scored 0.5x. It is now 16.8x, level with an immortal one. The last row is the limit, and it is deliberate upstream behaviour rather than a gap in the backport: the helper is gated on `_PyObject_GC_IS_TRACKED`, and a plain `object()` is not tracked, so it is skipped. That is exactly the shape of torch's `dtype` / `layout` / `memory_format` / `qscheme` singletons, which is why the torch diff above this one still has to make those types GC-tracked before anything can help them. CinderX has to move with this. `UpstreamBorrow` copies `specialize_module_load_attr_lock_held` and `specialize_load_global_lock_held` out of `Python/specialize.c` and recompiles them, and borrowing is per function rather than transitive, so the new static helper they now call was not carried across and the free-threaded cinderx build failed with an implicit-declaration error. Adding a `Borrow` directive for it to `borrowed-3.14.free-threading.c.template` fixes that. Only the free-threaded template needs it; the helper and both call sites are inside `#ifdef Py_GIL_DISABLED`, so the GIL build compiles them out. `borrowed-3.14.free-threading.gen_cached.c` is regenerated to match. That file is a checked-in snapshot and is not consumed by this build config (`borrowed_library` is called with the default `cached = False`, so the genrule regenerates from the template), but it is generated output and should not be left stale. Its delta folds in the body change from the gh-143469 backport below as well, since that commit alters the same borrowed function and nothing regenerated the snapshot at that point; no build breaks in between, because nothing compiles the snapshot. Validated end to end. Adding `bundle_runtime = True` to a `python_binary` temporarily builds it against the interpreter from this commit, so the whole stack can be exercised at once. Probe results at 32 threads, stock 3.14t against this stack: case stock stack touch np.zeros 0.1x 24.7x touch torch.empty 0.0x 18.3x touch torch.long 0.1x 16.8x touch torch.float32 0.1x 16.6x touch torch.Tensor 0.1x 16.7x torch.empty(4) 0.1x 11.7x as_tensor(dtype=long) 0.3x 2.1x numpy is fixed for free by the two CPython backports, with no numpy change. Two rows are unmoved and both are expected: `touch math.pi` (0.8x) and `touch shared obj` (0.5x) are mortal objects that are not GC-tracked, which `maybe_enable_deferred_ref_count` skips by design. `as_tensor(dtype=long)` reaching only 2.1x is the honest remaining gap. That row is an actual tensor *construction*, not a lookup, so what is left is inside torch: argument parsing, allocation, `TensorImpl` setup. Refcount contention on the namespace was never going to explain all of it, and this stack does not address it. Reviewed By: itamaro Differential Revision: D116032672 fbshipit-source-id: 2109b9ad7c9122e4635e04fe8f4bc2260ac11447
1 parent 8445723 commit 909aff7

1 file changed

Lines changed: 33 additions & 0 deletions

File tree

Python/specialize.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,21 @@ static int function_kind(PyCodeObject *code);
769769
static bool function_check_args(PyObject *o, int expected_argcount, int opcode);
770770
static uint32_t function_get_version(PyObject *o, int opcode);
771771

772+
#ifdef Py_GIL_DISABLED
773+
static void
774+
maybe_enable_deferred_ref_count(PyObject *op)
775+
{
776+
if (!_Py_IsOwnedByCurrentThread(op) && _PyObject_GC_IS_TRACKED(op)) {
777+
// For module level variables that are heavily used from multiple
778+
// threads, deferred reference counting provides good scaling
779+
// benefits. The downside is that the object will only be deallocated
780+
// by a GC run.
781+
PyUnstable_Object_EnableDeferredRefcount(op);
782+
}
783+
}
784+
#endif
785+
786+
772787
static int
773788
specialize_module_load_attr_lock_held(PyDictObject *dict, _Py_CODEUNIT *instr, PyObject *name)
774789
{
@@ -807,6 +822,16 @@ specialize_module_load_attr_lock_held(PyDictObject *dict, _Py_CODEUNIT *instr, P
807822
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OUT_OF_VERSIONS);
808823
return -1;
809824
}
825+
#ifdef Py_GIL_DISABLED
826+
// Upstream reads the value out of `_PyDict_LookupIndexAndValue`, which
827+
// 3.14 does not have, so take it from the entry instead. That can be NULL
828+
// for a deleted key at a still-valid index, which `_LOAD_ATTR_MODULE`
829+
// handles at run time via `DEOPT_IF(attr_o == NULL)`.
830+
PyObject *value = DK_UNICODE_ENTRIES(dict->ma_keys)[index].me_value;
831+
if (value != NULL) {
832+
maybe_enable_deferred_ref_count(value);
833+
}
834+
#endif
810835
write_u32(cache->version, keys_version);
811836
cache->index = (uint16_t)index;
812837
specialize(instr, LOAD_ATTR_MODULE);
@@ -1730,6 +1755,14 @@ specialize_load_global_lock_held(
17301755
SPECIALIZATION_FAIL(LOAD_GLOBAL, SPEC_FAIL_OUT_OF_RANGE);
17311756
goto fail;
17321757
}
1758+
#ifdef Py_GIL_DISABLED
1759+
// Same as above: no `_PyDict_LookupIndexAndValue` in 3.14, and
1760+
// `_LOAD_GLOBAL_MODULE` deopts on a NULL entry value itself.
1761+
PyObject *value = DK_UNICODE_ENTRIES(globals_keys)[index].me_value;
1762+
if (value != NULL) {
1763+
maybe_enable_deferred_ref_count(value);
1764+
}
1765+
#endif
17331766
cache->index = (uint16_t)index;
17341767
cache->module_keys_version = (uint16_t)keys_version;
17351768
specialize(instr, LOAD_GLOBAL_MODULE);

0 commit comments

Comments
 (0)