-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add GC infrastructure: tracking bits, tp_clear #6977
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.
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
Conversation
📝 WalkthroughWalkthroughThis PR introduces garbage collection cycle detection and breaking mechanisms for RustPython's VM. It adds a Changes
Sequence DiagramsequenceDiagram
participant Code as Client Code
participant PyRef as PyRef Creation
participant PyObj as PyObject
participant GCState as GC State
participant Dealloc as Deallocation
Code->>PyRef: new_ref(payload, type, dict)
PyRef->>PyObj: Create with payload
PyRef->>GCState: Check traversal capability
alt Has Traverse + Dict/HeapType
PyRef->>GCState: track_object()
GCState->>PyObj: set_gc_tracked()
PyObj->>PyObj: Mark GC bit
end
Note over GCState: GC Cycle Detection Triggered
GCState->>PyObj: Invoke clear callback
PyObj->>PyObj: Call tp_clear (via try_clear_obj)
PyObj-->>GCState: Return child references
GCState->>GCState: Process references for cycles
Code->>Dealloc: Object deallocation
Dealloc->>PyObj: default_dealloc()
Dealloc->>PyObj: tp_clear() if not cleared
Dealloc->>PyObj: Extract collected edges
Dealloc->>Dealloc: Free memory
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
GC bit operations (_PyObject_GC_TRACK/UNTRACK equivalent): - Add set_gc_bit() helper for atomic GC bit manipulation - Add set_gc_tracked() / clear_gc_tracked() methods - Update is_gc_tracked() to use GcBits::TRACKED flag - Call set_gc_tracked() in track_object() - Call clear_gc_tracked() for static types (they are immortal) tp_clear infrastructure (for breaking reference cycles): - Add try_clear_obj() function to call payload's try_clear - Add clear field to PyObjVTable - Add clear() method to PyType's Traverse impl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/vm/src/builtins/type.rs`:
- Around line 424-427: When marking static types immortal, in addition to
calling new_type.as_object().clear_gc_tracked() you must also remove the object
from the VM's GC tracking sets in GcState so it no longer appears in
gc.get_objects or generation counts; update the same block (the one handling
static types) to call the same removal logic used for weakref_type (i.e., remove
the object from the tracking sets / GcState tracking collections) immediately
after clear_gc_tracked() so is_gc_tracked() and the GC sets stay consistent.
| // Static types are not tracked by GC. | ||
| // They are immortal and never participate in collectable cycles. | ||
| new_type.as_object().clear_gc_tracked(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Untrack static types from GC state as well.
Clearing only the TRACKED bit leaves the type in GcState sets, so gc.get_objects/generation counts can still include it and diverge from is_gc_tracked(). Please also remove it from the tracking sets (similar to weakref_type).
💡 Suggested fix
- new_type.as_object().clear_gc_tracked();
+ let obj = new_type.as_object();
+ unsafe { crate::gc_state::gc_state().untrack_object(NonNull::from(obj)); }
+ obj.clear_gc_tracked();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Static types are not tracked by GC. | |
| // They are immortal and never participate in collectable cycles. | |
| new_type.as_object().clear_gc_tracked(); | |
| // Static types are not tracked by GC. | |
| // They are immortal and never participate in collectable cycles. | |
| let obj = new_type.as_object(); | |
| unsafe { crate::gc_state::gc_state().untrack_object(NonNull::from(obj)); } | |
| obj.clear_gc_tracked(); | |
🤖 Prompt for AI Agents
In `@crates/vm/src/builtins/type.rs` around lines 424 - 427, When marking static
types immortal, in addition to calling new_type.as_object().clear_gc_tracked()
you must also remove the object from the VM's GC tracking sets in GcState so it
no longer appears in gc.get_objects or generation counts; update the same block
(the one handling static types) to call the same removal logic used for
weakref_type (i.e., remove the object from the tracking sets / GcState tracking
collections) immediately after clear_gc_tracked() so is_gc_tracked() and the GC
sets stay consistent.
GC bit operations (_PyObject_GC_TRACK/UNTRACK equivalent):
tp_clear infrastructure (for breaking reference cycles):
Summary by CodeRabbit