gh-161524: Speed up copying sparse dictionaries

Author: localzet

Lib/test/test_capi/test_watchers.py

@@ -63,6 +63,23 @@ def test_clone(self):
             d.update(d2)
             self.assert_events(["clone"])
 
+    def test_clone_sparse_dict(self):
+        for keys in (list(range(200)), [f"key{i}" for i in range(200)]):
+            with self.subTest(key_type=type(keys[0])):
+                source = {key: key for key in keys}
+                for key in keys[:180]:
+                    del source[key]
+
+                target = {}
+                with self.watcher() as wid:
+                    self.watch(wid, target)
+                    target.update(source)
+                    self.assert_events(["clone"])
+
+                self.assertEqual(list(target.items()), list(source.items()))
+                source.clear()
+                self.assertEqual(len(target), 20)
+
     def test_no_event_if_not_watched(self):
         d = {}
         with self.watcher() as wid:

Lib/test/test_dict.py

@@ -86,6 +86,7 @@ def test_bool(self):
         self.assertIs(bool({}), False)
         self.assertIs(bool({1: 2}), True)
 
+
     def test_keys(self):
         d = {}
         self.assertEqual(set(d.keys()), set())
@@ -465,18 +466,120 @@ class A:
             self.assertEqual(gc.is_tracked(d), gc.is_tracked(d2))
 
     def test_copy_noncompact(self):
-        # Dicts don't compact themselves on del/pop operations.
-        # Copy will use a slow merging strategy that produces
-        # a compacted copy when roughly 33% of dict is a non-used
-        # keys-space (to optimize memory footprint).
-        # In this test we want to hit the slow/compacting
-        # branch of dict.copy() and make sure it works OK.
+        # Deletions leave holes in the entry table. Copying a sparse
+        # dictionary must preserve its contents and compact the table.
         d = {k: k for k in range(1000)}
         for k in range(950):
             del d[k]
         d2 = d.copy()
         self.assertEqual(d2, d)
 
+    def test_sparse_dict_copy(self):
+        for keys in ([f'key{i}' for i in range(120)], list(range(120))):
+            for remaining in (0, 1, 10, 40, 80, 119, 120):
+                with self.subTest(key_type=type(keys[0]), remaining=remaining):
+                    source = {key: object() for key in keys}
+                    for key in keys[:120 - remaining]:
+                        del source[key]
+
+                    expected = list(source.items())
+                    for result in (source.copy(), dict(source), {**source}):
+                        self.assertIsNot(result, source)
+                        self.assertEqual(list(result.items()), expected)
+                        for (_, value), (_, copied) in zip(
+                                expected, result.items()):
+                            self.assertIs(copied, value)
+                        result.clear()
+                        self.assertEqual(list(source.items()), expected)
+
+    def test_sparse_dict_copy_reinserted_keys(self):
+        for keys in ([f'key{i}' for i in range(120)], list(range(120))):
+            source = {key: object() for key in keys}
+            for key in keys[:100]:
+                del source[key]
+            for key in keys[:5]:
+                source[key] = object()
+
+            expected = list(source.items())
+            for result in (source.copy(), dict(source), {**source}):
+                self.assertEqual(list(result.items()), expected)
+
+    def test_sparse_dict_copy_does_not_rehash(self):
+        class Key:
+            def __init__(self, value):
+                self.value = value
+                self.hash_calls = 0
+
+            def __hash__(self):
+                self.hash_calls += 1
+                return self.value % 5
+
+            def __eq__(self, other):
+                return self is other
+
+        keys = [Key(i) for i in range(90)]
+        source = {key: key.value for key in keys}
+        for key in keys[:70]:
+            del source[key]
+        calls = [key.hash_calls for key in keys]
+        expected = list(source.items())
+        for result in (source.copy(), dict(source), {**source}):
+            self.assertEqual([key.hash_calls for key in keys], calls)
+            for (key, value), (copy_key, copy_value) in zip(
+                    expected, result.items()):
+                self.assertIs(copy_key, key)
+                self.assertEqual(copy_value, value)
+
+    def test_sparse_dict_copy_releases_references(self):
+        class Key:
+            pass
+
+        class Value:
+            pass
+
+        key = Key()
+        value = Value()
+        source = {i: None for i in range(150)}
+        source[key] = value
+        for i in range(150):
+            del source[i]
+
+        key_ref = weakref.ref(key)
+        value_ref = weakref.ref(value)
+        copies = [source.copy(), dict(source), {**source}]
+        del source, key, value
+
+        self.assertIsNotNone(key_ref())
+        self.assertIsNotNone(value_ref())
+        copies.clear()
+        gc.collect()
+        self.assertIsNone(key_ref())
+        self.assertIsNone(value_ref())
+
+    def test_sparse_dict_copy_threshold(self):
+        for key_type in (str, int):
+            for remaining in (1, 10, 14, 15, 16, 17, 30, 31, 32, 33, 500):
+                with self.subTest(key_type=key_type, remaining=remaining):
+                    d = {key_type(i): i for i in range(1000)}
+                    for i in range(1000 - remaining):
+                        del d[key_type(i)]
+                    expected = list(d.items())
+                    for copy in (d.copy(), dict(d), {**d}):
+                        self.assertEqual(list(copy.items()), expected)
+
+    @support.nomemtest
+    def test_sparse_dict_copy_no_memory(self):
+        import _testcapi
+
+        d = dict.fromkeys(range(128))
+        for i in range(96):
+            del d[i]
+
+        for copy in (d.copy, lambda: dict(d), lambda: {**d}):
+            with self.subTest(copy=copy):
+                with self.assertRaises(MemoryError):
+                    _testcapi.call_with_nomemory(0, 0, copy)
+
     @support.nomemtest
     def test_copy_empty_no_memory(self):
         import _testcapi
