Skip to content

perf(signals): omit() copies folded filters by exact concat while short, chains them past 8 keys - #3487

Closed
ryansolid wants to merge 2 commits into
nextfrom
perf/omit-hidden-chain
Closed

ryansolid wants to merge 2 commits into
nextfrom
perf/omit-hidden-chain

Conversation

@ryansolid

@ryansolid ryansolid commented Sep 16, 2026

Copy link
Copy Markdown
Member

Part of the yak/Kobalte convergence work tracked in #3389 (the "slimmer omit/merge records" item). Companion to #3486, split so CodSpeed attributes each.

What

When an omit folds over another view — an omit of an omit, or of a merge with omit leaves — the two filters were combined into one key list per leaf, per layer, by slice() + push. V8 grows the backing store to 1.5n + 16 slots on that push, so each copy allocated 2.4–2.7× an exact one (that was #3475's final form; it's where an 11% allocation increase on the polymorphic-chain SSR case between that PR's morning build and its merged form came from, found while isolating #3486). On a component chain (defaults → omit → statics → omit …) the copies held every key hidden so far, growing with the depth.

Two lists now combine with concat — one builtin call, exact-size — while the result is within 8 keys. Past that, or with a predicate on either side, they chain: one two-field link over the filter folded, no copy. isHidden walks the links in one loop. A predicate filter no longer needs a closure to combine; a filter with nothing to hide adds nothing.

Why the limit (first push vs this one)

The first push chained from the first fold. It cut allocation −23% at depth 7 and TurboFan build −32%, but CodSpeed read the tier-1 polymorphic-chain SSR bench −10%: the walk per key of every leaf cost the bytecode tiers more than the copies had. That bench's four omits hide 1 + 1 + 2 + 1 keys, so under the limit every leaf on that shape stays one flat list and a check is one includes; a deep leaf on a longer chain is one short list plus a few links. A limit of 12 was measured and is worse in every tier.

Numbers

Tier-1 polymorphic-chain SSR bench (Kobalte shape, renderToString, 200 rows), harness driven in-process under each tier, min-of-5 vs next:

tier chain chain-static compiled (control)
interpreter −7% −7% flat
Sparkplug −12% −11% flat
TurboFan −17% −16% flat

Depth-7 props chain (15 hidden keys on the deepest leaf, so it does chain): TurboFan build −14%, build + consume −9%; interpreter/Sparkplug +3–5% — that's the residual cost of the walk past the copy limit, on the artificial shape only. Depth 3 is flat in every tier.

The omit-static(5, 1, 2) reading on the first push was on a construct-only path that is byte-identical to next; same class of runner noise as the merge-proxy-keys-store readings on #3475.

2370 signals tests pass; a test pins the copy/chain rule across lists, predicates, empty omits and the 8-key boundary.

When an omit folds over another view — an omit of an omit, or of a merge
with omit leaves — the two filters were combined into one key list per
leaf, per layer. On a component chain (defaults -> omit -> statics ->
omit ...) each copy held every key hidden so far, growing with the depth:
at depth 7 the copies were the largest allocation of the views, 100-450
bytes a leaf. #3475's final form, slice() + push, made that worse than
concat: V8 grows the backing store to n + n/2 + 16 slots on the push past
capacity, 2.4-2.7x the bytes of an exact copy — which is where an 11%
allocation increase on the polymorphic-chain SSR case between that PR's
morning build and its merged form came from.

A filter is now one two-field link (inner filter, own filter) over the
filter it folds. hides() walks the links in one loop — a link is told
from a list or a predicate by its constructor, a load and a compare, since
instanceof and Array.isArray are builtin calls in the bytecode tiers —
and does the same includes work the combined list did. A filter with
nothing to hide adds no link; a predicate filter no longer needs a closure
to combine; a chain arriving as the outer filter is re-hung link by link
so every link's own filter stays atomic. The no-Proxy path is untouched:
no views exist there, so the filter is the caller's list or predicate.

Depth-7 Kobalte-shaped chain, min-of-N: TurboFan build -32%, build +
consume -11%, Maglev consume -11%; sampled allocation -23%. The
polymorphic-chain SSR case allocates 17% less per instance (24.7 -> 20.4
KB). In the interpreter and Sparkplug the check-heavy consume is +5-9%:
one includes builtin over a flat list is the cheapest possible check in
those tiers, and a walk over several links cannot match it. A per-leaf
Set flatten at table-build time was tried and made the interpreter worse
(the flatten loop and Set.has per key cost more than the walk), so the
trade is taken as is, for the optimized tiers and the allocation rate a
server sees.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 93e5359

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@solidjs/signals Patch
test-integration Patch
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
solid-js Patch
@solidjs/universal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coveralls

coveralls commented Sep 16, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 35071772842

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage remained the same at 71.46%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1018
Covered Lines: 772
Line Coverage: 75.83%
Relevant Branches: 790
Covered Branches: 520
Branch Coverage: 65.82%
Branches in Coverage %: Yes
Coverage Strength: 14.93 hits per line

💛 - Coveralls

@codspeed

codspeed Bot commented Sep 16, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 11.56%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 6 regressed benchmarks
✅ 168 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
omit 25.8 µs 39.8 µs -35.13%
polymorphic-chain: 200 rows (renderToString): chain 22.9 ms 26.8 ms -14.47%
polymorphic-chain: 200 rows (renderToString): chain-static 22.6 ms 26.4 ms -14.26%
build 280.6 µs 308.9 µs -9.18%
build + consume 837.8 µs 897.1 µs -6.61%
readBlocked 14.9 µs 15.8 µs -5.2%
projection derive: write one NESTED field (reference) 2.8 ms 2.6 ms +10.69%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing perf/omit-hidden-chain (93e5359) with next (28c65be)

Open in CodSpeed

… only past 8 keys

A chain from the first fold cut allocation by a quarter at depth 7 and
build time by a third under TurboFan, but the walk per key of every leaf
cost the bytecode tiers more than the copies had: CodSpeed read the
tier-1 polymorphic-chain SSR bench 10% worse, and omit-static reads paid
an extra call layer for a case with no chain at all.

Two lists now combine with concat — one builtin call, exact-size (the
slice() + push it replaces grew the backing store to 1.5n + 16 slots) —
while the result is within 8 keys; past that, or with a predicate on
either side, they chain as before. The tier-1 bench's four omits hide
1 + 1 + 2 + 1 keys, so every leaf on that shape stays one flat list and a
check is one includes; a deep leaf on a longer chain is one short list
plus a few links. isHidden is the walk itself (no second call layer), and
combineHidden tells a list, a predicate and a link apart by typeof and
constructor rather than Array.isArray / instanceof, which are builtin
calls in the bytecode tiers.

Tier-1 polymorphic-chain SSR bench (Kobalte shape, renderToString, 200
rows), the harness driven in-process under each tier, min of 5 vs next:
interpreter -7%, Sparkplug -12%, TurboFan -17%; the compiled control is
flat. Depth-7 props chain: TurboFan build -14%, build + consume -9%;
interpreter and Sparkplug +3-5%. A limit of 12 was measured and is worse
in every tier (more copies, less chain).

Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid ryansolid changed the title perf(signals): omit() chains folded filters instead of copying them perf(signals): omit() copies folded filters by exact concat while short, chains them past 8 keys Sep 16, 2026
@ryansolid

Copy link
Copy Markdown
Member Author

Correction — the numbers in the description are wrong, and CodSpeed is right.

Every local "vs next" measurement behind this PR resolved @solidjs/signals through the main worktree's dist, which was built at 15:30 — before #3475 landed on next at 15:53. So the baseline was the pre-#3475 code (concat + closure), not next (slice() + push). Against a freshly built next (5fa224a, both trees rebuilt, tier-1 harness driven in-process under each V8 tier, min-of-5):

variant tier-1 chain TurboFan tier-1 interp depth-7 build TurboFan depth-7 interp depth-7 alloc
next (slice+push) 0.95 ms 4.75 ms 6.2 µs 40.6 KB
this PR (concat ≤ 8, chain past) +34% +5% +16% +5% 24.1 KB
slice+push ≤ 8, chain past +4% −8% +4% 30.7 KB
exact preallocated copy, no chain +3% −9% +10% 28.8 KB

Two findings:

  1. concat is the regression, not the chain. At depth 3 no chain is ever built and build is still +16%: Array.prototype.concat is a C++ builtin TurboFan cannot inline, while slice/push are CSA builtins it inlines. My standalone microbench had shown concat faster because neither inlines in an isolated loop.
  2. slice() + push is the instruction-count floor for this operation in the bytecode tiers, which is what CodSpeed's simulation weights. Every alternative — a chain walk, an exact-size preallocated copy, concat — costs more instructions per fold or per check, even where it allocates 25–40% less and runs faster under TurboFan. On the tier-1 Kobalte shape the TurboFan gains are nil (its leaves hold ≤ 5 keys); the only TurboFan wins are on the artificial depth-7 chain.

Recommendation: close without merging. The allocation motivation does not turn into throughput on realistic shapes in any tier, and the description's tier table should be disregarded.

Claude via Cursor

@ryansolid

Copy link
Copy Markdown
Member Author

Closing without merging per the measurements above: against a correctly built next, no variant of the fold (chain, exact copy, concat) beats slice() + push on instruction count, and the TurboFan wins only appear on the artificial depth-7 chain. The allocation growth it targeted is real but costs no throughput in any tier.

Claude via Cursor

@ryansolid ryansolid closed this Sep 16, 2026
ryansolid added a commit that referenced this pull request Sep 16, 2026
…ord per layer, one-pass owners walk for ssrElement

An omit over a merge flattened at construction: one OmitView plus one
combined hidden-key list per flattened leaf, and the next merge() copied
those entries into its arrays — on a Kobalte-shaped chain (defaults +
omit + spread, four layers) ~19 records and as many list copies per
element, the largest allocation of the render. #3487 tried to make those
copies cheaper and could not beat slice+push on instruction count; this
does not make them.

The omit now holds the MergeView record itself (new source kind
SOURCE_MERGE) and is one record whatever the merge's leaf count; a later
merge() carries it as one entry, a later omit() folds into it. Nothing on
the way is a trap: sourceKeys/sourceHas/sourceGet, descriptors,
hasStaticKeys and the tables recurse into the record by function call —
the property #3454 established (consumers read the leaves, never through
a proxy) is kept, the per-leaf copies are not.

Three things had to hold for it to pay, each found by measurement:
- one walk per read: a nested entry answers presence and value together
  (MISSING sentinel), not has-then-get per level;
- a record reached through an outer view counts no reads toward its own
  table threshold, and the outer view's table is collected in one pass
  over the leaves (collectTable) — not one table per layer;
- sourceOwners(source, keys, owners): every key of a plain object, store
  or view in merged order with its owning object, one pass. ssrElement
  collects any non-plain spread (a view, a store, the array form with one
  among them) this way and reads owners[i][keys[i]] — the flat form's
  read cost without its construction cost. pushEntry is gone.

An omit's $SOURCES answers nothing now; consumers reach the record via
viewOf. Reads through an omit no longer count on the inner merge: the
view that was asked decides for the tree.

Measured against next (interleaved, min of N, quiet machine): tier-1
polymorphic-chain SSR −12% bytes/row, −2…−6% time across interp,
Sparkplug, Maglev, TurboFan; props-chain build −6…−65%, build+consume
−7…−31% by depth and tier; omit/merge micro-suite flat or better in
every shape; yak-bench SSR all-primitives lane +7% geomean, +20–36% on
the composition cases, which reach parity with yak's hand-rolled runtime.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
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.

2 participants