gh-155176: Don't track frozendicts whose contents can never be tracked by the GC - #155178
gh-155176: Don't track frozendicts whose contents can never be tracked by the GC#155178aisk wants to merge 2 commits into
Conversation
…tracked by the GC
| self.assertFalse(gc.is_tracked(frozendict())) | ||
| self.assertFalse(gc.is_tracked(frozendict({1: 2}))) | ||
| self.assertFalse(gc.is_tracked(frozendict.fromkeys('ab', 1))) | ||
| self.assertFalse(gc.is_tracked(frozendict({1: 2}) | {3: 4})) |
There was a problem hiding this comment.
please, test nested frozendicts :)
There was a problem hiding this comment.
I found that the current implementation doesn't support nested frozendicts / frozensets. The outer frozendict is conservatively tracked even if the nested one is untracked. This is because the current implementation of _PyObject_GC_MAY_BE_TRACKED only has special support for tuples:
cpython/Include/internal/pycore_gc.h
Lines 85 to 93 in dd5a285
frozenset has the same limitation. We can update _PyObject_GC_MAY_BE_TRACKED to support frozenset and frozendict too. But I think this could be out of the current PR's scope, and adding a test to check that nested frozendicts are not supported and then updating it in another PR seems meaningless. If you think it's OK, I'll create another issue to track the process.
|
I find |
I agree. But since most of the complexity comes from the current implementation and this change only adds a few branches to it, I guess this could be done in another issue, to reduce the review work for this change? If so, I'm happy to create a new issue to track it. |
4 participants
Uh oh!
There was an error while loading. Please reload this page.