Avoid redundant anchor lookups in Fragment.anchor() and unkeyed v-for - #188
Merged
Conversation
Adds a bench/ suite (element creation, component mount depth, attribute update at depth, keyed/unkeyed v-for reconciliation, and unkeyed list grow/shrink) using pytest-benchmark with DictRenderer and synchronous event loop, mirroring the setup in the sibling observ repo. The new benchmark workflow runs the suite twice on every PR - once against the master version of collagraph/ and once against the PR version - and fails when mean time regresses more than 5%. The CI test job is scoped to the tests directory so the matrix does not execute benchmarks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The allocation burst of list reconciliation triggers collection pauses in some rounds, which inflated stddev on the grow benchmarks to ~40% of the mean - far too noisy for the 5% regression gate in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- weak(): read __code__.co_argcount instead of building a full inspect.Signature for every wrapped watcher callback; this runs for every dynamic bind during create/mount. - ComponentFragment.mount(): use the existing first() helper to resolve the component root element instead of a hand-rolled BFS with a per-mount deque import. Adds a regression test for a component whose root element sits behind a v-if wrapper. - Component.emit(): avoid creating defaultdict entries for events that have no handlers and skip the set copy when empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The parent-chain walk ran on every reactive attribute update (via _set_attr/_rem_attr calling component.updated()) and on every ref (un)registration, making update cost scale with fragment depth. The result is now cached per fragment. The cache is invalidated in the parent property setter, on unmount(destroy=True), and - via a new recursive helper - after DynamicFragment._create_fragment_for_tag() transfers live children to a new active fragment on a <component :is> tag switch, which reassigns _parent directly and changes the owning component of every transferred descendant. Two regression tests cover the tag-switch reparenting: updated() attribution and string-ref registration must follow the new owner. Both fail when the invalidation hook is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- anchor(): a membership check followed by .index() scanned the parent's children twice; use a single .index() in a try/except, for both children and slot_contents. - Unkeyed ListFragment updates mounted every appended fragment with a freshly computed self.anchor(), which walks the tree per item even though the anchor (the first element after the list) cannot change while appending items from the list's own subtree. Compute it lazily once per update run, turning growth by n items from O(n^2) anchor work into O(n). Same hoist in ListFragment.mount(). Adds a regression test asserting that growing an unkeyed v-for with a trailing sibling inserts elements in order, each anchored before that sibling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
berendkleinhaneveld
added a commit
that referenced
this pull request
Jul 7, 2026
Resolves conflicts in fragment.py by porting the perf work from master (PRs #186-#188) onto the two-tree fragment model: - Keep both _render_parent (branch) and _component_parent_cache (master) on Fragment. - anchor() keeps the unified render_children() traversal, which subsumes master's separate children/slot_contents lookups. - ListFragment.mount() keeps the lazy loop-invariant anchor from master, applied to _generated_fragments. - ComponentFragment.mount() uses master's simpler `self.component._element = self.first()`. - _invalidate_component_parent_cache() traverses via iter_all_children() instead of the removed children/slot_contents attributes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
berendkleinhaneveld
added a commit
that referenced
this pull request
Jul 21, 2026
Features: - Add pure-Python view API as alternative to cgx templates (#193) - Support text elements for PySide widgets that display text (#191) Fixes & internals: - Fragment parenting overhaul (#162) - Fix PyInstaller hook for CGX files inside packages (#184) - Write compiled AST to temp file when CGX_DEBUG is set (#175) Performance: - Speed up mount path: cheap arity check, reuse first(), leaner emit (#186) - Cache Fragment._component_parent() lookups (#187) - Avoid redundant anchor lookups in Fragment.anchor() and unkeyed v-for (#188) Documentation: - Add MkDocs documentation with GitHub Pages deployment (#176) - Add internals architecture documentation page (#194) - Add docs badge and links to README (#192) Tooling & CI: - Add benchmark suite and per-PR benchmark CI workflow (#185) - Make benchmark CI guard robust against run-to-run noise (#196) - Update GitHub actions from Node 20 to Node 24 (#190) - Migrate from pre-commit to prek (#195) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Summary
Fourth PR of the performance series (stacked on #187 → #186 → #185).
Fragment.anchor(): replaced theself in parent.childrenmembership check followed by.index(self)(two O(n) scans) with a single.index()in atry/except ValueError, for both thechildrenandslot_contentsbranches. Behavior-identical, half the scanning.ListFragmentupdates: every appended fragment was mounted with a freshly computedself.anchor(). The anchor (the first element after the list) is loop-invariant while appending items from the list's own subtree, so it is now computed lazily once per update run. Same hoist inListFragment.mount().v-forwith a trailing sibling inserts elements in order, each anchored before that sibling.Honest benchmark result: no measurable gain
The plan expected the per-item
anchor()recomputation to be O(n²) for growing lists, but measurement disproves that premise:anchor()scans the template-level sibling list (a handful of fragments), not the growing item list, so the per-item cost was a small constant. With the GC-stabilized benchmarks,unkeyed_list_grow[1k]is 24.97ms → 24.65ms (within noise); growth cost is dominated by fragment/watcher creation, not anchor lookups.The change still removes strictly redundant work, guards the adversarial case (a
v-forpositioned after many template siblings, where the saved scan does scale), and simplifies reasoning about the update loop — but on the current benchmark scenarios it is performance-neutral. Fine to merge on those merits or close; measurement says this is not where the time goes.For the record, the measurements also show where unkeyed-list time does go: ~2.4ms per 100 items is fragment creation (reactive context + watcher setup per item), and the
siblingsvariant costs ~17% extra at 1k purely fromDictRenderer.insert's O(n)children.index(anchor)— that renderer-side cost would be the target of a future batch-insert (insert_many) API discussed in the plan.Test plan
uv run pytest tests -q— 353 passed, 1 skipped (new regression test included)uv run ruff check ./uv run ruff format --check .— cleanuv run pytest bench --benchmark-only --benchmark-compare— all groups within noise, no regressions🤖 Generated with Claude Code