From 6140e4ca8f86047cca4c9480b70869d9208fb958 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Fri, 28 Jul 2017 11:54:29 -0400 Subject: [PATCH 1/5] bpo-31179: Make dict.copy() up to 5.5 times faster. --- Lib/test/test_dict.py | 37 +++++++++- .../2017-08-10-17-32-48.bpo-31179.XcgLYI.rst | 1 + Objects/dictobject.c | 73 +++++++++++++++++-- 3 files changed, 103 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-08-10-17-32-48.bpo-31179.XcgLYI.rst diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 4386eda3ae48fdd..55ab2bb58c27b07 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -271,11 +271,44 @@ def __new__(cls): self.assertEqual(baddict3.fromkeys({"a", "b", "c"}), res) def test_copy(self): - d = {1:1, 2:2, 3:3} - self.assertEqual(d.copy(), {1:1, 2:2, 3:3}) + d = {1: 1, 2: 2, 3: 3} + self.assertIsNot(d.copy(), d) + self.assertEqual(d.copy(), d) + self.assertEqual(d.copy(), {1: 1, 2: 2, 3: 3}) + + copy = d.copy() + d[4] = 4 + self.assertNotEqual(copy, d) + self.assertEqual({}.copy(), {}) self.assertRaises(TypeError, d.copy, None) + def test_copy_fuzz(self): + for dict_size in [10, 100, 1000, 10000, 100000]: + dict_size = random.randrange( + dict_size // 2, dict_size + dict_size // 2) + with self.subTest(dict_size=dict_size): + d = {} + for i in range(dict_size): + d[i] = i + + d2 = d.copy() + self.assertIsNot(d2, d) + self.assertEqual(d, d2) + d2['key'] = 'value' + self.assertNotEqual(d, d2) + self.assertEqual(len(d2), len(d) + 1) + + def test_copy_maintains_tracking(self): + class A: + pass + + key = A() + + for d in ({}, {'a': 1}, {key: 'val'}): + d2 = d.copy() + self.assertEqual(gc.is_tracked(d), gc.is_tracked(d2)) + def test_get(self): d = {} self.assertIs(d.get('c'), None) diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-08-10-17-32-48.bpo-31179.XcgLYI.rst b/Misc/NEWS.d/next/Core and Builtins/2017-08-10-17-32-48.bpo-31179.XcgLYI.rst new file mode 100644 index 000000000000000..12ebc8bfc7976c2 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-08-10-17-32-48.bpo-31179.XcgLYI.rst @@ -0,0 +1 @@ +Make dict.copy() up to 5.5 times faster. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index b20b85c909e3331..e0084dd4b7ebbb5 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -615,6 +615,60 @@ new_dict_with_shared_keys(PyDictKeysObject *keys) return new_dict(keys, values); } + +static PyObject * +clone_dict(PyObject *s) +{ + PyDictKeysObject *keys; + PyDictObject *new; + PyDictObject *ms; + Py_ssize_t i, n, keys_size; + PyDictKeyEntry *entry, *ep0; + PyObject *value; + + assert(PyDict_CheckExact(s)); + ms = (PyDictObject*)s; + assert(ms->ma_values == NULL); + assert(ms->ma_keys->dk_refcnt == 1); + + keys_size = _PyDict_KeysSize(ms->ma_keys); + keys = PyObject_MALLOC(keys_size); + if (keys == NULL) { + PyErr_NoMemory(); + return NULL; + } + + memcpy(keys, ms->ma_keys, keys_size); + + /* After copying key/value pairs, we need to inref all + keys and values and they are about to be co-owned by a + new dict object. */ + ep0 = DK_ENTRIES(keys); + n = keys->dk_nentries; + for (i = 0; i < n; i++) { + entry = &ep0[i]; + value = entry->me_value; + if (value != NULL) { + Py_INCREF(value); + Py_INCREF(entry->me_key); + } + } + + new = (PyDictObject *)new_dict(keys, NULL); + if (new == NULL) { + /* In case of an error, `new_dict()` will take care of + cleaning up `keys`. */ + return NULL; + } + new->ma_used = ms->ma_used; + assert(_PyDict_CheckConsistency(new)); + if (_PyObject_GC_IS_TRACKED(s)) { + /* Maintain tracking. */ + _PyObject_GC_TRACK(new); + } + return (PyObject *)new; +} + PyObject * PyDict_New(void) { @@ -2510,13 +2564,20 @@ PyDict_Copy(PyObject *o) _PyObject_GC_TRACK(split_copy); return (PyObject *)split_copy; } - copy = PyDict_New(); - if (copy == NULL) + if (PyDict_CheckExact(o) && mp->ma_values == NULL && + mp->ma_keys->dk_refcnt == 1) + { + return clone_dict(o); + } + else { + copy = PyDict_New(); + if (copy == NULL) + return NULL; + if (PyDict_Merge(copy, o, 1) == 0) + return copy; + Py_DECREF(copy); return NULL; - if (PyDict_Merge(copy, o, 1) == 0) - return copy; - Py_DECREF(copy); - return NULL; + } } Py_ssize_t From 60ed4adb1aaecac40d1ef28ad29192618b0535bf Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Fri, 11 Aug 2017 10:39:12 -0400 Subject: [PATCH 2/5] Address Inada's review comments --- Objects/dictobject.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index e0084dd4b7ebbb5..10ddd39530651f3 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -619,26 +619,27 @@ new_dict_with_shared_keys(PyDictKeysObject *keys) static PyObject * clone_dict(PyObject *s) { + assert(PyDict_CheckExact(s)); + PyDictKeysObject *keys; PyDictObject *new; - PyDictObject *ms; + PyDictObject *orig = (PyDictObject*)s; Py_ssize_t i, n, keys_size; PyDictKeyEntry *entry, *ep0; PyObject *value; - assert(PyDict_CheckExact(s)); - ms = (PyDictObject*)s; - assert(ms->ma_values == NULL); - assert(ms->ma_keys->dk_refcnt == 1); + assert(orig->ma_values == NULL); + assert(orig->ma_keys->dk_refcnt == 1); + assert(orig->ma_used == orig->ma_keys->dk_nentries); - keys_size = _PyDict_KeysSize(ms->ma_keys); + keys_size = _PyDict_KeysSize(orig->ma_keys); keys = PyObject_MALLOC(keys_size); if (keys == NULL) { PyErr_NoMemory(); return NULL; } - memcpy(keys, ms->ma_keys, keys_size); + memcpy(keys, orig->ma_keys, keys_size); /* After copying key/value pairs, we need to inref all keys and values and they are about to be co-owned by a @@ -660,9 +661,9 @@ clone_dict(PyObject *s) cleaning up `keys`. */ return NULL; } - new->ma_used = ms->ma_used; + new->ma_used = orig->ma_used; assert(_PyDict_CheckConsistency(new)); - if (_PyObject_GC_IS_TRACKED(s)) { + if (_PyObject_GC_IS_TRACKED(orig)) { /* Maintain tracking. */ _PyObject_GC_TRACK(new); } @@ -2564,20 +2565,21 @@ PyDict_Copy(PyObject *o) _PyObject_GC_TRACK(split_copy); return (PyObject *)split_copy; } + if (PyDict_CheckExact(o) && mp->ma_values == NULL && - mp->ma_keys->dk_refcnt == 1) + mp->ma_keys->dk_refcnt == 1 && + mp->ma_used == mp->ma_keys->dk_nentries) { return clone_dict(o); } - else { - copy = PyDict_New(); - if (copy == NULL) - return NULL; - if (PyDict_Merge(copy, o, 1) == 0) - return copy; - Py_DECREF(copy); + + copy = PyDict_New(); + if (copy == NULL) return NULL; - } + if (PyDict_Merge(copy, o, 1) == 0) + return copy; + Py_DECREF(copy); + return NULL; } Py_ssize_t From 03d51acc58e00b45537038516ad20737faa45911 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Sun, 21 Jan 2018 14:47:09 -0500 Subject: [PATCH 3/5] Reformat code; use fast copy when dict is slightly non-compact --- Objects/dictobject.c | 61 ++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 10ddd39530651f3..146686988dfd2dd 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -617,23 +617,14 @@ new_dict_with_shared_keys(PyDictKeysObject *keys) static PyObject * -clone_dict(PyObject *s) +clone_dict(PyDictObject *orig) { - assert(PyDict_CheckExact(s)); - - PyDictKeysObject *keys; - PyDictObject *new; - PyDictObject *orig = (PyDictObject*)s; - Py_ssize_t i, n, keys_size; - PyDictKeyEntry *entry, *ep0; - PyObject *value; - + assert(PyDict_CheckExact(orig)); assert(orig->ma_values == NULL); assert(orig->ma_keys->dk_refcnt == 1); - assert(orig->ma_used == orig->ma_keys->dk_nentries); - keys_size = _PyDict_KeysSize(orig->ma_keys); - keys = PyObject_MALLOC(keys_size); + Py_ssize_t keys_size = _PyDict_KeysSize(orig->ma_keys); + PyDictKeysObject *keys = PyObject_MALLOC(keys_size); if (keys == NULL) { PyErr_NoMemory(); return NULL; @@ -641,23 +632,23 @@ clone_dict(PyObject *s) memcpy(keys, orig->ma_keys, keys_size); - /* After copying key/value pairs, we need to inref all + /* After copying key/value pairs, we need to incref all keys and values and they are about to be co-owned by a new dict object. */ - ep0 = DK_ENTRIES(keys); - n = keys->dk_nentries; - for (i = 0; i < n; i++) { - entry = &ep0[i]; - value = entry->me_value; + PyDictKeyEntry *ep0 = DK_ENTRIES(keys); + Py_ssize_t n = keys->dk_nentries; + for (Py_ssize_t i = 0; i < n; i++) { + PyDictKeyEntry *entry = &ep0[i]; + PyObject *value = entry->me_value; if (value != NULL) { Py_INCREF(value); Py_INCREF(entry->me_key); } } - new = (PyDictObject *)new_dict(keys, NULL); + PyDictObject *new = (PyDictObject *)new_dict(keys, NULL); if (new == NULL) { - /* In case of an error, `new_dict()` will take care of + /* In case of an error, `new_dict()` takes care of cleaning up `keys`. */ return NULL; } @@ -2539,7 +2530,13 @@ PyDict_Copy(PyObject *o) PyErr_BadInternalCall(); return NULL; } + mp = (PyDictObject *)o; + if (mp->ma_used == 0) { + /* The dict is empty; just return a new dict. */ + return PyDict_New(); + } + if (_PyDict_HasSplitTable(mp)) { PyDictObject *split_copy; Py_ssize_t size = USABLE_FRACTION(DK_SIZE(mp->ma_keys)); @@ -2566,11 +2563,25 @@ PyDict_Copy(PyObject *o) return (PyObject *)split_copy; } - if (PyDict_CheckExact(o) && mp->ma_values == NULL && - mp->ma_keys->dk_refcnt == 1 && - mp->ma_used == mp->ma_keys->dk_nentries) + if (PyDict_CheckExact(mp) && + mp->ma_values == NULL && mp->ma_keys->dk_refcnt == 1 && + mp->ma_used >= mp->ma_keys->dk_nentries - 3) { - return clone_dict(o); + /* Use fast-copy if: + + (1) 'mp' is an instance of a subclassed dict; 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 3 non-used keys. + + The last condition (3) is important to guard against a pathalogical + 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. + */ + return clone_dict(mp); } copy = PyDict_New(); From bb6d328df26d5b83927c16adbefcbbca2d951797 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Mon, 22 Jan 2018 10:41:45 -0500 Subject: [PATCH 4/5] Address Victor's comments --- Objects/dictobject.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 146686988dfd2dd..edd63fa3615351f 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -617,14 +617,14 @@ new_dict_with_shared_keys(PyDictKeysObject *keys) static PyObject * -clone_dict(PyDictObject *orig) +clone_combined_dict(PyDictObject *orig) { assert(PyDict_CheckExact(orig)); assert(orig->ma_values == NULL); assert(orig->ma_keys->dk_refcnt == 1); Py_ssize_t keys_size = _PyDict_KeysSize(orig->ma_keys); - PyDictKeysObject *keys = PyObject_MALLOC(keys_size); + PyDictKeysObject *keys = PyObject_Malloc(keys_size); if (keys == NULL) { PyErr_NoMemory(); return NULL; @@ -2563,9 +2563,8 @@ PyDict_Copy(PyObject *o) return (PyObject *)split_copy; } - if (PyDict_CheckExact(mp) && - mp->ma_values == NULL && mp->ma_keys->dk_refcnt == 1 && - mp->ma_used >= mp->ma_keys->dk_nentries - 3) + if (PyDict_CheckExact(mp) && mp->ma_values == NULL && + ((double)mp->ma_used / mp->ma_keys->dk_nentries) >= 0.8) { /* Use fast-copy if: @@ -2574,14 +2573,14 @@ PyDict_Copy(PyObject *o) (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 3 non-used keys. + do fast-copy only if it has at most 20% non-used keys. The last condition (3) is important to guard against a pathalogical 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. */ - return clone_dict(mp); + return clone_combined_dict(mp); } copy = PyDict_New(); From 47413f6fe66364062b027894e6c974cd42519025 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Mon, 22 Jan 2018 11:15:40 -0500 Subject: [PATCH 5/5] Use integer arith to compute ratio; bump it to 33% --- Lib/test/test_dict.py | 13 +++++++++++++ Objects/dictobject.c | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 55ab2bb58c27b07..aa149d31eb0e22c 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -309,6 +309,19 @@ class A: d2 = d.copy() 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. + 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_get(self): d = {} self.assertIs(d.get('c'), None) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index edd63fa3615351f..259fa2556535b1c 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2564,7 +2564,7 @@ PyDict_Copy(PyObject *o) } if (PyDict_CheckExact(mp) && mp->ma_values == NULL && - ((double)mp->ma_used / mp->ma_keys->dk_nentries) >= 0.8) + (mp->ma_used >= (mp->ma_keys->dk_nentries * 2) / 3)) { /* Use fast-copy if: @@ -2573,7 +2573,7 @@ PyDict_Copy(PyObject *o) (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 20% non-used keys. + do fast-copy only if it has at most 1/3 non-used keys. The last condition (3) is important to guard against a pathalogical case when a large dict is almost emptied with multiple del/pop