Skip to content

Fix gh-559 memory growth: build .ss lazily instead of storing it - #598

Open
eriknw wants to merge 1 commit into
17-viz-fixesfrom
18-ss-namespace-refcycle
Open

Fix gh-559 memory growth: build .ss lazily instead of storing it#598
eriknw wants to merge 1 commit into
17-viz-fixesfrom
18-ss-namespace-refcycle

Conversation

@eriknw

@eriknw eriknw commented Aug 4, 2026

Copy link
Copy Markdown
Member

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 CLIGive Feedback 💬

@eriknw
eriknw marked this pull request as ready for review August 4, 2026 16:07
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 9146314 to 37dca2b Compare August 4, 2026 16:12
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 37dca2b to 231b1d9 Compare August 5, 2026 00:06
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 231b1d9 to 609f44d Compare August 5, 2026 03:18
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 609f44d to 3e41cb9 Compare August 5, 2026 17:44
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 3e41cb9 to 949f5e1 Compare August 5, 2026 18:03
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 949f5e1 to cce705d Compare August 5, 2026 18:05
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from cce705d to 1706f84 Compare August 6, 2026 07:59
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 1706f84 to be91313 Compare August 6, 2026 15:39
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from be91313 to 5913c15 Compare August 6, 2026 15:41
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 5913c15 to 03fffef Compare August 6, 2026 20:36
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 03fffef to 1c7e63b Compare August 6, 2026 20:42
@eriknw
eriknw force-pushed the 18-ss-namespace-refcycle branch from 1c7e63b to bfce0b2 Compare August 7, 2026 02:48
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
eriknw force-pushed the 18-ss-namespace-refcycle branch from bfce0b2 to 7a5cb1e Compare August 7, 2026 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant