gc: add CollectResult, stats fields, get_referrers, and fix count reset

youknowone · youknowone · commit d100dc2b53ba · 2026-03-05T09:14:30.000+09:00
- Add CollectResult struct with collected/uncollectable/candidates/duration
- Add candidates and duration fields to GcStats and gc.get_stats()
- Pass CollectResult to gc.callbacks info dict
- Reset generation counts for all collected generations (0..=N)
- Return 0 for third value in gc.get_threshold() (3.13+)
- Implement gc.get_referrers() by scanning all tracked objects
- Add DEBUG_COLLECTABLE output for collectable objects
- Update test_gc.py to expect candidates/duration in stats
diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
@@ -822,7 +822,7 @@ def test_get_stats(self):
         for st in stats:
             self.assertIsInstance(st, dict)
             self.assertEqual(set(st),
-                             {"collected", "collections", "uncollectable"})
+                             {"collected", "collections", "uncollectable", "candidates", "duration"})
             self.assertGreaterEqual(st["collected"], 0)
             self.assertGreaterEqual(st["collections"], 0)
             self.assertGreaterEqual(st["uncollectable"], 0)
diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs
@@ -28,12 +28,23 @@ bitflags::bitflags! {
     }
 }
 
+/// Result from a single collection run
+#[derive(Debug, Default)]
+pub struct CollectResult {
+    pub collected: usize,
+    pub uncollectable: usize,
+    pub candidates: usize,
+    pub duration: f64,
+}
+
 /// Statistics for a single generation (gc_generation_stats)
 #[derive(Debug, Default)]
 pub struct GcStats {
     pub collections: usize,
     pub collected: usize,
     pub uncollectable: usize,
+    pub candidates: usize,
+    pub duration: f64,
 }
 
 /// A single GC generation with intrusive linked list
@@ -55,6 +66,8 @@ impl GcGeneration {
                 collections: 0,
                 collected: 0,
                 uncollectable: 0,
+                candidates: 0,
+                duration: 0.0,
             }),
         }
     }
@@ -77,14 +90,24 @@ impl GcGeneration {
             collections: guard.collections,
             collected: guard.collected,
             uncollectable: guard.uncollectable,
+            candidates: guard.candidates,
+            duration: guard.duration,
         }
     }
 
