Skip to content

perf(store): skip per-call allocations on the store setter path - #4635

Closed
brenelz wants to merge 4 commits into
solidjs:nextfrom
brenelz:perf/store-write-alloc
Closed

brenelz wants to merge 4 commits into
solidjs:nextfrom
brenelz:perf/store-write-alloc

Conversation

@brenelz

@brenelz brenelz commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Every store setter call allocated at least two Sets: writeScopes = new Set() for the write scope, and a written-keys Set per record it touched in touch(). Most setters write one store and one key, so on the single-key write path these Sets were most of the per-call garbage.

  • writeScope now holds the setter's scope in a plain variable. The writeScopes Set is created only when a child proxy from another store joins the draft (draftServe). inDraft, the get trap and the shallow-view check read writeScope first.
  • t.wk now holds a bare key for a setter's first write to a record. A second key creates the Set. When a later setter in the same batch writes the record, the chain tail u can be that bare key. The three readers that walk written keys handle both forms: overlayRebuilds, the privatize branch of foldTarget, and notifyWrites (which passes the list on to reportReplacedContainers).
  • Array writes always touch the index and then length, so they still create the Set. It is seeded with .add().add() instead of an array literal, so no temporary array is allocated.

I also tried swapping pendingNotify with a spare Set instead of spreading it into an array and clearing it. I left it out: it cost 40 B minified for a 2–4% gain, because V8's Set.clear() allocates a new backing table anyway.

Numbers

Local prod builds of @solidjs/signals, next (456e6d5) and this branch loaded in one process and run interleaved, 5 rounds, median ms per iteration. The machine was under load from other jobs, so the base-vs-base column shows the noise level.

bench next this PR change base vs base
write-floor (2,000 one-key stores, one write each, then flush) 0.506 0.385 −22.7% +1.0%
input burst (200 one-key writes, each flushed) 0.427 0.326 −13.8% −5.1%
array write (500 four-item arrays, one index each) 0.164 0.153 −7% —
store root write (20,000-key record) 0.032 0.029 −10.9% −4.5%
projection root write 0.045 0.043 −4.8% 0.0%
projection nested write 0.038 0.039 −3.3% +0.7%
selection toggle (1,000 keyed readers) 0.386 0.398 +3.1% −0.5%

An earlier run on the same base gave write-floor −21.5%, input burst −8.8% and selection toggle −2.7%. The selection toggle difference is within noise.

Size

node size.mjs --no-gate --json for head and base, then node gate.mjs head.json base.json, measured after merging next at b656418. Four scenarios that include stores grew. None is a frozen floor, so this PR raises their caps and recorded minified sizes, with a ledger line in scenarios.js for each.

scenario minified brotli cap
signals: + createStore +182 B +66 B 15.03 KB to 15.09 KB
app: hydrating + every store primitive family +188 B +85 B 33.15 KB to 33.24 KB
app: compiled CSR +188 B +68 B 26.61 KB to 26.68 KB
app: compiled hydrating +188 B +76 B 32.87 KB to 32.93 KB

Each new cap is the measured brotli size plus 10 B, rounded up to 0.01 KB, and each recorded minified size is the measured one. With the new caps, the gate passes.

How did you test this change?

  • pnpm build, then npx vitest run in packages/signals: 474 files, 6437 tests pass. tsc reports no errors in src or in the new test.
  • The new test, tests/store/written-key-chain-privatize.test.ts, covers the privatize branch of the fold when the written-key chain ends in a bare key. To reach that branch, the descendant has to be written first in the batch so that its fold path-copies the row. Both cases fail if writtenList drops bare keys.
  • One existing test, createStore.test.ts "wrapped nested class", fails if the joined-scope Set is never created, so the writeScopes path is covered.
  • npx vitest run in packages/web: 242 files, 1702 tests pass.
  • A local differential fuzz of multi-setter batches checked the final store against a plain-object model over 3 seeds of 3,000 trials each, with 0 mismatches on both next and this branch. The fuzz is not part of the PR.

🤖 Generated with Claude Code

@changeset-bot

changeset-bot Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f6e773a

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

This PR includes changesets to release 15 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
board-example Patch
canvas-example Patch
server-dbmon-example Patch
todos-server-example 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

@codspeed

codspeed Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

2 warnings

⚠️ 2 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ 5 benchmarks with a significant change ran on different hardware

Their base and head results were measured on machines with a different CPU, core count or memory, so the changes might not come from the code. Run base and head on the same runner type.

Learn more about Macro Runners

See affected benchmarks

⚡ 4 improved benchmarks
❌ 1 regressed benchmark
✅ 196 untouched benchmarks
⏩ 5 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ dbmon partial tick 46.6 ms 49.1 ms -5.11%
⚡ projection derive: write one NESTED field (reference) 2.4 ms 2.1 ms +14.67%
⚡ steady state: owned backings (#3360) 6.5 ms 5.7 ms +13.69%
⚡ fresh stores: create + first write + first commit (reporter shape) 15.5 ms 13.9 ms +10.97%
⚡ memo + sync render effect only (reference) 33.3 ms 31.7 ms +5.06%

Tip

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


Comparing brenelz:perf/store-write-alloc (f6e773a) with next (b656418)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@ryansolid

ryansolid commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Thanks for digging into this. We ran it on the Octane board against next (Chromium headless, Solid's own per-op scores, two interleaved rounds; this PR's packages/signals/src changes applied onto next b656418), on the five suites whose Solid fixtures use stores:

suite this PR next round to round the store-write ops
store-selector-fanout +3.6% 2.3% store_write +1.2%, store_write_after_rerenders −0.9%
js-framework +0.9% −1.8% update 0.0% (−5.6%, +5.9%)
dbmon −0.4% −1.1% tick +1.6%, tick_partial +1.6%
effectful-list −8.8% −1.0% driven by a single clock-limited update_nodeps reading
svg-dashboard −0.5% −0.3% flat

The single-key setter calls this targets (store_write, js-framework update) come out flat. Once a write's reactive work and DOM writes follow it, the two short-lived Sets per call don't show, and V8's young-generation GC is cheap for exactly that kind of garbage. The write-floor and input-burst numbers are real, but they measure setter calls with nothing behind them.

Against that, it costs ~+190 B minified across four store scenarios (with their caps raised), and wk becomes a key-or-Set union that three readers in the fold path (overlayRebuilds, foldTarget's privatize branch, notifyWrites) must all handle. That path has been a source of store bugs before.

So we'll pass on this one.

🤖 Generated with Claude Code

@brenelz brenelz closed this Nov 3, 2026
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