Skip to content

Commit

Permalink
Don't untrack dicts
Browse files Browse the repository at this point in the history
  • Loading branch information
@markshannon
markshannon committed Nov 6, 2024
1 parent 9e2d93c commit 3c18fc8
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 218 deletions.
109 changes: 0 additions & 109 deletions Lib/test/test_dict.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -880,115 +880,6 @@ class C(object):
gc.collect()
self.assertIs(ref(), None, "Cycle was not collected")

def _not_tracked(self, t):
# Nested containers can take several collections to untrack
gc.collect()
gc.collect()
self.assertFalse(gc.is_tracked(t), t)

def _tracked(self, t):
self.assertTrue(gc.is_tracked(t), t)
gc.collect()
gc.collect()
self.assertTrue(gc.is_tracked(t), t)

def test_string_keys_can_track_values(self):
# Test that this doesn't leak.
for i in range(10):
d = {}
for j in range(10):
d[str(j)] = j
d["foo"] = d

@support.cpython_only
def test_track_literals(self):
# Test GC-optimization of dict literals
x, y, z, w = 1.5, "a", (1, None), []

self._not_tracked({})
self._not_tracked({x:(), y:x, z:1})
self._not_tracked({1: "a", "b": 2})
self._not_tracked({1: 2, (None, True, False, ()): int})
self._not_tracked({1: object()})

# Dicts with mutable elements are always tracked, even if those
# elements are not tracked right now.
self._tracked({1: []})
self._tracked({1: ([],)})
self._tracked({1: {}})
self._tracked({1: set()})

@support.cpython_only
def test_track_dynamic(self):
# Test GC-optimization of dynamically-created dicts
class MyObject(object):
pass
x, y, z, w, o = 1.5, "a", (1, object()), [], MyObject()

d = dict()
self._not_tracked(d)
d[1] = "a"
self._not_tracked(d)
d[y] = 2
self._not_tracked(d)
d[z] = 3
self._not_tracked(d)
self._not_tracked(d.copy())
d[4] = w
self._tracked(d)
self._tracked(d.copy())
d[4] = None
self._not_tracked(d)
self._not_tracked(d.copy())

# dd isn't tracked right now, but it may mutate and therefore d
# which contains it must be tracked.
d = dict()
dd = dict()
d[1] = dd
self._not_tracked(dd)
self._tracked(d)
dd[1] = d
self._tracked(dd)

d = dict.fromkeys([x, y, z])
self._not_tracked(d)
dd = dict()
dd.update(d)
self._not_tracked(dd)
d = dict.fromkeys([x, y, z, o])
self._tracked(d)
dd = dict()
dd.update(d)
self._tracked(dd)

d = dict(x=x, y=y, z=z)
self._not_tracked(d)
d = dict(x=x, y=y, z=z, w=w)
self._tracked(d)
d = dict()
d.update(x=x, y=y, z=z)
self._not_tracked(d)
d.update(w=w)
self._tracked(d)

d = dict([(x, y), (z, 1)])
self._not_tracked(d)
d = dict([(x, y), (z, w)])
self._tracked(d)
d = dict()
d.update([(x, y), (z, 1)])
self._not_tracked(d)
d.update([(x, y), (z, w)])
self._tracked(d)

@support.cpython_only
def test_track_subtypes(self):
# Dict subtypes are always tracked
class MyDict(dict):
pass
self._tracked(MyDict())

def make_shared_key_dict(self, n):
class C:
pass
Expand Down
92 changes: 4 additions & 88 deletions Objects/dictobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,7 @@ new_dict(PyInterpreterState *interp,
mp->ma_used = used;
mp->_ma_watcher_tag = 0;
ASSERT_CONSISTENT(mp);
_PyObject_GC_TRACK(mp);
return (PyObject *)mp;
}

Expand Down Expand Up @@ -1578,64 +1579,6 @@ _PyDict_HasOnlyStringKeys(PyObject *dict)
return 1;
}

#define MAINTAIN_TRACKING(mp, key, value) \
do { \
if (!_PyObject_GC_IS_TRACKED(mp)) { \
if (_PyObject_GC_MAY_BE_TRACKED(key) || \
_PyObject_GC_MAY_BE_TRACKED(value)) { \
_PyObject_GC_TRACK(mp); \
} \
} \
} while(0)

