Repository navigation
perf(maui): track a recycled view's list without a weak reference per item view - #56
Open
michaelstonis wants to merge 3 commits into
Open
michaelstonis wants to merge 3 commits into
michaelstonis wants to merge 3 commits into
Conversation
…cument the option Passing 0 to a list that had never opted in created a watcher and a PropertyChanged subscription for a list that keeps nothing. Adds the README section for the option, tests that a list which has not opted in is not watched and that opting in after rows are bound takes effect, and puts every MAUI test class through UseStellarComponents so results cannot depend on which class initialises ReactiveUI first. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… item view Every item view of every ItemsView, opted in or not, allocated a WeakReference to its list the first time it got a parent, because by the time MAUI reports the window gone the parent has already been cleared. The view manager now remembers the list's ItemsHostWatcher instead, which holds the list weakly itself, with a shared None standing in for a list that has not opted in. That is 24 bytes, a GC handle and a finalizer less per item view (measured: 24 B to 0 B on the first parent change), and a recycle no longer looks the watcher up. TryPark no longer builds the Parking and its weak references before the watcher has accepted the view: the watcher creates the slot's weak reference only when it takes the view, under the same lock, and hands it back for the manager to keep. A view refused by a full list allocates nothing. Find skips the table lookup until some list has opted in, so an app that never calls RecycledItemViewLimit pays a static read per bind. A view now learns on bind, not on recycle, whether its list keeps recycled views, so a view bound before its list opted in is torn down once more and kept from its next bind on; the test, the README and the XML docs say so. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The public documentation misstates cross-platform late opt-in behavior, and the research verdict contradicts its described core change.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
Optimizes MAUI recycled-row tracking by eliminating unnecessary per-view weak references and deferred parking allocations.
Changes:
- Shares list watchers through a sentinel-backed lookup.
- Allocates parking state only for accepted views.
- Adds allocation tests and lifecycle documentation.
| File | Description |
|---|---|
Stellar.Maui/MauiViewManager.cs |
Tracks shared list watchers and defers parking allocation. |
Stellar.Maui/ItemsHostWatcher.cs |
Adds sentinel, host tracking, and lazy slots. |
Stellar.Maui/Extensions/ItemsViewExtensions.cs |
Avoids watcher creation for a zero limit. |
Stellar.Maui.UnitTests/StellarTestBase.cs |
Documents shared ReactiveUI initialization. |
Stellar.Maui.UnitTests/MauiTestBase.cs |
Uses common test initialization. |
Stellar.Maui.UnitTests/ItemsViewRecyclingTests.cs |
Covers allocations and late opt-in behavior. |
review-follow-ups-research-2026-10-05.md |
Records research and measurements. |
README.md |
Documents recycled-row retention. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+526
to
+528
| - Set the limit before the list shows its rows. A row learns whether its list keeps rows | ||
| when it is bound, so a row that was already bound when the limit was raised is torn down | ||
| once more and kept from the next time it is bound. |
Comment on lines
+18
to
+21
| /// views that take everything they show from bindings to the view model. Set the limit | ||
| /// before the list shows its rows: a view learns whether its list keeps views when it | ||
| /// is bound, so one bound before the limit was raised is torn down once more and kept | ||
| /// from the next time it is bound. |
Comment on lines
+42
to
+43
| **Verdict: solved** on `fix/maui-replaced-view-model-lifecycle` at `b8c839f`, contained in | ||
| `Stellar.Maui`, no core change, no per-recycle allocation. |
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.
Stacked on #54 (its commit shows here until it merges).
Why
Every item view of every
ItemsViewallocated aWeakReference<ItemsView?>the first time it was parented, opted in or not: 24 bytes plus a GC handle and a finalizer per row. The reason the list has to be remembered at all is in the MAUI source:Element.SetParentclearsRealParentbeforeOnParentSetpropagates the window loss, so when a view learns its window is gone its parent is already null.TryParkalso built theParkingobject (three more weak references) before the watcher could refuse the view.Full reasoning, MAUI/AndroidX citations and measurements are in
review-follow-ups-research-2026-10-05.md, added here.What
ItemsHostWatcher(which already holds the list weakly) instead of its own weak reference. A staticItemsHostWatcher.Nonestands in for lists that have not opted in, andFindshort-circuits on a static counter while no list in the process has opted in.TryParklets the watcher take the slot first; the slot's weak reference and theParkingare created only once a view has actually been kept.LockinItemsHostWatcherstays: every library path is on the UI thread, but it guardsDisposeand limit changes from app code, and uncontended it is a CAS.Behaviour change: a list that opts in after its rows are bound now keeps each row from its next bind, not its next recycle (a row learns its list's watcher when bound). README and XML docs say to set the limit before the list shows rows. The #54 test for late opt-in is renamed and adjusted accordingly.
Measured
Verification
dotnet build Stellar.slnf -c Release: 0 errors;Stellar.UnitTests259/259;Stellar.Maui.UnitTests201/201 onnet10.0.🤖 Generated with Claude Code