@@ -1958,6 +2061,19 @@ def test_copy(self):
         self.assertEqual(d2, frozendict(x=1, y=2))
         self.assertEqual(type(d2), frozendict)
 
+    def test_copy_from_sparse_dict(self):
+        for keys in ([f'key{i}' for i in range(1000)], list(range(1000))):
+            source = {key: key for key in keys}
+            for key in keys[:700]:
+                del source[key]
+
+            expected = list(source.items())
+            d = frozendict(source)
+            self.assertEqual(list(d.items()), expected)
+            self.assertEqual(list(d.copy().items()), expected)
+            self.assertEqual(list(dict(d).items()), expected)
+            self.assertEqual(list(FrozenDict(d).copy().items()), expected)
+
     def test_merge(self):
         # test "a | b" operator
         self.assertEqual(frozendict(x=1) | frozendict(y=2),

Misc/NEWS.d/next/Core_and_Builtins/2026-11-08-18-55-00.gh-issue-161524.KV7s2N.rst

@@ -0,0 +1,2 @@
+Speed up copying dictionaries with many deleted entries by copying live
+entries directly into a compact table instead of inserting them one by one.

Objects/dictobject.c

@@ -4296,6 +4296,70 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
     return dict_merge_from_seq2(d, seq2, override);
 }
 
+
+/* The generic merge visits only live entries, so it is faster when
+   very few entries remain in the original table. */
+static inline bool
+can_clone_sparse_dict(PyDictObject *mp)
+{
+    return mp->ma_used >= mp->ma_keys->dk_nentries / 64;
+}
+
+/* Clone only live entries from a combined table, preserving insertion order.
+   The source is locked while its keys and values are referenced. */
+static PyDictKeysObject *
+clone_compact_dict_keys(PyDictObject *orig)
+{
+    assert(PyAnyDict_Check(orig));
+    assert(Py_TYPE(orig)->tp_iter == dict_iter);
+    assert(orig->ma_values == NULL);
+    assert(orig->ma_used > 0);
+    if (!PyFrozenDict_Check(orig)) {
+        ASSERT_DICT_LOCKED(orig);
+    }
+
+    PyDictKeysObject *oldkeys = orig->ma_keys;
+    Py_ssize_t used = orig->ma_used;
+    int unicode = DK_IS_UNICODE(oldkeys);
+    PyDictKeysObject *keys = new_keys_object(estimate_log2_keysize(used),
+                                            unicode);
+    if (keys == NULL) {
+        return NULL;
+    }
+
+    Py_ssize_t n = 0;
+    if (unicode) {
+        PyDictUnicodeEntry *src = DK_UNICODE_ENTRIES(oldkeys);
+        PyDictUnicodeEntry *dst = DK_UNICODE_ENTRIES(keys);
+        for (Py_ssize_t i = 0; i < oldkeys->dk_nentries; i++) {
+            if (src[i].me_value != NULL) {
+                dst[n].me_key = Py_NewRef(src[i].me_key);
+                dst[n].me_value = Py_NewRef(src[i].me_value);
+                n++;
+            }
+        }
+        assert(n == used);
+        build_indices_unicode(keys, dst, n);
+    }
+    else {
+        PyDictKeyEntry *src = DK_ENTRIES(oldkeys);
+        PyDictKeyEntry *dst = DK_ENTRIES(keys);
+        for (Py_ssize_t i = 0; i < oldkeys->dk_nentries; i++) {
+            if (src[i].me_value != NULL) {
+                dst[n].me_key = Py_NewRef(src[i].me_key);
+                dst[n].me_hash = src[i].me_hash;
+                dst[n].me_value = Py_NewRef(src[i].me_value);
+                n++;
+            }
+        }
+        assert(n == used);
+        build_indices_generic(keys, dst, n);
+    }
+    keys->dk_nentries = n;
+    keys->dk_usable -= n;
+    return keys;
+}
+
 static int
 dict_dict_merge(PyDictObject *mp, PyDictObject *other, int override, PyObject **dupkey)
 {
@@ -4313,15 +4377,19 @@ dict_dict_merge(PyDictObject *mp, PyDictObject *other, int override, PyObject **
         override = 1;
         PyDictKeysObject *okeys = other->ma_keys;
 
-        // If other is clean, combined, and just allocated, just clone it.
-        if (mp->ma_values == NULL &&
-            other->ma_values == NULL &&
-            other->ma_used == okeys->dk_nentries &&
-            (DK_LOG_SIZE(okeys) == PyDict_LOG_MINSIZE ||
-             USABLE_FRACTION(DK_SIZE(okeys)/2) < other->ma_used)
-        ) {
+        // Clone a combined table directly, compacting deleted entries.
+        if (mp->ma_values == NULL && other->ma_values == NULL &&
+            can_clone_sparse_dict(other)) {
             _PyDict_NotifyEvent(PyDict_EVENT_CLONED, mp, (PyObject *)other, NULL);
-            PyDictKeysObject *keys = clone_combined_dict_keys(other);
+            PyDictKeysObject *keys;
+            if (other->ma_used == okeys->dk_nentries &&
+                (DK_LOG_SIZE(okeys) == PyDict_LOG_MINSIZE ||
+                 USABLE_FRACTION(DK_SIZE(okeys)/2) < other->ma_used)) {
+                keys = clone_combined_dict_keys(other);
+            }
+            else {
+                keys = clone_compact_dict_keys(other);
+            }
             if (keys == NULL)
                 return -1;
 
@@ -4618,24 +4686,17 @@ copy_lock_held_untracked(PyObject *o, int as_frozendict)
     }
 
     if (Py_TYPE(mp)->tp_iter == dict_iter &&
-            mp->ma_values == NULL &&
-            (mp->ma_used >= (mp->ma_keys->dk_nentries * 2) / 3))
+        mp->ma_values == NULL && can_clone_sparse_dict(mp))
     {
-        /* Use fast-copy if:
-
-           (1) type(mp) doesn't override tp_iter; and
-
-           (2) 'mp' is not a split-dict; and
-
-           (3) if 'mp' is non-compact ('del' operation does not resize dicts),
-               do fast-copy only if it has at most 1/3 non-used keys.
-
-           The last condition (3) is important to guard against a pathological
-           case when a large dict is almost emptied with multiple del/pop
-           operations and copied after that.  In cases like this, we defer to
-           PyDict_Merge, which produces a compacted copy.
-        */
-        PyDictKeysObject *keys = clone_combined_dict_keys(mp);
+        /* Clone compact tables as-is, but omit deleted entries from sparse
+           tables so copying an almost emptied dict stays inexpensive. */
+        PyDictKeysObject *keys;
+        if (mp->ma_used >= (mp->ma_keys->dk_nentries * 2) / 3) {
+            keys = clone_combined_dict_keys(mp);
+        }
+        else {
+            keys = clone_compact_dict_keys(mp);
+        }
         if (keys == NULL) {
             return NULL;
         }