[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Change by Pablo Galindo Salgado : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___ ___

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Pablo Galindo Salgado added the comment: New changeset 7c797982383ebfd9cca39c480dcf6132979dd06f by Brandt Bucher in branch '3.8': bpo-42536: GC track recycled tuples (GH-23623) (GH-23652) https://github.com/python/cpython/commit/7c797982383ebfd9cca39c480dcf6132979dd06f -- _

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Pablo Galindo Salgado added the comment: New changeset 60463e8e4f79e5b5e96dc43fb83ded373b489e33 by Brandt Bucher in branch '3.9': bpo-42536: GC track recycled tuples (GH-23623) (GH-23651) https://github.com/python/cpython/commit/60463e8e4f79e5b5e96dc43fb83ded373b489e33 -- _

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Change by Brandt Bucher : -- pull_requests: +22520 pull_request: https://github.com/python/cpython/pull/23652 ___ Python tracker ___ ___

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Change by Brandt Bucher : -- pull_requests: +22519 pull_request: https://github.com/python/cpython/pull/23651 ___ Python tracker ___ ___

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: New changeset 226a012d1cd61f42ecd3056c554922f359a1a35d by Brandt Bucher in branch 'master': bpo-42536: GC track recycled tuples (GH-23623) https://github.com/python/cpython/commit/226a012d1cd61f42ecd3056c554922f359a1a35d -- _

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: > Would it be possible to push a quick & dirty workaround for test_itertools? > I'm getting more and more buildbot Refleaks failures. Oh, I looked at PR 23623 and it got many approvals. I'm fine with waiting for a few days until PR 23623 is merged.

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: Would it be possible to push a quick & dirty workaround for test_itertools? I'm getting more and more buildbot Refleaks failures. For example, always skip TestGC.test_zip() for now. This issue seems quite complex and I would prefer to have time to think abou

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: Adding: - collections.OrderedDict.items -- ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: Yeah, I'm fine with that. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Pablo Galindo Salgado added the comment: > I'll merge it and move on to similar PRs for the other affected stuff: Not sure if is more ergonomic, but I would prefer to fix this in one PR so the buildbots don't test the partial fixes. Also is easier to revert if something goes wrong with the

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: Thanks for that detailed explanation, Pablo. If nobody objects to the current zip fix, I'll merge it and move on to similar PRs for the other affected stuff: - dict.items - enumerate - functools.reduce - itertools.product - itertools.combinations - itertools.

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Pablo Galindo Salgado added the comment: I forgot to mention the conclusion: my conclusion is that although the GC is resilient to these shenanigans (it was also extensive validation along with the algorithm), I still prefer to track it on the next() call because is technically the right thi

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Pablo Galindo Salgado added the comment: > Or, if z's refcount drops to zero and it's cleaned up, its traverse function > may *never* be called, which leaves the untracked r -> o -> r cycle. This is a real problem, indeed. We would need to add the tracking to the tp_dealloc of the zip object

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: > I added some comments in the PR regarding the possibility of forcing the > tracking on the visiting function Thinking about this more, I'm a bit hesitant to put the re-tracking code in the traverse function (mostly stemming from my lack of knowledge about o

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Pablo Galindo Salgado added the comment: > The GC visit may happen when the refcount of the result tuple is > 1, but it > may drop back to 1 before __next__ called again Good point. Also, is it possible that a more complex cycle ends having more than one reference to that result, so indeed t

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: > I added some comments in the PR regarding the possibility of forcing the > tracking on the visiting function to redirect the cost to gc passes instead > of when calling next()... Yep, I agree that this approach is better. > ...but there is another option:

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Pablo Galindo Salgado added the comment: > not untracking tuples that have refcount == 1. Actually, this may not be a good idea as well: running test_list shows that there are 14786 tuples with refcount == 1 that would have been untracked. This is a non-trivial workload to the gc passes that

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Pablo Galindo Salgado added the comment: I added some comments in the PR regarding the possibility of forcing the tracking on the visiting function to redirect the cost to gc passes instead of when calling next() but there is another option: not untracking tuples that have refcount == 1. --

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: functools.reduce looks affected, too. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: > I add Pablo and Tim who love GC puzzles. Well, I don’t think the GC is really doing anything wrong here... untracking these sort of tuples likely results in a noticeable reduction in collection times. This code is definitely testing the limits of what is “ok

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: The zip bug is quite old and was already tested by test_itertools for a very long time. Suddenly, it started to fail on more and more Refleaks buildbots. It may be because the GC must be triggered at a very specific point, and the default GC threshold trigge

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: I add Pablo and Tim who love GC puzzles. -- nosy: +pablogsal, tim.peters ___ Python tracker ___ _

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: Serhiy Storchaka: > Why there are cycles at all? In normal case there should not be cycle and the > result tuple should be destroyed when the iteration ends. test_itertools creates a reference cycle *on purpose*. See test_zip_leak2.py simplified code, it co

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: test_zip_leak2.py: simplified example to trigger the GC issue. Copy it to Lib/test/ and reproduce the leak with: ./python -m test -R 3:3 test_zip_leak2 Extract of the code: --- container = [] iterator = zip([container], [container]) # untrack the intern

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: Extract of Brandt's PR: // The GC may have untracked this result tuple if its elements were all // untracked. Since we're recycling it, make sure it's tracked again: if (!_PyObject_GC_IS_TRACKED(result)) { _PyObject_GC_TRACK(result); } I would like to und

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Serhiy Storchaka added the comment: Why there are cycles at all? In normal case there should not be cycle and the result tuple should be destroyed when the iteration ends. -- ___ Python tracker

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: Also, it appears enumerate is affected as well. -- ___ Python tracker ___ ___ Python-bugs-list mai

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: (By the way, I didn't know that -F runs the tests forever... so I was waiting *almost* forever for it to finish!) -- ___ Python tracker ___ _

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: It looks like Victor's original issue is unrelated to zip, though. That test run is clean after adding the same fix to: - itertools.product - itertools.combinations - itertools.combinations_with_replacement - itertools.permutations - itertools.zip_longest ...

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Change by Brandt Bucher : -- keywords: +patch pull_requests: +22491 stage: -> patch review pull_request: https://github.com/python/cpython/pull/23623 ___ Python tracker ___ __

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: Some quick benchmarks on a normal build with CPU isolation, tuned with pyperf. No PGO/LTO. $ ./python -m pyperf timeit --rigorous --setup 'i = (None,) * 10_000_000' 'for _, _ in zip(i, i): pass' # Current master. . Mea

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: Simple demo: >>> import gc >>> gc.disable() >>> z = zip([[]]) >>> gc.is_tracked(next(z)) True >>> z = zip([[]]) >>> gc.collect() 0 >>> gc.is_tracked(next(z)) False -- ___ Python tracker

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Brandt Bucher added the comment: It looks like the GC untracks the None-filled result tuple in untrack_tuples, and it's never re-tracked again. This can also happen if it's filled with atomic values on an early iteration and the GC visits it. Perhaps a simple fix is to call _PyObject_GC_TRAC

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Change by Raymond Hettinger : -- assignee: rhettinger -> ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: htt

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Raymond Hettinger added the comment: Adding Brandt because he recently worked on the zip() code. -- nosy: +brandtbucher ___ Python tracker ___

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: > The GC is supposed to be able to break such cycle, no? In zip_new(), lz->result tuple is *not* always tracked by the GC. -- ___ Python tracker ___

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

STINNER Victor added the comment: Raymond Hettinger: > If this is new, pairwise() is the most likely cause. I reproduced the issue without involving the itertools module. It seems to be a zip_next() micro-optimization which keeps an object alive longer than epxected. This micro-optimization

[issue42536] Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references)

Change by STINNER Victor : -- title: test_itertools leaks sometimes references -> Iterating on a zip keeps objects alive longer than expected (test_itertools leaks sometimes references) ___ Python tracker __