Repository navigation
Share ctypes backing-owner snapshots between aggregate copies - #9392
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Optional collection during allocation crates/vm/src/gc_state.rs, crates/vm/src/object/core.rs |
track_new_object_impl and a shared PyRef allocation path accept a collection flag. new_ref enables collection; new_ref_without_collection disables it on supported host builds. |
Owner groups and snapshots crates/vm/src/stdlib/_ctypes/base.rs |
PrivateRefs tracks owner groups and reference counts, traverses and clears retained references, and caches a tuple snapshot for multiple owners until the owner set changes. |
CData hidden-reference integration crates/vm/src/stdlib/_ctypes/base.rs |
CDataValue and retention helpers use optional HiddenRefs groups in conversions, assignments, and keep operations. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Refactor
Possibly related PRs
- RustPython/RustPython#9391: Changes
PrivateRefsowner structure and traversal paths used by this PR. - RustPython/RustPython#9390: Adds GC traversal and clearing for ctypes state that this PR extends.
- RustPython/RustPython#9251: Introduces the reference-counted ownership and snapshot model that this PR changes.
Suggested reviewers: youknowone
Merge Risk: 
Minimal · up to f4df5
-
No actionable merge-blocking defect is established by the reviewed changes.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed -
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Assisted-by: Codex:GPT-6
Assisted-by: Codex:GPT-6
b23664e to
332101a
Compare
|
|
||
| /// Allocate and track internal storage without running collection during | ||
| /// publication. The next ordinary tracked allocation still checks thresholds. | ||
| pub(crate) fn new_ref_without_collection( |
There was a problem hiding this comment.
the naming is confusing. new_ref_without_collection is default false call of new_ref_with_collection. those names are looking symmetric.
| /// # Safety | ||
| /// obj must be a valid pointer to an untracked PyObject with freshly zeroed `gc_bits`. | ||
| #[inline] | ||
| pub(crate) unsafe fn track_new_object_with_collection(obj: NonNull<PyObject>, collect: bool) { |
There was a problem hiding this comment.
let's give a good name here. I will think a bit more.
Assisted-by: Codex:GPT-6
Addresses the quadratic private ownership metadata identified while reviewing #9251.
Summary
Performance
Repeated aggregate copies with an unchanged owner set:
Peak process RSS in the Debian scenario: 236.2 MiB → 70.9 MiB.
AI assistance
Written with Codex (GPT-6), reviewed by a human before submission.
Summary by CodeRabbit