Skip to content

fix(signals): A28 at the store backing — an unflushed write is invisible to owner-context readers until the carrying flush (S6) - #3526

Closed
ryansolid wants to merge 3 commits into
nextfrom
fix/store-a28-backing
Closed

ryansolid wants to merge 3 commits into
nextfrom
fix/store-a28-backing

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Stacked on #3525 (← #3523). The S6 fix, alone, so its perf and bytes are judged apart from the consolidation.

The bug (S6)

Core serve() serves an unflushed node's committed value and re-runs the reader in the carrying flush (A28). The store's backing selection served the pending backing to every owner-context reader at once. A derivation created inside a foreign action that adopted a mainline setStore published the unflushed 1; the signal published 0. Ruled 2026-09-17: store rules follow signal rules.

The fix

  • ensurePB stamps the target with clock when the draft opens outside a flush from outside any owner — A28 (4)'s promoted-write exemption (a projection derive's writes stay visible to its block).
  • pendingBackingVisible's owner-context branch serves committed for a backing stamped this tick and marks the reader late linker.
  • getNode gains a third born kind: a node born in the unflushed window stages the pending backing's value, so the carrying flush commits it through the node and the late-linked reader finds it there. stageHeldKey generalizes to one stageKey(node, nv, txn | null).
  • deep() / snapshot() keep their speculative-peek contract; nodeValue already has the arm through serve().

Gate

check result
signals suite 3011 pass; S6 flips from recorded divergence to parity pin (three shapes, with [0] → [0, 1] at the commit)
oracles identical
posture matrix exactly the 12 S6 cells move, to the signal's values; all six signal/store pairs 0 diffs
size +283 B minified store (was +402 at the old twin); one brotli cap ratcheted (+23 B)

Net with the consolidation PR: −30 B minified store, S6 closed.

Made with Cursor

@changeset-bot

changeset-bot Bot commented Sep 17, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4be36d0

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

@ryansolid
ryansolid changed the base branch from refactor/store-hold-visible to refactor/owns-hold September 17, 2026 20:35
@ryansolid
ryansolid force-pushed the fix/store-a28-backing branch from 2d222e7 to da012d1 Compare September 17, 2026 21:39
@ryansolid
ryansolid changed the base branch from refactor/owns-hold to next September 17, 2026 21:39
@ryansolid ryansolid closed this Sep 17, 2026
@ryansolid ryansolid reopened this Sep 17, 2026
@coveralls

coveralls commented Sep 17, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 35285034403

Coverage remained the same at 71.304%

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: 1035
Covered Lines: 783
Line Coverage: 75.65%
Relevant Branches: 798
Covered Branches: 524
Branch Coverage: 65.66%
Branches in Coverage %: Yes
Coverage Strength: 15.06 hits per line

💛 - Coveralls

@codspeed

codspeed Bot commented Sep 17, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 5.7%

⚡ 2 improved benchmarks
❌ 15 regressed benchmarks
✅ 155 untouched benchmarks
⏩ 3 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
class store object, toggled in place × 500 38.9 ms 44.9 ms -13.43%
style store object, mutated in place × 500 39.7 ms 45.5 ms -12.76%
reconcile: deep tree, 10 of ~12k paths subscribed 3.2 ms 3.6 ms -11.27%
store reconcile tree shuffle: 1111 keyed nodes 32.4 ms 35.6 ms -8.86%
style store object, replaced × 500 43.9 ms 47.9 ms -8.36%
reconcile: deep tree, all ~12k paths subscribed 72.8 ms 79.4 ms -8.3%
store reconcile tree reverse: 1111 keyed nodes 29 ms 31.5 ms -7.87%
dbmon full tick — deep reconcile 681.2 ms 730.3 ms -6.72%
spread store record × 500 31.9 ms 34.2 ms -6.71%
dbmon full tick 696.9 ms 746.3 ms -6.62%
dbmon partial tick — deep reconcile 710.7 ms 760.9 ms -6.61%
dbmon partial tick 724.2 ms 771.9 ms -6.19%
steady state: owned backings (#3360) 8 ms 8.5 ms -6.15%
class store object, replaced × 500 41.7 ms 44.3 ms -6.06%
dbmon full tick — shallow reconcile 533.2 ms 561.6 ms -5.06%
storeWrap:shallow 1k records 4.5 ms 3.6 ms +24.2%
storeWrap:deep 1k records 12.3 ms 11.6 ms +5.44%

Tip

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


Comparing fix/store-a28-backing (4be36d0) with next (dd908fc)

Open in CodSpeed

Footnotes

  1. 3 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 added a commit that referenced this pull request Sep 17, 2026
… write, in a WeakMap)

CodSpeed on #3526: -7…-19% on the store write floor. ensurePB runs on every
trap write of a draft, and the stamp sat before its pb === null branch — a
WeakMap.set per WRITE (batched: 1000 sets per commit), and a WeakMap.set on a
fresh target pays the identity hash plus the ephemeron. The stamp now sits
inside the branch, once per draft open beside queueFold, in a Map whose
entry leaves at the drain.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
ryansolid added a commit that referenced this pull request Sep 17, 2026
… write, in a WeakMap)

