-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Python gc fixes prototype #22022
base: master
Are you sure you want to change the base?
Python gc fixes prototype #22022
Conversation
1226333
to
071ef41
Compare
59a6b3c
to
196cb5d
Compare
196cb5d
to
d7f5bc7
Compare
d7f5bc7
to
9098a07
Compare
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.
Updated. Please note, as usual, that docs and tests are far from complete.
Reviewed 1 of 4 files at r2, 2 of 35 files at r4.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes
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.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes
bindings/pydrake/systems/framework_py_semantics.cc
line 593 at r4 (raw file):
template <typename T> struct BuilderLifeSupport {
This bit (and changes to drake/systems/framework) helps cover the rare, but probably unavoidable case of a program that adds systems via Python, but calls Build() from C++. See, for example, Anzu just before anzu#14558. Note that from-Python Build() calls abandon()
, so that in the all-Python case there are no immortals (leaks), only collectible ref-cycles.
9098a07
to
3799f27
Compare
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.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes
bindings/pydrake/systems/framework_py_semantics.cc
line 593 at r4 (raw file):
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
This bit (and changes to drake/systems/framework) helps cover the rare, but probably unavoidable case of a program that adds systems via Python, but calls Build() from C++. See, for example, Anzu just before anzu#14558. Note that from-Python Build() calls
abandon()
, so that in the all-Python case there are no immortals (leaks), only collectible ref-cycles.
To be clear, the code this enables is the code that was deleted in anzu#14558.
3799f27
to
6e5ecc3
Compare
c65d58d
to
3ed3f6a
Compare
This patch adds a life support interface that allows arbitrary objects to track the lifetime of systems as they get transferred from a diagram builder to a diagram.
wip: declare victory on existing memory leak tests
3ed3f6a
to
952135f
Compare
This is the dev branch for fixes toward #14387 . I won't merge anything directly from here, but rather peel of merge-able PRs separately. This branch should always contain the remainder of the work, though not necessarily in proper form.
This change is