Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Incremental cycle GC #108362

Open
markshannon opened this issue Aug 23, 2023 · 6 comments
Open

Incremental cycle GC #108362

markshannon opened this issue Aug 23, 2023 · 6 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@markshannon
Copy link
Member

markshannon commented Aug 23, 2023 •
edited by bedevere-app bot

The current GC is both inefficient and can have very long pause times.
The efficiency can be improved by tweaking thresholds or by using fewer generations (see #100403), but that does nothing to shorten pause times.

We should use an incremental collector, it can improve efficiency and hugely reduce maximum pause times.

See #100403 for more details on the general idea and the algorithm.

Linked PRs
@bedevere-bot bedevere-bot mentioned this issue Aug 23, 2023
Merged
@iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 27, 2023
markshannon added a commit that referenced this issue Feb 5, 2024
@markshannon
GH-108362: Incremental GC implementation (GH-108038)
36518e6
@encukou
Copy link
Member

encukou commented Feb 6, 2024

Several stable buildbots fail on the new assertion:

test_thread_safety (test.test_multiprocessing_forkserver.test_processes.WithProcessesTestFinalize.test_thread_safety) ... Assertion failed: (gc_list_is_empty(to) || gc_old_space(to_tail) == gc_old_space(from_tail)), function gc_list_merge, file gc.c, line 320.
Fatal Python error: Aborted
Current thread 0x0000000170977000 (most recent call first):
  Garbage-collecting
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/multiprocessing/util.py", line 189 in __init__
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/_test_multiprocessing.py", line 4595 in __init__
  File "__init__", line ??? in __init__
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/_test_multiprocessing.py", line 4618 in make_finalizers
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/threading.py", line 1026 in run
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/threading.py", line 1089 in _bootstrap_inner
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/threading.py", line 1046 in _bootstrap
Thread 0x000000016f96b000 (most recent call first):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/multiprocessing/util.py", line 214 in __call__
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/multiprocessing/util.py", line 296 in _run_finalizers
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/_test_multiprocessing.py", line 4607 in run_finalizers
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/threading.py", line 1026 in run
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/threading.py", line 1089 in _bootstrap_inner
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/threading.py", line 1046 in _bootstrap
Thread 0x00000001e2c35ec0 (most recent call first):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/_test_multiprocessing.py", line 4631 in test_thread_safety
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/unittest/case.py", line 589 in _callTestMethod
  ...

Do you have a quick fix that can be added today? If not, please revert first and then investigate.
(I can help, but not today.)

@markshannon
Copy link
Member Author

I'll take a look. I can't reproduce the failure locally and it will be tricky to diagnose without a C backtrace.

@bedevere-app bedevere-app bot mentioned this issue Feb 6, 2024
Closed
@markshannon
Copy link
Member Author

@encukou
I can't fix it now.
I'll do a revert PR tomorrow, unless you beat me to it.

@markshannon markshannon mentioned this issue Feb 7, 2024
Closed
markshannon added a commit to faster-cpython/cpython that referenced this issue Feb 7, 2024
@markshannon
Revert "pythonGH-108362: Incremental GC implementation (pythonGH-108038…
f0dd4df 
…)"

This reverts commit 36518e6.
@bedevere-app bedevere-app bot mentioned this issue Feb 7, 2024
Merged
pablogsal pushed a commit that referenced this issue Feb 7, 2024
@markshannon
GH-108362: Revert "GH-108362: Incremental GC implementation (GH-108038)…
8a3c499 
…" (#115132)

Revert "GH-108362: Incremental GC implementation (GH-108038)"

This reverts commit 36518e6.
@encukou
Copy link
Member

encukou commented Feb 7, 2024

Thank you for the revert.

The bigmem buildbot also failed on the change. For next PR fix, please run the bigmem tests (./python -m test test_bigmem -v -M 20G or however much RAM you can spare) and/or the bigmem buildbot.

@encukou encukou mentioned this issue Feb 7, 2024
Closed
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this issue Feb 14, 2024
@markshannon @fsc-eriker
pythonGH-108362: Incremental GC implementation (pythonGH-108038)
d28be26
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this issue Feb 14, 2024
@markshannon @fsc-eriker
pythonGH-108362: Revert "pythonGH-108362: Incremental GC implementati…
0b95cb5 
…on (pythonGH-108038)" (python#115132)

Revert "pythonGH-108362: Incremental GC implementation (pythonGH-108038)"

This reverts commit 36518e6.
This was referenced Mar 1, 2024
Closed
Merged
oraluben pushed a commit to oraluben/cpython that referenced this issue Mar 5, 2024
@leveretconey @oraluben
Backport pythonGH-108362: Incremental GC implementation
af62957 
Implements incremental cyclic GC.
Instead of traversing one generation on each collection, we traverse the young generation and the oldest part of the old generation. By traversing the old generation a chunk at a time, we keep pause times down a lot.
markshannon added a commit that referenced this issue Mar 20, 2024
@markshannon
GH-108362: Incremental Cycle GC (GH-116206)
1530932
vstinner pushed a commit to vstinner/cpython that referenced this issue Mar 20, 2024
@markshannon @vstinner
pythonGH-108362: Incremental Cycle GC (pythonGH-116206)
f4b2af9
gvanrossum added a commit to gvanrossum/cpython that referenced this issue Mar 23, 2024
@gvanrossum
Revert "pythonGH-108362: Incremental Cycle GC (pythonGH-116206)"
e6480f7 
Second half of the revert.

This reverts commit 1530932.
@gvanrossum
Copy link
Member

@markshannon I have two buildbot failures crashing on an assert in gc_list_merge() that you touched:

        assert(gc_list_is_empty(to) ||
            gc_old_space(to_tail) == gc_old_space(from_tail));

This is in https://buildbot.python.org/all/#/builders/223/builds/5581/steps/5/logs/stdio and also in https://buildbot.python.org/all/#/builders/469/builds/7667/steps/6/logs/stdio.

@gvanrossum gvanrossum mentioned this issue Mar 23, 2024
Merged
@gvanrossum
Copy link
Member

I see it in other PRs too, e.g.
https://buildbot.python.org/all/#/builders/146/builds/8134/steps/4/logs/stdio

adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
@markshannon @adorilson
pythonGH-108362: Incremental Cycle GC (pythonGH-116206)
41b15e9
@gvanrossum gvanrossum mentioned this issue Mar 25, 2024
Merged
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
@markshannon @diegorusso
pythonGH-108362: Incremental Cycle GC (pythonGH-116206)
56ad354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@encukou @iritkatriel @gvanrossum @markshannon