Fix gh-559 memory growth: build .ss lazily instead of storing it - #598
Open
eriknw wants to merge 1 commit into
Open
Fix gh-559 memory growth: build .ss lazily instead of storing it#598eriknw wants to merge 1 commit into
eriknw wants to merge 1 commit into
Conversation
eriknw
marked this pull request as ready for review
August 4, 2026 16:07
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 4, 2026 16:12
9146314 to
37dca2b
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 5, 2026 00:06
37dca2b to
231b1d9
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 5, 2026 03:18
231b1d9 to
609f44d
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 5, 2026 17:44
609f44d to
3e41cb9
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 5, 2026 18:03
3e41cb9 to
949f5e1
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 5, 2026 18:05
949f5e1 to
cce705d
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 6, 2026 07:59
cce705d to
1706f84
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 6, 2026 15:39
1706f84 to
be91313
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 6, 2026 15:41
be91313 to
5913c15
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 6, 2026 20:36
5913c15 to
03fffef
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 6, 2026 20:42
03fffef to
1c7e63b
Compare
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 7, 2026 02:48
1c7e63b to
bfce0b2
Compare
Every Matrix and Vector was born inside two reference cycles: the stored self.ss held _parent back to the object, and so did ss.config. Instances therefore died by the cyclic garbage collector rather than by refcount, and their C-side GrB buffers accumulated between gc sweeps. With gc disabled, a scaled version of the gh-559 batched-mxm loop grew without bound. The .ss namespace is now built per access (a property inside the existing class_property), nothing is stored on the parent, no cycle forms, and objects free as soon as their refcount drops. Two user-visible behavior changes: A.ss is A.ss is now False. It was True, because .ss was a stored attribute; each access now returns a fresh namespace object. Code that compares .ss by identity, or that caches attributes on it, will see the difference. Assigning A.ss now raises AttributeError. It previously succeeded and silently replaced the namespace, because "ss" was in __slots__. Class-level Matrix.ss and Vector.ss still resolve to the ss class, so the import_* classmethods are unchanged. Building the namespace per access costs roughly 215ns against roughly 78ns for the stored attribute (timeit), so about 1% of a single small mxm plus to_dense iteration, which runs about 15us. The .ss namespace is typically touched once per user operation. Six of the eight new tests fail without the fix. They assert on the reference graph rather than on process memory, which a functional suite cannot see and which an RSS delta would measure only statistically: a weakref must be dead the instant the last strong reference drops with the cyclic collector switched off, and a batched mxm loop must not raise the number of live Matrix objects reported by gc.get_objects(). The remaining two cover invariants the fix has to preserve, class-level access and views whose _parent is set, so they pass either way.
eriknw
force-pushed
the
18-ss-namespace-refcycle
branch
from
August 7, 2026 05:09
bfce0b2 to
7a5cb1e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every Matrix and Vector was born inside two reference cycles: the stored
self.ss held _parent back to the object, and so did ss.config. Instances
therefore died by the cyclic garbage collector rather than by refcount, and
their C-side GrB buffers accumulated between gc sweeps. With gc disabled, a
scaled version of the gh-559 batched-mxm loop grew without bound. The .ss
namespace is now built per access (a property inside the existing
class_property), nothing is stored on the parent, no cycle forms, and objects
free as soon as their refcount drops.
Two user-visible behavior changes:
A.ss is A.ss is now False. It was True, because .ss was a stored attribute;
each access now returns a fresh namespace object. Code that compares .ss by
identity, or that caches attributes on it, will see the difference.
Assigning A.ss now raises AttributeError. It previously succeeded and
silently replaced the namespace, because "ss" was in slots.
Class-level Matrix.ss and Vector.ss still resolve to the ss class, so the
import_* classmethods are unchanged.
Building the namespace per access costs roughly 215ns against roughly 78ns for
the stored attribute (timeit), so about 1% of a single small mxm plus to_dense
iteration, which runs about 15us. The .ss namespace is typically touched once
per user operation.
Six of the eight new tests fail without the fix. They assert on the reference
graph rather than on process memory, which a functional suite cannot see and
which an RSS delta would measure only statistically: a weakref must be dead the
instant the last strong reference drops with the cyclic collector switched off,
and a batched mxm loop must not raise the number of live Matrix objects
reported by gc.get_objects(). The remaining two cover invariants the fix has
to preserve, class-level access and views whose _parent is set, so they pass
either way.
Stack created with GitHub Stacks CLI • Give Feedback 💬