void
_PyDict_MaybeUntrack(PyObject *op)
{
PyDictObject *mp;
PyObject *value;
Py_ssize_t i, numentries;

ASSERT_WORLD_STOPPED_OR_DICT_LOCKED(op);

if (!PyDict_CheckExact(op) || !_PyObject_GC_IS_TRACKED(op))
return;

mp = (PyDictObject *) op;
ASSERT_CONSISTENT(mp);
numentries = mp->ma_keys->dk_nentries;
if (_PyDict_HasSplitTable(mp)) {
for (i = 0; i < numentries; i++) {
if ((value = mp->ma_values->values[i]) == NULL)
continue;
if (_PyObject_GC_MAY_BE_TRACKED(value)) {
return;
}
}
}
else {
if (DK_IS_UNICODE(mp->ma_keys)) {
PyDictUnicodeEntry *ep0 = DK_UNICODE_ENTRIES(mp->ma_keys);
for (i = 0; i < numentries; i++) {
if ((value = ep0[i].me_value) == NULL)
continue;
if (_PyObject_GC_MAY_BE_TRACKED(value))
return;
}
}
else {
PyDictKeyEntry *ep0 = DK_ENTRIES(mp->ma_keys);
for (i = 0; i < numentries; i++) {
if ((value = ep0[i].me_value) == NULL)
continue;
if (_PyObject_GC_MAY_BE_TRACKED(value) ||
_PyObject_GC_MAY_BE_TRACKED(ep0[i].me_key))
return;
}
}
}
_PyObject_GC_UNTRACK(op);
}

void
_PyDict_EnablePerThreadRefcounting(PyObject *op)
{
Expand Down Expand Up @@ -1761,7 +1704,6 @@ insert_split_value(PyInterpreterState *interp, PyDictObject *mp, PyObject *key,
{
assert(PyUnicode_CheckExact(key));
ASSERT_DICT_LOCKED(mp);
MAINTAIN_TRACKING(mp, key, value);
PyObject *old_value = mp->ma_values->values[ix];
if (old_value == NULL) {
_PyDict_NotifyEvent(interp, PyDict_EVENT_ADDED, mp, key, value);
Expand Down Expand Up @@ -1818,8 +1760,6 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
if (ix == DKIX_ERROR)
goto Fail;

MAINTAIN_TRACKING(mp, key, value);

if (ix == DKIX_EMPTY) {
assert(!_PyDict_HasSplitTable(mp));
/* Insert into new slot. */
Expand Down Expand Up @@ -1878,8 +1818,6 @@ insert_to_emptydict(PyInterpreterState *interp, PyDictObject *mp,
/* We don't decref Py_EMPTY_KEYS here because it is immortal. */
assert(mp->ma_values == NULL);

MAINTAIN_TRACKING(mp, key, value);

size_t hashpos = (size_t)hash & (PyDict_MINSIZE-1);
dictkeys_set_index(newkeys, hashpos, 0);
if (unicode) {
Expand Down Expand Up @@ -3770,11 +3708,6 @@ dict_dict_merge(PyInterpreterState *interp, PyDictObject *mp, PyDictObject *othe
STORE_USED(mp, other->ma_used);
ASSERT_CONSISTENT(mp);

if (_PyObject_GC_IS_TRACKED(other) && !_PyObject_GC_IS_TRACKED(mp)) {
/* Maintain tracking. */
_PyObject_GC_TRACK(mp);
}

return 0;
}
}
Expand Down Expand Up @@ -4024,8 +3957,7 @@ copy_lock_held(PyObject *o)
split_copy->ma_used = mp->ma_used;
split_copy->_ma_watcher_tag = 0;
dictkeys_incref(mp->ma_keys);
if (_PyObject_GC_IS_TRACKED(mp))
_PyObject_GC_TRACK(split_copy);
_PyObject_GC_TRACK(split_copy);
return (PyObject *)split_copy;
}

Expand Down Expand Up @@ -4060,10 +3992,6 @@ copy_lock_held(PyObject *o)

new->ma_used = mp->ma_used;
ASSERT_CONSISTENT(new);
if (_PyObject_GC_IS_TRACKED(mp)) {
/* Maintain tracking. */
_PyObject_GC_TRACK(new);
}

return (PyObject *)new;
}
Expand Down Expand Up @@ -4350,8 +4278,6 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
*result = NULL;
}
}

MAINTAIN_TRACKING(mp, key, value);
STORE_USED(mp, mp->ma_used + 1);
assert(mp->ma_keys->dk_usable >= 0);
ASSERT_CONSISTENT(mp);
Expand Down Expand Up @@ -4801,15 +4727,8 @@ dict_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
d->ma_values = NULL;
ASSERT_CONSISTENT(d);

if (type != &PyDict_Type) {
// Don't track if a subclass tp_alloc is PyType_GenericAlloc()
if (!_PyObject_GC_IS_TRACKED(d)) {
_PyObject_GC_TRACK(d);
}
}
else {
// _PyType_AllocNoTrack() does not track the created object
assert(!_PyObject_GC_IS_TRACKED(d));
if (!_PyObject_GC_IS_TRACKED(d)) {
_PyObject_GC_TRACK(d);
}
return self;
}
Expand Down Expand Up @@ -6755,9 +6674,6 @@ make_dict_from_instance_attributes(PyInterpreterState *interp,
}
}
PyDictObject *res = (PyDictObject *)new_dict(interp, keys, values, used, 0);
if (track && res) {
_PyObject_GC_TRACK(res);
}
return res;
}

