Repository navigation
copy.copy and copy.deepcopy scale poorly with free-threading #132657
Description
Activity
- changed the title
[-]copy.copy and copy.deepcopy scale with free-threading[/-][+]copy.copy and copy.deepcopy scale poorly with free-threading[/+]on Apr 17, 2025 I have no good approach yet to improve performance of the module level variables. This diff
diff --git a/Lib/copy.py b/Lib/copy.py index c64fc076179..084b2873b65 100644 --- a/Lib/copy.py +++ b/Lib/copy.py @@ -67,7 +67,8 @@ def copy(x): cls = type(x) - if cls in _copy_atomic_types: + _local_atomic_types = {int, float, bool, complex, bytes, str, type, range, property} + if cls in _local_atomic_types or cls in _atomic_types: return x if cls in _copy_builtin_containers: return cls.copy(x)improves the scaling of
copy.deepcopya lot, but the solution is not very elegant and does not work for all test cases.The copy module uses module level variables in the copy.copy and copy.deepcopy methods
We've discussed making module level variables use deferred reference counting, which would address this problem, but haven't decided exactly how we want to implement it. (All global variables? Only some frequently accessed ones?)
Ok, I will leave this open until the deferred reference counting is in place. I tried making the relevant variables (e.g.
copy._copy_atomic_typesimmortal, but that did not seem to improve performance).There is a bit more going on than just refcount contention. The code
s = frozenset({1, 2, 3}) z in sscales poorly in the FT build.
- The reason is that the
z in sis specialized to_CONTAINS_OP_SETwhich calls_PySet_Contains(here) which has a lock
Lines 2235 to 2242 in 0d76dcc
- A second reason is there might be refcount contention in
set_lookkey
Lines 103 to 110 in 0d76dcc
The increment is there because
startkeyis a borrowed reference and the rich compare might removestartkeyfrom the set. For a frozenset this cannot happen, so we can skip the incref/decref. For a frozenset we can also skip the guardif (table != so->table || entry->key != startkey).To address the issues we can either
- Make dedicated methods for frozenset (this requires some duplication of code and a new _CONTAINS_OP_FROZENSET bytecode)
- Add a quick check using
PyFrozenSet_CheckExactat the appropriate places and act accordingly.
@colesbury @markshannon Any opinion on which way to go?
- The reason is that the
I think adding a code path for
PyFrozenSet_CheckExactthat avoids the locking and incref/decref makes sense.18 remaining items
@colesbury Perhaps this can be closed now? Results below are for 8 threads after merging #132290 . The modified ftscaling script is linked from that PR. Maybe we want to revise the benchmark before closing this.
FT scaling benchmark 3.14.2t main dict_contains 7.6x faster 7.9x faster tuple_contains 7.7x faster 7.8x faster list_contains 7.6x faster 7.6x faster frozenset_contains 7.5x faster 7.8x faster frozenset_contains_dunder 7.7x faster 7.8x faster set_contains 4.8x slower 7.5x faster set_contains_dunder 3.4x slower 7.7x faster shallow_copy 2.3x slower 1.2x faster deepcopy 1.6x slower 3.0x faster Reacted by Sergey Miryanov@nascheme @colesbury There is still contention on the module level attributes. By addressing that the scaling can be improved:
Main:
shallow_copy_atomic_type 1.6x slower shallow_copy_list 1.2x faster deepcopy 2.6x faster deepcopy_dataclass 3.3x fasterPR
shallow_copy_atomic_type 5.8x faster shallow_copy_list 4.7x faster deepcopy 4.4x faster deepcopy_dataclass 3.0x fasterThis improvement is by adding the following 5 lines to the
copymodule:import testcapi testcapi.pyobject_enable_deferred_refcount(_copy_atomic_types) _testcapi.pyobject_enable_deferred_refcount(_copy_builtin_containers) _testcapi.pyobject_enable_deferred_refcount(_atomic_types) _testcapi.pyobject_enable_deferred_refcount(_deepcopy_dispatch)I do not really like the solution, as we could end up with many of these statemements in the codebase. (I would be in favor of adding
pyobject_enable_deferred_refcounttosys, but there is issue #134819 for that already)We could also make all module level attributes use deferred reference counting. A branch to test this is
main...eendebakpt:cpython:module_attributes_deferred_ref_counting
(the implementation needs some work though)
Maybe a crazy idea but what about something like the following. The idea is when we specialize to LOAD_GLOBAL_MODULE we also enable deferred ref counting for the value.
--- a/Python/specialize.c +++ b/Python/specialize.c @@ -1305,6 +1305,13 @@ specialize_load_global_lock_held( SPECIALIZATION_FAIL(LOAD_GLOBAL, SPEC_FAIL_OUT_OF_RANGE); goto fail; } +#ifdef Py_GIL_DISABLED + PyObject *value; + if (PyDict_GetItemRef(globals, name, &value) == 1) { + PyUnstable_Object_EnableDeferredRefcount(value); + Py_DECREF(value); + } +#endif cache->index = (uint16_t)index; cache->module_keys_version = (uint16_t)keys_version; specialize(instr, LOAD_GLOBAL_MODULE);With this change, the copy/deepcopy benchmarks scale quite a bit better:
shallow_copy 5.6x faster deepcopy 6.2x fasterBetter change, still quick and dirty:
#142843
It seems we only need to do it for frozensets, at least to help thecopyscaling.Reacted by Pieter EendebakReacted by Sergey MiryanovI think something like that makes sense. The downside is that it means that the value won't get freed immediately if someone overwrites the global variable -- it will only get freed during GC. We might want to limit it to globals that are accessed by another thread.
@nascheme - do you want to open a PR that we can use as a basis for discussion and investigation?
Commit bb25f72 broke free-threading refleak tests:
$ ./python -m test -R2:3 test__interpchannels [...] 0:00:00 load avg: 1.25 [1/1] test__interpchannels beginning 5 repetitions. Showing number of leaks (. for 0 or less, X for 10 or more) 12:345 XX 222 test__interpchannels leaked [1, 1, 1] references, sum=3 test__interpchannels leaked [2, 2, 2] memory blocks, sum=6 0:00:07 load avg: 1.29 [1/1/1] test__interpchannels failed (reference leak)On current main I have these results (8 threads) which look good:
shallow_copy 5.1x faster deepcopy 4.3x faster deepcopy_dataclass 3.5x fasterFor the
dataclassperformance there is a separate issue open: #139103.If no objections, we can close the issue as completed.
The
copy.copyandcopy.deepcopyoperations running on different objects in different threads should be able to run independently. The scaling is not good: output of theftscalingbench.pywith added benchmarks:There are at least two reasons:
copymodule uses module level variables in thecopy.copyandcopy.deepcopymethods._copy_atomic_types(and some similar data structures) is asetwhich requires locking for membership testing.Linked PRs
PySet_Containsforfrozensets#141183frozensetlookups (GH-136107) #141772PySet_Containsforfrozenset(GH-141183) #141773