|
| 1 | +From fef3eed829b8f69f38708059e75d25bd72825a6a Mon Sep 17 00:00:00 2001 |
| 2 | +From: Neil Schemenauer <nas@arctrix.com> |
| 3 | +Date: Wed, 26 Nov 2025 20:40:48 -0800 |
| 4 | +Subject: [PATCH] Revert GC changes to "work_to_do" logic. |
| 5 | + |
| 6 | +This reverts parts of the GH-140262 change. The changes that affect the |
| 7 | +tuple untracking are left unchanged. Revert the changes to the |
| 8 | +calculation of the increment size, based on the "work_to_do" variable. |
| 9 | +This causes cyclic garbage to be collected more quickly. Revert also |
| 10 | +the change to test_gc.py, which was done because the expected GC |
| 11 | +collection was taking longer to happen. |
| 12 | + |
| 13 | +With the tuple untrack change, the performance regression as reported by |
| 14 | +bug GH-139951 is still resolved (work_to_do changes are not required). |
| 15 | +--- |
| 16 | + Lib/test/test_gc.py | 17 ++++++++--------- |
| 17 | + Python/gc.c | 6 ++---- |
| 18 | + 2 files changed, 10 insertions(+), 13 deletions(-) |
| 19 | + |
| 20 | +diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py |
| 21 | +index ec5df4d20e7085..5d610c38579bd6 100644 |
| 22 | +--- a/Lib/test/test_gc.py |
| 23 | ++++ b/Lib/test/test_gc.py |
| 24 | +@@ -1493,11 +1493,10 @@ def callback(ignored): |
| 25 | + # The free-threaded build doesn't have multiple generations, so |
| 26 | + # just trigger a GC manually. |
| 27 | + gc.collect() |
| 28 | +- assert not detector.gc_happened |
| 29 | + while not detector.gc_happened: |
| 30 | + i += 1 |
| 31 | +- if i > 100000: |
| 32 | +- self.fail("gc didn't happen after 100000 iterations") |
| 33 | ++ if i > 10000: |
| 34 | ++ self.fail("gc didn't happen after 10000 iterations") |
| 35 | + self.assertEqual(len(ouch), 0) |
| 36 | + junk.append([]) # this will eventually trigger gc |
| 37 | + |
| 38 | +@@ -1569,8 +1568,8 @@ def __del__(self): |
| 39 | + gc.collect() |
| 40 | + while not detector.gc_happened: |
| 41 | + i += 1 |
| 42 | +- if i > 50000: |
| 43 | +- self.fail("gc didn't happen after 50000 iterations") |
| 44 | ++ if i > 10000: |
| 45 | ++ self.fail("gc didn't happen after 10000 iterations") |
| 46 | + self.assertEqual(len(ouch), 0) |
| 47 | + junk.append([]) # this will eventually trigger gc |
| 48 | + |
| 49 | +@@ -1587,8 +1586,8 @@ def test_indirect_calls_with_gc_disabled(self): |
| 50 | + detector = GC_Detector() |
| 51 | + while not detector.gc_happened: |
| 52 | + i += 1 |
| 53 | +- if i > 100000: |
| 54 | +- self.fail("gc didn't happen after 100000 iterations") |
| 55 | ++ if i > 10000: |
| 56 | ++ self.fail("gc didn't happen after 10000 iterations") |
| 57 | + junk.append([]) # this will eventually trigger gc |
| 58 | + |
| 59 | + try: |
| 60 | +@@ -1598,11 +1597,11 @@ def test_indirect_calls_with_gc_disabled(self): |
| 61 | + detector = GC_Detector() |
| 62 | + while not detector.gc_happened: |
| 63 | + i += 1 |
| 64 | +- if i > 100000: |
| 65 | ++ if i > 10000: |
| 66 | + break |
| 67 | + junk.append([]) # this may eventually trigger gc (if it is enabled) |
| 68 | + |
| 69 | +- self.assertEqual(i, 100001) |
| 70 | ++ self.assertEqual(i, 10001) |
| 71 | + finally: |
| 72 | + gc.enable() |
| 73 | + |
| 74 | +diff --git a/Python/gc.c b/Python/gc.c |
| 75 | +index d067a6144b0763..c4c56e319f4853 100644 |
| 76 | +--- a/Python/gc.c |
| 77 | ++++ b/Python/gc.c |
| 78 | +@@ -1644,7 +1644,7 @@ assess_work_to_do(GCState *gcstate) |
| 79 | + scale_factor = 2; |
| 80 | + } |
| 81 | + intptr_t new_objects = gcstate->young.count; |
| 82 | +- intptr_t max_heap_fraction = new_objects*2; |
| 83 | ++ intptr_t max_heap_fraction = new_objects*3/2; |
| 84 | + intptr_t heap_fraction = gcstate->heap_size / SCAN_RATE_DIVISOR / scale_factor; |
| 85 | + if (heap_fraction > max_heap_fraction) { |
| 86 | + heap_fraction = max_heap_fraction; |
| 87 | +@@ -1659,9 +1659,6 @@ gc_collect_increment(PyThreadState *tstate, struct gc_collection_stats *stats) |
| 88 | + GC_STAT_ADD(1, collections, 1); |
| 89 | + GCState *gcstate = &tstate->interp->gc; |
| 90 | + gcstate->work_to_do += assess_work_to_do(gcstate); |
| 91 | +- if (gcstate->work_to_do < 0) { |
| 92 | +- return; |
| 93 | +- } |
| 94 | + untrack_tuples(&gcstate->young.head); |
| 95 | + if (gcstate->phase == GC_PHASE_MARK) { |
| 96 | + Py_ssize_t objects_marked = mark_at_start(tstate); |
| 97 | +@@ -1705,6 +1702,7 @@ gc_collect_increment(PyThreadState *tstate, struct gc_collection_stats *stats) |
| 98 | + gc_collect_region(tstate, &increment, &survivors, stats); |
| 99 | + gc_list_merge(&survivors, visited); |
| 100 | + assert(gc_list_is_empty(&increment)); |
| 101 | ++ gcstate->work_to_do += gcstate->heap_size / SCAN_RATE_DIVISOR / scale_factor; |
| 102 | + gcstate->work_to_do -= increment_size; |
| 103 | + |
| 104 | + if (gc_list_is_empty(not_visited)) { |
0 commit comments