Skip to content

feat(parallel): give result slots back once their handle has taken the answer - #31

Merged
lisachenko merged 3 commits into
mainfrom
claude/issue-16-slot-recycling
Aug 16, 2026
Merged

feat(parallel): give result slots back once their handle has taken the answer#31
lisachenko merged 3 commits into
mainfrom
claude/issue-16-slot-recycling

Conversation

@lisachenko

Copy link
Copy Markdown
Owner

Closes #16. Consumer half of the recycling work; depends on lisachenko/php-shared-data-extension#25. This package requires the substrate as dev-main, so CI here stays red until that lands on substrate main — expected, with precedent in this repo.

What it adds

The substrate now recycles result slots through a generation-tagged free list. This PR is the signal that says a slot has been read — and the audit that says when saying so is true.

When it is safe. SlotTable::settle() is the only place this package reads a shared slot record, and it copies the whole outcome into per-process state before returning: the decoded value, or a ParallelTaskException built from the shared error object's named properties. After that nothing here looks at shared memory for that slot again — refresh() skips locally complete slots, and failPendingOf() reaches the record only through refresh(). So JoinHandle takes its answer out of the slot, keeps it, and only then releases: capture, then release. Awaiting the same handle again replays what was kept.

What came out of the record stays valid: an OBJ/STR answer is an arena address and the arena frees nothing, so releasing a slot returns the slot record, never the memory the answer lives in. Same for a panic's SharedError — the strings the exception carries are arena strings that outlive any slot. No error-graph reclamation is attempted, and none should be from a worker: children never free arena memory, so those stay leak-until-teardown (#19's ground).

One counter, two lifecycles

This lands on top of #29, which counted claims to decide when to drop a slot's local view, while this branch counted handles to decide when to give the shared record back — two mechanisms measuring the same thing. They are now one counter, ResultSlot::$claims, and forgetIfSettled() is the single decision: at zero claims on a settled, unwaited-on slot the local view is dropped always, whatever this process's role, while the shared record goes back only if this process owns it and the family really settled it. An adopter drops its view and releases nothing; a slot whose worker died drops its view and keeps its record out of circulation, because that record is still pending where a zombie may write to it. Pending slots are never forgotten whatever the claim count — failPendingOf() must still find them.

JoinHandle gives its one claim back at the first of two moments — awaited, or collected — guarded so the pair cannot count as two (attachResult() of a slot this process opened is legitimately two handles on one slot). Awaiting releases immediately, which is strictly stronger than release-at-collection: a batch that spawns N and awaits in a loop holds one slot at the end, not N.

A settled, never-awaited handle recycles its record and discards the answer, deliberately. Keeping it is the exhaustion bug itself for fire-and-forget code, and nothing is silently misled — a sibling holding the id gets a generation refusal, not the next task's result. Dropping a handle whose task is still pending keeps the slot until it settles.

Merging the two mechanisms surfaced a real bug that main's own suite caught: #29's settle() ends by forgetting an unheld slot, and this branch's adopt() settles eagerly to refuse a stale ticket at attach time — so adopting an already-settled slot forgot the view before the caller's claim was counted. adopt() now claims before it settles, with the ordering written down at the call site.

Cross-process semantics, stated plainly: the owner releases; an adopter in another process must finish reading before the owner's await() returns, or its next read is refused by generation — loudly, never with another task's result. A shared reader count in the slot record is the alternative if unordered multi-process readers ever become a real workload.

Option 2 as well

new Runtime(..., slots: N) sizes the table, and the README gains a result-slot budget section: one slot per spawn, given back at await(), so the supply bounds concurrency rather than throughput — with the arithmetic and the two exceptions spelled out.

Acceptance criteria from #16

  • A pool spawning far more tasks than the slot supply runs to completiontestAPoolOutlivesItsResultSlotSupply.phpt: 8 slots, 100 tasks, 1 slot record ever created, 0 outstanding, 99 recycled.
  • A handle on a recycled slot id fails loudlytestAHandleOnARecycledSlotIsRefusedRatherThanAnsweredWithAnotherTasksResult.phpt: await task A, spawn B onto the same record, attachResult() A's id, assert the refusal names the slot and both generations and that B is unharmed.
  • soak-arena-watermark.php shows slot consumption plateauing — new slots series with a zero-tolerance plateau verdict; the tool now runs on a deliberately small table (--slots=64, 320 spawns) because a table big enough to absorb the run would prove nothing. --leak-slots makes the new detector fail on demand, as --self-test does for the watermark.

Plus testAnUnawaitedHandleGivesItsSlotBackWhenItIsCollected.phpt on a one-slot table, where the second spawn can only be allocated if the dropped handle's record really came back.