-    pub fn update_stats(&self, collected: usize, uncollectable: usize) {
+    pub fn update_stats(
+        &self,
+        collected: usize,
+        uncollectable: usize,
+        candidates: usize,
+        duration: f64,
+    ) {
         let mut guard = self.stats.lock();
         guard.collections += 1;
         guard.collected += collected;
         guard.uncollectable += uncollectable;
+        guard.candidates += candidates;
+        guard.duration += duration;
     }
 
     /// Reset the stats mutex to unlocked state after fork().
@@ -346,25 +369,27 @@ impl GcState {
     }
 
     /// Perform garbage collection on the given generation
-    pub fn collect(&self, generation: usize) -> (usize, usize) {
+    pub fn collect(&self, generation: usize) -> CollectResult {
         self.collect_inner(generation, false)
     }
 
     /// Force collection even if GC is disabled (for manual gc.collect() calls)
-    pub fn collect_force(&self, generation: usize) -> (usize, usize) {
+    pub fn collect_force(&self, generation: usize) -> CollectResult {
         self.collect_inner(generation, true)
     }
 
-    fn collect_inner(&self, generation: usize, force: bool) -> (usize, usize) {
+    fn collect_inner(&self, generation: usize, force: bool) -> CollectResult {
         if !force && !self.is_enabled() {
-            return (0, 0);
+            return CollectResult::default();
         }
 
         // Try to acquire the collecting lock
         let Some(_guard) = self.collecting.try_lock() else {
-            return (0, 0);
+            return CollectResult::default();
         };
 
+        let start_time = std::time::Instant::now();
+
         // Memory barrier to ensure visibility of all reference count updates
         // from other threads before we start analyzing the object graph.
         core::sync::atomic::fence(Ordering::SeqCst);
@@ -392,11 +417,21 @@ impl GcState {
         }
 
         if collecting.is_empty() {
-            self.generations[0].count.store(0, Ordering::SeqCst);
-            self.generations[generation].update_stats(0, 0);
-            return (0, 0);
+            for i in 0..=generation {
+                self.generations[i].count.store(0, Ordering::SeqCst);
+            }
+            let duration = start_time.elapsed().as_secs_f64();
+            self.generations[generation].update_stats(0, 0, 0, duration);
+            return CollectResult {
+                collected: 0,
+                uncollectable: 0,
+                candidates: 0,
+                duration,
+            };
         }
 
+        let candidates = collecting.len();
+
         if debug.contains(GcDebugFlags::STATS) {
             eprintln!(
                 "gc: collecting {} objects from generations 0..={}",
@@ -495,9 +530,17 @@ impl GcState {
         if unreachable.is_empty() {
             drop(gen_locks);
             self.promote_survivors(generation, &survivor_refs);
-            self.generations[0].count.store(0, Ordering::SeqCst);
-            self.generations[generation].update_stats(0, 0);
-            return (0, 0);
+            for i in 0..=generation {
+                self.generations[i].count.store(0, Ordering::SeqCst);
+            }
+            let duration = start_time.elapsed().as_secs_f64();
+            self.generations[generation].update_stats(0, 0, candidates, duration);
+            return CollectResult {
+                collected: 0,
+                uncollectable: 0,
+                candidates,
+                duration,
+            };
         }
 
         // Release read locks before finalization phase.
@@ -507,9 +550,17 @@ impl GcState {
 
         if unreachable_refs.is_empty() {
             self.promote_survivors(generation, &survivor_refs);
-            self.generations[0].count.store(0, Ordering::SeqCst);
-            self.generations[generation].update_stats(0, 0);
-            return (0, 0);
+            for i in 0..=generation {
+                self.generations[i].count.store(0, Ordering::SeqCst);
+            }
+            let duration = start_time.elapsed().as_secs_f64();
+            self.generations[generation].update_stats(0, 0, candidates, duration);
+            return CollectResult {
+                collected: 0,
+                uncollectable: 0,
+                candidates,
+                duration,
+            };
         }
 
         // 6b: Record initial strong counts (for resurrection detection)
@@ -612,6 +663,16 @@ impl GcState {
         // Resurrected objects stay tracked — just drop our references
         drop(resurrected);
 
+        if debug.contains(GcDebugFlags::COLLECTABLE) {
+            for obj in &truly_dead {
+                eprintln!(
+                    "gc: collectable <{} {:p}>",
+                    obj.class().name(),
+                    obj.as_ref()
+                );
+            }
+        }
+
         if debug.contains(GcDebugFlags::SAVEALL) {
             let mut garbage_guard = self.garbage.lock();
             for obj_ref in truly_dead.iter() {
@@ -633,12 +694,20 @@ impl GcState {
             });
         }
 
-        // Reset gen0 count
-        self.generations[0].count.store(0, Ordering::SeqCst);
+        // Reset counts for all collected generations
+        for i in 0..=generation {
+            self.generations[i].count.store(0, Ordering::SeqCst);
+        }
 
-        self.generations[generation].update_stats(collected, 0);
+        let duration = start_time.elapsed().as_secs_f64();
+        self.generations[generation].update_stats(collected, 0, candidates, duration);
 
-        (collected, 0)
+        CollectResult {
+            collected,
+            uncollectable: 0,
+            candidates,
+            duration,
+        }
     }
 
     /// Promote surviving objects to the next generation.
diff --git a/crates/vm/src/stdlib/gc.rs b/crates/vm/src/stdlib/gc.rs
@@ -55,11 +55,11 @@ mod gc {
         }
 
         // Invoke callbacks with "start" phase
-        invoke_callbacks(vm, "start", generation_num as usize, 0, 0);
+        invoke_callbacks(vm, "start", generation_num as usize, &Default::default());
 
         // Manual gc.collect() should run even if GC is disabled
         let gc = gc_state::gc_state();
-        let (collected, uncollectable) = gc.collect_force(generation_num as usize);
+        let result = gc.collect_force(generation_num as usize);
 
         // Move objects from gc_state.garbage to vm.ctx.gc_garbage (for DEBUG_SAVEALL)
         {
@@ -74,26 +74,21 @@ mod gc {
         }
 
         // Invoke callbacks with "stop" phase
-        invoke_callbacks(
-            vm,
-            "stop",
-            generation_num as usize,
-            collected,
-            uncollectable,
-        );
+        invoke_callbacks(vm, "stop", generation_num as usize, &result);
 
-        Ok(collected as i32)
+        Ok((result.collected + result.uncollectable) as i32)
     }
 
     /// Return the current collection thresholds as a tuple.
+    /// The third value is always 0 (matching 3.13+).
     #[pyfunction]
     fn get_threshold(vm: &VirtualMachine) -> PyObjectRef {
-        let (t0, t1, t2) = gc_state::gc_state().get_threshold();
+        let (t0, t1, _t2) = gc_state::gc_state().get_threshold();
         vm.ctx
             .new_tuple(vec![
                 vm.ctx.new_int(t0).into(),
                 vm.ctx.new_int(t1).into(),
-                vm.ctx.new_int(t2).into(),
+                vm.ctx.new_int(0).into(),
             ])
             .into()
     }
@@ -148,6 +143,8 @@ mod gc {
                 vm.ctx.new_int(stat.uncollectable).into(),
                 vm,
             )?;
+            dict.set_item("candidates", vm.ctx.new_int(stat.candidates).into(), vm)?;
+            dict.set_item("duration", vm.ctx.new_float(stat.duration).into(), vm)?;
             result.push(dict.into());
         }
 
@@ -189,10 +186,30 @@ mod gc {
     /// Return the list of objects that directly refer to any of the arguments.
     #[pyfunction]
     fn get_referrers(args: FuncArgs, vm: &VirtualMachine) -> PyListRef {
-        // This is expensive: we need to scan all tracked objects
-        // For now, return an empty list (would need full object tracking to implement)
-        let _ = args;
-        vm.ctx.new_list(vec![])
+        use std::collections::HashSet;
+
+        // Build a set of target object pointers for fast lookup
+        let targets: HashSet<usize> = args
+            .args
+            .iter()
+            .map(|obj| obj.as_ref() as *const crate::PyObject as usize)
+            .collect();
+
+        let mut result = Vec::new();
+
+        // Scan all tracked objects across all generations
+        let all_objects = gc_state::gc_state().get_objects(None);
+        for obj in all_objects {
+            let referent_ptrs = unsafe { obj.gc_get_referent_ptrs() };
+            for child_ptr in referent_ptrs {
+                if targets.contains(&(child_ptr.as_ptr() as usize)) {
+                    result.push(obj.clone());
+                    break;
+                }
+            }
+        }
+
+        vm.ctx.new_list(result)
     }
 
     /// Return True if the object is tracked by the garbage collector.
@@ -243,8 +260,7 @@ mod gc {
         vm: &VirtualMachine,
         phase: &str,
         generation: usize,
-        collected: usize,
-        uncollectable: usize,
+        result: &gc_state::CollectResult,
     ) {
         let callbacks_list = &vm.ctx.gc_callbacks;
         let callbacks: Vec<PyObjectRef> = callbacks_list.borrow_vec().to_vec();
@@ -255,8 +271,12 @@ mod gc {
         let phase_str: PyObjectRef = vm.ctx.new_str(phase).into();
         let info = vm.ctx.new_dict();
         let _ = info.set_item("generation", vm.ctx.new_int(generation).into(), vm);
-        let _ = info.set_item("collected", vm.ctx.new_int(collected).into(), vm);
-        let _ = info.set_item("uncollectable", vm.ctx.new_int(uncollectable).into(), vm);
+        let _ = info.set_item("collected", vm.ctx.new_int(result.collected).into(), vm);
+        let _ = info.set_item(
+            "uncollectable",
+            vm.ctx.new_int(result.uncollectable).into(),
+            vm,
+        );
 
         for callback in callbacks {
             let _ = callback.call((phase_str.clone(), info.clone()), vm);