CodSpeed on #3526: -7…-19% on the store write floor. ensurePB runs on every
trap write of a draft, and the stamp sat before its pb === null branch — a
WeakMap.set per WRITE (batched: 1000 sets per commit), and a WeakMap.set on a
fresh target pays the identity hash plus the ephemeron. The stamp now sits
inside the branch, once per draft open beside queueFold, in a Map whose
entry leaves at the drain.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
@ryansolid
ryansolid force-pushed the fix/store-a28-backing branch from b32fc8c to e2ea639 Compare September 17, 2026 22:31
ryansolid and others added 3 commits September 17, 2026 15:59
…ble to owner-context readers until the carrying flush (S6; store follows signal)

Ruled 2026-09-17: store rules follow signal rules. Core serve() serves an
unflushed node's committed value and re-runs the reader in the carrying
flush; the store's backing selection served the pending backing to every
owner-context reader at once. A derivation created inside a foreign action
that adopted a mainline setStore published the unflushed 1; the signal
published 0.

Now: ensurePB stamps the target with clock when the draft opens outside a
flush from outside any owner (A28 (4) — a write inside a computation stays
visible to its block, core's notePromotedWrite twin); pendingBackingVisible's
owner-context branch serves committed for a backing stamped this tick and
marks the reader late linker; getNode gains the third born kind — a node
born in the unflushed window stages the pending backing's value — with
stageHeldKey generalized to one stageKey(node, nv, txn | null) (a hold's
staging in its transaction's list; the unflushed window's through
queuePendingNode, adopted-unflushed under a transaction body). deep() /
snapshot() keep their speculative-peek contract; nodeValue already has the
arm through serve().

Pinned: posture-store-parity S6 flips from a recorded divergence to a parity
pin (three shapes, with the commit follow-through). Matrix: exactly the 12
S6 cells move, to the signal's values; all six paired states 0 diffs.

+283 B minified store (down from +402 at the old twin); one brotli cap
ratcheted.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
… write, in a WeakMap)

CodSpeed on #3526: -7…-19% on the store write floor. ensurePB runs on every
trap write of a draft, and the stamp sat before its pb === null branch — a
WeakMap.set per WRITE (batched: 1000 sets per commit), and a WeakMap.set on a
fresh target pays the identity hash plus the ephemeron. The stamp now sits
inside the branch, once per draft open beside queueFold, in a Map whose
entry leaves at the drain.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
CodSpeed on #3526 after the per-draft Map: still -7…-9% on the store write
floor (style/class store objects mutated in place x500, fresh stores) — a
Map with 500–2000 set/delete cycles per commit churns its backing store.
The stamp is now `target.uf`, a plain field initialized in the target
literal (TargetShape / createTarget). The array shape rule in target.ts was
re-measured on Node 26: array and object targets stay fast-properties at
25–28 named+symbol props; CodSpeed's array benches guard it.

Local interleaved A/B against next on the write-floor and commit-boundary
benches is within noise (-3.5…+3.8%, base spread up to 10%).

Co-authored-by: Claude via Cursor <noreply@cursor.com>
@ryansolid
ryansolid force-pushed the fix/store-a28-backing branch from e2ea639 to 4be36d0 Compare September 17, 2026 23:03
@ryansolid

Copy link
Copy Markdown
Member Author

Not shipping for now. The fix is right by the ruling (store follows signal) but needs one bit of per-target state — "was this backing opened this tick, outside a flush, by imperative code" — and every home for it regressed a hot path on CodSpeed:

stamp write floor dbmon
WeakMap per trap write −7…−19% flat
Map per draft open −7…−9% (set/delete churn) flat
target field uf ~flat −5…−7% — the 24th named field flips array targets to dictionary mode on CI's V8 (the target.ts shape rule holds there; it did not reproduce on Node 26 locally)

Untried: a symbol on the pb container (free on the hot paths, but the symbol rides into the committed object and every Reflect.ownKeys over a backing has to skip it). Not worth the surface for this shape — an imperative write read by a derivation created inside a foreign action before the flush.

S6 stays pinned on next at the store's current value in posture-store-parity, ruled store-follows-signal and deferred; it closes for free if the store's value selection ever moves to one home, where the node carries the stamp. The branch stays for reference.

Claude via Cursor

@ryansolid ryansolid closed this Sep 18, 2026
ryansolid added a commit that referenced this pull request Sep 18, 2026
…6 not shipped (#3526: the three-stamp cost record), bytes through #3523

Co-authored-by: Claude via Cursor <noreply@cursor.com>
github-actions Bot pushed a commit to yumemi-thomas/solid that referenced this pull request Sep 18, 2026
…nistic

'a grouped boundary that errors still records' failed CI on solidjs#3526 with
heldMs 7.28 against a >= 10 floor: a 5 ms timer (the error) and a 20 ms
timer (the sibling) raced on a loaded runner, and the hold is the gap
between them. B now settles a fixed 30 ms AFTER A has failed (a deferred A
resolves before it throws), so the hold's lower bound is a single timer's —
a timer never fires early — and the floor is 25. Same shape for
'order=together': B is released 40 ms after A has settled, not 40 ms after
the stream started; floors 35 (held) and 40 (B's duration); A's duration is
asserted shorter than its own hold rather than under a wall-clock constant.

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