Skip to content

perf(bench): fix 10K fixture isolation in createNested and createSortable#272

Merged
johnleider merged 2 commits into
vuetifyjs:masterfrom
sridhar-3009:fix/bench-fixture-isolation-259
Jun 5, 2026
Merged

perf(bench): fix 10K fixture isolation in createNested and createSortable#272
johnleider merged 2 commits into
vuetifyjs:masterfrom
sridhar-3009:fix/bench-fixture-isolation-259

Conversation

@sridhar-3009
Copy link
Copy Markdown
Contributor

Summary

Closes #259.

Ten-thousand-item mutation benches were folding O(n) onboard setup into every measured iteration. Since onboarding 10K items costs ~16ms (same order as the operation under test), the measured time was dominated by fixture setup rather than the operation being benchmarked. This caused inaccurate 'slow' tier badges in the docs metrics pipeline.

Changes

createNested/index.bench.ts

open/close operations (10K):

  • open() on an already-open node and expandAll() on an already-expanded tree are idempotent at state level — Set.add() is a no-op when the id is already present.
  • collapseAll() on an already-collapsed tree spreads an empty openedIds set and does nothing.
  • Replaced fresh-per-iteration 10K fixture with a shared pre-built fixture per describe block. Measures the O(depth) guard check / O(n) scan in isolation.

selection operations (10K):

  • selectAll() and select(root) on an already-fully-selected tree cause no state changes but still perform the full O(n) descendant traversal.
  • Replaced fresh-per-iteration 10K fixture with a shared pre-selected fixture. Measures cascade traversal cost without onboard setup noise.

createSortable/index.bench.ts

move operations (10K):

  • Shared fixtures with alternating targets: odd iterations move forward, even iterations restore (e.g. index 5000 → 5001 → 5000 → …). Both directions have identical O(n) map-rebuild cost.

swap operations (10K):

  • swap(A, B) is self-inverting: two consecutive calls return to the original state. One shared fixture, called per iteration.

reorder operations (10K):

  • Alternating between PERMUTATION_10K (reversed) and PERMUTATION_10K_ORIG (original order). Each direction is a full O(n) reindex.

events overhead (10K):

  • Same alternating-move pattern as move operations. Listener remains attached across iterations so the no-listener vs. listener comparison stays valid.

Approach

Per .claude/rules/benchmarks.md:

  • Read-only operations use shared fixtures (already correct for 1K benches).
  • Mutation operations that are idempotent (no state change when applied to already-settled state) now use shared fixtures for the 10K case.
  • State-mutating operations that cannot be made idempotent use self-inverting call pairs (swap, alternating move targets, alternating reorder permutations) — each pair of iterations returns to the original state with identical cost in both directions.
  • 1K benches retain fresh-per-iteration fixtures (setup overhead is acceptable at that scale).

…able

Ten-thousand-item mutation benches were folding O(n) onboard setup into
every measured iteration, masking the actual operation cost and
inflating 'slow' tier badges in the docs metrics pipeline.

createNested open/close operations:
- Open/expandAll on already-open nodes and collapseAll on already-closed
  nodes are idempotent at state level. Share a pre-built fixture per
  describe block instead of rebuilding 10K nodes each iteration.

createNested selection operations:
- selectAll/select on already-selected items cause no state changes but
  still do the full O(n) descendant traversal. Share a pre-selected
  fixture so the measured cost is the cascade scan, not onboard setup.

createSortable move/swap/reorder operations:
- swap is self-inverting (A↔B then B↔A = identity): use one shared
  fixture, call swap per iteration.
- move and reorder use alternating targets (forward/backward), also
  self-inverting: each pair of iterations returns to the original state.
  Both directions have identical O(n) map-rebuild cost.
- events overhead 10K benches follow the same alternating-move pattern.

Fixes vuetifyjs#259
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 5, 2026

Open in StackBlitz

commit: d8fb727

@johnleider johnleider added this to the v1.0.0 milestone Jun 5, 2026
@johnleider johnleider self-requested a review June 5, 2026 18:10
The shared collapsed fixture left openedIds empty, so collapseAll spread an
empty set and measured nothing (~0.001ms / 889K hz) — ~4000x faster than the
1k sibling, inverting its tier badge. Alternate collapse<->expand on a
dedicated fixture so every iteration runs a full O(n) pass; now ~0.29ms, in
line with expandAll(10k).
@johnleider
Copy link
Copy Markdown
Member