Verification

Against the exact tree the substrate PR produces (vendored copy byte-identical to the merged substrate branch, prefault included), PHP 8.4:

suite      OK (108 tests, 108 assertions)
phpstan    level max, no errors
cs:check   clean

soak-arena-watermark:
  result slots: 320 spawns over a table of 64 — 1 records ever created, 319 recycled, 0 still out
  slots      plateau — flat at 1 slot record(s)     watermark  plateau +0.0 KiB
  allocated  plateau +0.0 KiB                        rss        plateau +0.0 KiB    PASS
soak-memory-flatness       PASS
soak-no-leftover-children  PASS
soak-preemption            PASS (2996 preemptions / 30.4 s, 0 wrong results, 2075/2075 fibers)
--self-test / --leak-slots both FAIL as required

Ad-hoc stress: 1600 concurrent-batch spawns over 24 slots (0 wrong, high-water 8, 0 outstanding); 240 spawns with 80 panics and repeat-awaits; 300 paced fire-and-forget spawns on a table that created one record; 50 unpaced spawns on a 4-slot table refused cleanly with the outstanding/retired counts named.

Known, pre-existing, not fixed here: sustained panics still exhaust the substrate's 256-entry registry (entries table full) — one persisted SharedError graph per panic. Reproduced identically on main; #19's ground, worth its own issue if panic-storm workloads matter.

🤖 Generated with Claude Code

https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA


Generated by Claude Code

claude and others added 3 commits August 16, 2026 09:22
…e answer

A pool spawning ten tasks a second exhausted the 4096-slot supply in under seven
minutes however few tasks were ever in flight, because a slot was taken per
spawnParallel() and never returned. The substrate now recycles slots through a
generation-tagged free list; this is the consumer half - the signal that says a
slot has been read, and the audit that says when saying so is true.

**When it is safe.** `SlotTable::settle()` is the only place this package reads a
shared slot record, and it copies the whole outcome into per-process state before
returning: the decoded value, or a `ParallelTaskException` built from the shared
error object's named properties. After that, nothing here looks at shared memory
for that slot again - `refresh()` skips locally complete slots, and
`failPendingOf()` reaches the record only through `refresh()`. So `JoinHandle`
takes its answer out of the slot, keeps it, and only then releases: capture, then
release. Awaiting the same handle again replays what was kept rather than reading
a record that may already belong to somebody else.

What came *out* of the record stays valid. An `OBJ` or `STR` answer is an arena
address, and the arena frees nothing - releasing a slot returns the slot **record**,
never the memory the answer lives in. The same reasoning covers a panic's
`SharedError`: the strings the exception carries are arena strings that outlive any
slot. No attempt is made to reclaim an error graph, and none should be from a
worker: children never free arena memory, so error graphs stay leak-until-teardown
(that is #19's ground, not this change's).

**Who releases.** The owner - the process that opened the slot - and only once every
local handle on it is satisfied, which is what the per-slot handle count is for
(`attachResult()` of a slot this process opened is legitimately two handles on one
slot). An adopter never releases: it cannot know who else in the family is still
reading. A cross-process reader therefore has to finish before the owner's await()
returns, and if it does not, its next read is refused by generation rather than
answered with the next task's result.

**What deliberately does not come back:** a slot whose worker died before settling
it (still pending in shared memory, and a zombie worker may yet write to it), and a
handle nobody awaits (nothing has read it, so nothing can say it is safe). Both are
the pre-recycling behaviour, now scoped to the cases that actually need it. The
local slot map is pruned on release either way - a per-process map that only ever
grows is a leak of its own on a pool that runs for days.

Option 2 as well: `new Runtime(..., slots: N)` sizes the table, and the README
documents the arithmetic - one slot per spawn, given back at await, so the supply
bounds concurrency rather than throughput, with the two exceptions above spelled
out.

`tools/soak-arena-watermark.php` now runs on a deliberately small table (64 slots,
320 spawns) and reports the high-water mark, which must be exactly flat: a slot is
either handed back or it is not, so that series has no tolerance to spend.
`--leak-slots` makes the new detector fail on demand, the way `--self-test` does for
the watermark.

Requires the recycling substrate (lisachenko/php-shared-data-extension), so CI here
stays red until that lands on its main.

Suite: 100 tests OK on PHP 8.4; phpstan level max clean; soak-arena-watermark,
soak-memory-flatness and soak-no-leftover-children all PASS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
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
The previous run installed the substrate from main before
lisachenko/php-shared-data-extension#25 landed there, so the
generation-ticket surface this branch consumes did not exist yet.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
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.

Result slots are bounded and never reclaimed, so a long-running pool exhausts them

2 participants