Expand Down
2 changes: 0 additions & 2 deletions Objects/moduleobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,6 @@ static void
track_module(PyModuleObject *m)
{
_PyDict_EnablePerThreadRefcounting(m->md_dict);
PyObject_GC_Track(m->md_dict);

_PyObject_SetDeferredRefcount((PyObject *)m);
PyObject_GC_Track(m);
}
Expand Down
4 changes: 0 additions & 4 deletions Python/bytecodes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2316,10 +2316,6 @@ dummy_func(
DEOPT_IF(ep->me_key != name);
PyObject *old_value = ep->me_value;
DEOPT_IF(old_value == NULL);
/* Ensure dict is GC tracked if it needs to be */
if (!_PyObject_GC_IS_TRACKED(dict) && _PyObject_GC_MAY_BE_TRACKED(PyStackRef_AsPyObjectBorrow(value))) {
_PyObject_GC_TRACK(dict);
}
_PyDict_NotifyEvent(tstate->interp, PyDict_EVENT_MODIFIED, dict, name, PyStackRef_AsPyObjectBorrow(value));
ep->me_value = PyStackRef_AsPyObjectSteal(value);
// old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
Expand Down
4 changes: 0 additions & 4 deletions Python/executor_cases.c.h
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 4 additions & 7 deletions Python/gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ move_unreachable(PyGC_Head *young, PyGC_Head *unreachable)
}

static void
untrack_tuples_and_dicts(PyGC_Head *head)
untrack_tuples(PyGC_Head *head)
{
PyGC_Head *next, *gc = GC_NEXT(head);
while (gc != head) {
Expand All @@ -735,9 +735,6 @@ untrack_tuples_and_dicts(PyGC_Head *head)
if (PyTuple_CheckExact(op)) {
_PyTuple_MaybeUntrack(op);
}
else if (PyDict_CheckExact(op)) {
_PyDict_MaybeUntrack(op);
}
gc = next;
}
}
Expand Down Expand Up @@ -1295,7 +1292,7 @@ gc_collect_young(PyThreadState *tstate,
GCState *gcstate = &tstate->interp->gc;
PyGC_Head *young = &gcstate->young.head;
PyGC_Head *visited = &gcstate->old[gcstate->visited_space].head;
untrack_tuples_and_dicts(&gcstate->young.head);
untrack_tuples(&gcstate->young.head);
GC_STAT_ADD(0, collections, 1);
#ifdef Py_STATS
{
Expand Down Expand Up @@ -1576,7 +1573,7 @@ gc_collect_increment(PyThreadState *tstate, struct gc_collection_stats *stats)
GC_STAT_ADD(1, objects_transitively_reachable, objects_marked);
gcstate->work_to_do -= objects_marked;
gc_list_set_space(&gcstate->young.head, gcstate->visited_space);
untrack_tuples_and_dicts(&gcstate->young.head);
untrack_tuples(&gcstate->young.head);
gc_list_merge(&gcstate->young.head, &increment);
gcstate->young.count = 0;
gc_list_validate_space(&increment, gcstate->visited_space);
Expand Down Expand Up @@ -1621,7 +1618,7 @@ gc_collect_full(PyThreadState *tstate,
PyGC_Head *young = &gcstate->young.head;
PyGC_Head *pending = &gcstate->old[gcstate->visited_space^1].head;
PyGC_Head *visited = &gcstate->old[gcstate->visited_space].head;
untrack_tuples_and_dicts(&gcstate->young.head);
untrack_tuples(&gcstate->young.head);
/* merge all generations into pending */
gc_list_validate_space(young, 1-gcstate->visited_space);
gc_list_merge(young, pending);
Expand Down
4 changes: 0 additions & 4 deletions Python/generated_cases.c.h
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3c18fc8

Please sign in to comment.