Review

Thanks for this — the diagnosis is correct and the approach is sound. I traced the source to confirm the idempotency claims, then ran both bench files before/after to measure the actual effect. The sortable half is solid; the nested half has one real defect: Collapse all (10,000) now measures an empty set instead of the operation.

✅ Verified correct

I confirmed in source that open / expandAll / select / selectAll and move / swap / reorder all run their full O(n)/O(depth) bodies with no early-return on settled state, so shared fixtures genuinely isolate operation cost. The onboard you removed was even bigger than the "~16ms" estimate — for the depth-4 tree it's ~48ms, which is what was dominating the 10K numbers.

Local before/after (illustrative, single machine):

createNested 10K before after
Expand all 48.6 ms 0.22 ms ✅ real O(n)
Select root cascade 48.5 ms 0.56 ms
Select all 54.4 ms 1.63 ms
Collapse all 46.6 ms 0.001 ms ❌ no-op (see below)

Sortable after-numbers are all sane (move 0.81–1.66ms, swap 2.07ms, reorder 3.32ms; no degenerate values). Two subtle things you got right and worth calling out: the self-inverting move/swap/reorder pattern keeps every iteration doing genuine work, and because move() only emits move:ticket on an actual index change, the alternating targets keep the emit path live — so the no-listener vs listener comparison stays valid. The listener is also correctly attached once. 👍

🔴 Collapse all (10,000) measures nothing

The fixture is tree10kCollapsed = createPopulatedNested(TREE_10K) — i.e. openedIds is empty. collapseAll() iterates [...openedIds], so on an empty set it spreads nothing, filters nothing, deletes nothing. It clocks ~0.001 ms (889K hz) — ~200× faster than expandAll on the same data and ~4,000× faster than the 1K collapseAll, which you (correctly) left on a fresh expanded fixture doing real work. The matrix will then show 10K-collapse as blazing and 1K-collapse as slow — the operation appearing to speed up as the tree grows, which is the misleading tier this PR is meant to remove, inverted.

The asymmetry: expandAll iterates children (structural map, ~11,111 entries regardless of open state) → idempotent with work → sharing an expanded fixture is correct. collapseAll iterates openedIds (variable, 0 when collapsed) → sharing a collapsed fixture zeroes the work. The PR description names this ("does nothing"), but for a benchmark "does nothing" = "measures nothing."

Fix: measure collapseAll on a populated set. Since it mutates to empty and can't be idempotent on one shared fixture, the cleanest option consistent with your own sortable approach is a self-inverting collapseAllexpandAll alternation (both O(n), no setup cost). I've pushed that to the branch — see the follow-up commit.

Minor

  • Open single node (10,000) on the expanded fixture measures the already-open fast path (Set.add of a present id skips the reactivity trigger). Microseconds either way for a single node, so the number isn't misleading — just noting the "open a node" intent never actually opens one. Low priority.

Out of scope, don't forget

  • The tier badges won't move from this PR alone. They're driven by committed apps/docs/src/data/metrics.json + apps/docs/public/benchmarks.json, regenerated in standalone chore: regenerate benchmark and metrics data commits on a canonical runner. This PR is right not to commit a contributor's hardware numbers — but a maintainer needs to run pnpm metrics and land the follow-up for the badge fix to actually take effect.
  • Commit type: .bench.ts isn't shipped and nothing in the package got faster, so perf(bench): will surface a spurious entry in the release notes. Suggest chore(bench): (or test(bench):).

Recommendation

One substantive change needed (Collapse all (10,000) against a populated tree — pushed) and a re-title to chore(bench):. Everything else is good to go.

@johnleider johnleider merged commit b4a4bf8 into vuetifyjs:master Jun 5, 2026
13 checks passed
@johnleider
Copy link
Copy Markdown
Member

Thank you for your contribution and interest in improving Vuetify!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] benchmarks: mutation benches fold O(n) onboard setup into measured iterations, inflating 'slow' tiers

2 participants