solidjs / solidjs/solid

Under a parked transaction, an owner that recreates a child runs twice per mainline write (zombie rerun notifies through the A30-kept tail before the commit trims it)

Open
#3,546 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
TypeScript
Stars
36.1k
Forks
1.1k
Avg merge
9h 18m
Merged PRs (30d)
195

Description

Found while fixing #3543. Not a leak, values are identical, but it is a 2× cost on a common shape for as long as any action is pending.

What happens

While any transaction is parked, an owner that recreates a child each pass — a compiled <Show when={n() > 0 && n() < 2}> condition, whose getter is memo(() => n() > 0)() && n() < 2 — runs twice per mainline write, creating and disposing an extra child each time. The zombie child itself runs once.

Per write to n, in the ambient flush (activeTransition === null, transitions.size > 0), from an instrumented run of packages/signals/tests/zombie-recompute-keeps-flag.test.ts:

  1. runHeap(dirtyQueue) (scheduler.ts:796): the owner P recomputes first (a child the owner creates doesn't lift the owner's height, core.ts:1962). It zombifies the old child C1 — still DIRTY|IN_HEAP from the write, so markDisposal migrates it to zombieQueue — creates C2, stages, queuePendingNode(P).
  2. runHeap(zombieQueue) (scheduler.ts:875): C1 reruns as a zombie. Its value changed, so insertSubs notifies its subscribers — and P is still one, because A30 (#3469, 27b24aa06) keeps a staged pass's previous dep tail linked until the commit trims it.
  3. finalizePureQueue(): commitPendingNodes() disposes C1 and trims P's tail — after P is already dirty. The runHeap at scheduler.ts:1323 recomputes P again with identical inputs: zombifies C2, creates C3, commits again.

Measured on that test: 40 alternating writes cost 60 nested-memo runs with no transaction parked and 100 with one parked — the extra 40 are 20 zombie reruns plus 20 second-pass creations. Both are waste: P's staged value from step 1 already reflects the new n, and nothing ever renders C1's rerun.

Why A30 is not the problem

A30 is right to keep the tail: at recompute time nobody knows whether this flush will park, and if it does, the committed frame (which derives from C1) stays on screen and must keep receiving writes (#3469, #3410). The tail dep in #3469 is a live signal written in a later flush — that is the case A30 exists for. Here the tail dep is a zombie that reruns inside the same flush, a few lines before the commit that disposes it. Trimming earlier reopens #3469.

Proposal

Zombies whose owner commits this flush should not rerun before that commit. Concretely, in the ambient-with-parked-transactions branch, commit pending nodes before running the zombie queue (or run the zombie queue after finalizePureQueue and before the effect phases). A zombie whose owner commits is disposed by the commit and never reruns — which is already what happens when no transaction is parked (zombieQueue isn't run at all). Only zombies of actually-parked owners rerun, which is what :875 is for (#2916, #3463), and their owners' tails are legitimately kept.

Alternative if the ordering is load-bearing elsewhere: in insertSubs, skip a subscriber whose link to the notifying node lies past its _depsTail (a held-trim tail) when the notifier is a zombie — the notification is to a frame this flush is replacing, not to the live pass. Narrower, but it leaves the pointless zombie rerun in place.

Acceptance: the run-count bound in zombie-recompute-keeps-flag.test.ts (controlRuns + 40, currently exact) tightens to controlRuns; lane-outside-view.test.ts (#3463) and transition-orphan-recompute.test.ts ("still updates zombies for mainline writes in a separate flush") stay green.

Related: #3543 (the flag loss this surfaced), #3469 / A30, #3463, #3410.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the ambient parked-transaction paths in packages/signals/src/scheduler.ts, especially runHeap and finalizePureQueue, and review the owner-height behavior in core.ts. Run packages/signals/tests/zombie-recompute-keeps-flag.test.ts, then compare the related lane-outside-view.test.ts and transition-orphan-recompute.test.ts cases. Done means the run-count bound reaches controlRuns while both related zombie-update tests remain green.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.