fix(parallel): forget a result slot's local view once nothing can read it - #29
Merged
Merged
Conversation
…d it SlotTable::open() put a ResultSlot in $slots and nothing ever took one out, so a spawnParallel()->await() loop retained one per spawn - about 220 bytes with its array bucket - for the whole life of the run. Nothing reports it: the arena watermark is byte-flat because a published task and an integer result allocate nothing, and memory_get_usage(true) is flat because 2 MiB chunk granularity swallows it for hundreds of rounds. The only symptom is the parent's RSS climbing, which is what issue #24 measured (24 KiB over 40 rounds, 192 KiB over 120) and read as native memory outside the PHP heap. It is not native: memory_get_usage(FALSE) climbs 1.76 KiB per 8-spawn round, which is the retention exactly. The answer was never in the entry - it is in shared memory - so the entry is worth keeping only while something in this process can still ask for it. open() and adopt() now hand out a claim, JoinHandle carries it and releases it in its destructor, and a settled slot with no claims and no waiters is dropped. A pending slot is never dropped, whatever its claim count, because failPendingOf() still has to be able to turn it into a throw. The shared slot keeps its answer and its id; adopt() takes a fresh view of it whenever one is wanted again. Measured on tools/soak-arena-watermark.php, tolerance unchanged at 4 KiB: RSS climb 24.0 KiB -> 12.0 KiB at 40 rounds. The remainder was the substrate faulting its pre-sized slot table in a page at a time and is fixed there; with both, the soak reports a plateau at 40 and at 120 rounds and --self-test still FAILs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
lisachenko
marked this pull request as ready for review
August 16, 2026 09:24
lisachenko
pushed a commit
that referenced
this pull request
Aug 16, 2026
Unifies this branch's shared-record recycling with #29's local-view claim counting, which landed while this was in flight and solved the other half of the same lifecycle. #27 and #28 merged through with two expectation updates. ## One counter, two lifecycles #29 counted claims to decide when to drop a slot's LOCAL VIEW; this branch counted handles to decide when to give the SHARED RECORD back. Two mechanisms measuring the same thing. They are now one - `ResultSlot::$claims`, #29's name kept - and `forgetIfSettled()` is where the single decision fans out: claims == 0 && complete && no waiters -> the local view is dropped, ALWAYS, whatever this process's role -> the shared record goes back only if $owned && $sharedSettled The distinction is the point. The view is per-process bookkeeping and its only cost is memory; the record is family-wide state and giving it back is a claim that nobody will read it through this ticket again. An adopter drops its view and releases nothing. A slot whose worker died drops its view and keeps its record out of circulation, because the record is still pending in shared memory where a zombie may yet write to it. `JoinHandle` now gives its one claim back at the FIRST of two moments - awaited, or collected - guarded by `relinquish()` so the pair can never count as two. Awaiting releases at once, which is stronger than #29's release-at-collection: a batch that spawns N and awaits them in a loop is down to one live slot by the end of the loop rather than N until the handles array dies. The destructor still carries the case it was added for, a handle nobody awaits. ## The decision #29 could not have had to make A settled, never-awaited handle now recycles its shared record and DISCARDS the answer. The alternative - keep it until teardown - is the exhaustion bug itself for fire-and-forget code, which is a real pattern. Nothing is silently misled by the choice, because a sibling holding the id gets a generation refusal rather than the next task's result. A handle dropped while its task is still PENDING keeps the slot until it settles, so overrunning the table with in-flight work is still the same typed refusal it always was. Verified: 300 paced fire-and-forget spawns run on a one-record table, and 50 unpaced spawns on a four-slot table are refused cleanly. ## A real bug the merge produced, caught by main's suite #29 made `settle()` end with `forgetIfSettled()`; this branch made `adopt()` settle eagerly so a stale ticket is refused at attach time. Together, `adopt()` of an already-settled slot forgot the view before the caller's claim was counted, and `await()` then failed with OutOfBounds - `testAPanicWhoseDetailCannotBeAttached` caught it. `adopt()` now claims BEFORE it settles, with the ordering written down. ## Conflicts - `src/Parallel/SlotTable.php` - seven regions, resolved as above. - `AGENTS.md` - both sides appended slot bullets and both edited the soak table. Kept both, rewritten around the two-lifetimes framing, and #27's preemption-soak row and self-test paragraph merged with this branch's `--leak-slots`. - `JoinHandle`/`ResultSlot`/`README` auto-merged and were then reconciled by hand; the README's slot-budget table had claimed a never-awaited handle holds its slot for the rest of the run, which is no longer true. `tools/soak-arena-watermark.php` did not conflict - main never touched it - and its RSS verdict now passes for the two reasons #24 fixed plus this branch's own pruning. ## Expectation updates - `testACompletedResultSlotIsNotRetainedByItsWaiter.phpt`: "views after await, handle still held" is 0 rather than 1, because awaiting now releases immediately. Its subject is untouched, and the destructor path it also covered moves to a new test rather than being dropped. - `testAWorkerKilledInsideACriticalSectionReportsTheRecoveredLock.phpt`: a lost slot is named `#0/gen1`, since an arena slot id carries its generation now. - New `testAnUnawaitedHandleGivesItsSlotBackWhenItIsCollected.phpt` pins the destructor path on a ONE-slot table, where a second spawn can only be allocated if the dropped handle's record really came back. Suite: 108 tests OK on PHP 8.4, against substrate main merged into this branch's substrate counterpart. phpstan level max clean, cs:check clean. All four soaks PASS (arena-watermark, memory-flatness, no-leftover-children, preemption), and both arena-watermark detectors still FAIL on demand. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #24.
The diagnosis — two components, and the arithmetic closes
The issue's premise ("native memory, outside PHP's allocator") turned out to be wrong, and correcting it was the key. Both soaks sample only
memory_get_usage(true)— 2 MiB chunk granularity, flat at 8192.0 KiB throughout — andsoak-memory-flatness.phpbuilds its runtime with no workers, so it never touches the parallel path and exonerates Layer 1 only. Samplingmemory_get_usage(false)shows the PHP heap itself climbing 1.70 KiB/round while chunks never move.Bisection with minimal per-path probes (channel-only,
mutableHandle()writes, slot-only, spawn-only, full round; 200 rounds each, warmup dropped, probe's own overhead subtracted):spawnParallel()->await()Component 1 (~77%):
SlotTable::$slotswas never pruned.open()inserts aResultSlotand nothing ever removed it —settle()only flipscomplete. Every spawn retained ~220 B (192 B object size class + ~32 B amortized bucket) for the life of the run; 8 spawns/round predicts 1.76 KiB/round against 1756 B measured (0.3% off).Component 2 (~23%): the pre-sized shared slot table faults in over the whole run.
MAP_ANONYMOUSpages materialize on first store, and slots are consumed one per spawn, so the table's residency arrives 64 B at a time — measured off/proc/self/smapsat 62.7 B per consumed slot with the PHP heap byte-flat. No arena counter can see it because nothing is allocated. Fixed upstream by lisachenko/php-shared-data-extension#24 (Arena::prefault()at table creation, pre-fork).The apparent superlinearity in the issue (0.83 KiB/round at 40 rounds vs 2.16 at 120) is RSS page granularity: early rounds spend already-resident chunk headroom. The named suspects are all cleared with numbers — per-refresh CData views (the arena binds one
uint64_t*per process),WaiterTable-per-await (only built in the substrate's spin-loopawait(), which this runtime never calls), per-await interned strings (heap flat in slot mode), and the ~14 B/deref FFI-callback leak (would need ~130 derefs per spawn on a path that makes none).The fix here — claim-counted local views
The local
ResultSlotis bookkeeping, not the answer: the answer is in shared memory and stays there. So the view is now claimed —open()/adopt()hand out one claim, theJoinHandlecarries it, and the handle's destructor gives it back; a settled slot with no claims and no waiters is forgotten. A pending slot is never dropped whatever its claim count, sofailPendingOf()can still turn a dead worker into a throw at the waiter. The dispatch-failure path inWorkerSupervisor::spawn()releases the claim no handle will ever carry. The shared slot keeps its answer and its id —adopt()takes a fresh view whenever one is wanted again — andSlotTable::liveViews()is the diagnostic a memory gate wants: it tracks work in flight, not total spawns.Before / after (tolerance untouched at 4096)
tools/soak-arena-watermark.php--self-teststill comes back FAIL — the detector still detects.Verification
PHP 8.4.19,
-d ffi.enable=1 -d opcache.jit=off: full suite OK (99 tests) — including a run against the unpatched substrate, so this branch is independently mergeable; PHPStan level max clean;cs:checkclean;soak-memory-flatnessandsoak-no-leftover-childrenPASS. NewtestACompletedResultSlotIsNotRetainedByItsWaiter.phptpins the pruning. PHP 8.5 is CI-only (the local vendor tree resolves the 8.4 z-engine line).Full green (
rss plateau +0.0) requires the substrate PR as well; each half stands alone and neither regresses without the other.🤖 Generated with Claude Code
https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
Generated by Claude Code