StrandExecutor still rebuilds the map node and the Strand on every serial dispatch -- 152 bytes, 2 allocations, and removing them needs a deregistration hook

[Yaraslaut]

What is left after morph#660

morph#660 listed three directions and this took the second — shrink what the
drain-and-erase rebuilds. Measured on 7a343e6f, x86-64 Linux, GCC 16.2.1 /
libstdc++, -O2, with tests/bench/bench_dispatch_allocations.cpp:

before   4159 allocations / 398072 bytes over 200 round trips   (20.80 / 1990.4 per call)
         #7    32 bytes  strand.hpp:104   _strands map node
         #8   152 bytes  strand.hpp:106   make_shared<Strand>
         #9    64 bytes  <stl_deque>      the Strand's std::queue deque map
         #10  512 bytes  <stl_deque>      the Strand's std::queue first buffer

after    3786 allocations / 279208 bytes over 200 round trips   (18.93 / 1396.0 per call)
         #7    32 bytes   _strands map node
         #8   120 bytes   make_shared<Strand>

152 bytes and 2 allocations per serial local dispatch remain, about 11% of
the 1396 bytes a round trip now costs. They are the unordered_map node and the
Strand control block, and no container change removes them: they exist because
scheduleNext erases the map entry the moment the strand drains, so the next
post() for the same model takes the if (!slot) branch and builds both again.

Why it was not done in morph#660

Keeping the slot alive across the drain is direction 1 of morph#660's three, and
it is not a container swap — it is a change of lifetime policy. StrandExecutor
has no deregistration hook: the erase is the only thing that ever removes a
key, so a retained slot is retained until ~StrandExecutor. A process that
registers and drops many short-lived models would grow _strands without bound.

Making that safe means adding a forget(ModelId)-shaped API and wiring it into
LocalBackend/RemoteServer model deregistration, and doing it without
disturbing the {_mapMtx, strand->mtx} atomicity that
docs/spec/core/executor.md ("The per-key serialisation invariant") and the
comments at strand.hpp:81/:167 record a previous defect forcing into shape.
That is a different, larger and riskier change than the one morph#660 closed
with, so it was deliberately not smuggled in.

Verification status

• Reproduced: both figures above are runs on this workstation, before and
  after the morph#660 change, same binary, same flags. The four attributions in
  the "before" block come from a backtrace() added to the bench's counting
  hook, resolved through dladdr + addr2line -f -i -C — real output, not a
  reading of the code.
• Inferred, not measured: that the two remaining allocations are
  unavoidable without a lifetime change. The reasoning is the erase, which is
  visible in the code; no alternative design was prototyped and measured.
• Not measured: the time cost of either the current state or a fix. Nothing
  here claims a latency change; 152 bytes of short-lived churn per dispatch may
  be entirely acceptable.
• Not measured: how much of a realistic workload is serial. Under concurrent
  load the strand stays armed and this cost amortises away completely.
• Not verified: the figures on libc++, MSVC or the WASM toolchain.

What would change the verdict

• Close as invalid if a measurement shows the remaining 152 bytes are noise
  for every workload that matters — which is the likely outcome, and is why this
  is filed rather than worked. 11% of an allocation census is not by itself a
  reason to change a lifetime rule.
• Raise in priority if a serial GUI workload is demonstrated to be
  allocation-bound, or if a deregisterModel hook lands on StrandExecutor for
  an unrelated reason, at which point the retained-slot design costs much less.

Filed as the remainder of morph#660 rather than reopening it: morph#660's own
"Directions" section named three, one was taken, and the other two are a
different change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Triage: valid, and correctly separated from #660

#660's fix removed the two allocations that were never about the strand's lifetime — libstdc++'s std::deque building a node map and a 512-byte buffer in its default constructor. What this ticket names is the remainder: 2 allocations / 152 bytes per serial dispatch, being the map node and the make_shared<Strand> control block.

triage: valid, enhancement, area: core.

Splitting it out was the right call. The two halves need different changes with different risk: #660's was a container swap inside Strand, touching no lifetime or locking rule, and it was provable by a TSan run plus a mutation. This one needs a StrandExecutor deregistration hook — i.e. changing when the map slot dies — which is exactly the {_mapMtx, strand->mtx} atomicity that strand.hpp:81 and :167 record a previous defect having forced into shape. Folding them together would have put a locking change and a container change behind one green suite.

The figures are the filer's and I have not reproduced them — morph_bench_alloc landed with #661 and I have run no build. The instrument is on master now, so the next person re-measures with one command.

For whoever takes it

• Read the atomicity comments before designing anything. The erase's shape is not incidental; a previous race forced it. test_strand_race.cpp does not detect the race it is the regression test for -- 10/10 passes under TSan against a build with the two-step drain-and-erase restored #668 is directly relevant: the named regression test for that invariant reportedly does not detect its own regression, so a passing [strand] suite is not evidence for this change. Either fix test_strand_race.cpp does not detect the race it is the regression test for -- 10/10 passes under TSan against a build with the two-step drain-and-erase restored #668 first or bring your own detector, and prove it can see the hazard before trusting its silence — which is what core: StrandExecutor rebuilds a model's Strand on every serial dispatch -- 4 allocations and 752 of the 1995 bytes per local call #660's lane did.
• Weigh it against what it buys. 152 bytes and 2 allocations out of ~1396 bytes and ~18.9 allocations per call is roughly 11% of bytes. That is real, but it is a change to lifetime rules on the concurrency-critical path, and core: StrandExecutor rebuilds a model's Strand on every serial dispatch -- 4 allocations and 752 of the 1995 bytes per local call #660 already took the cheap 30%. This may be a ticket whose honest answer is "not worth the risk", and saying so with the measurement would be a good outcome.
• The magnitude is libstdc++-specific; the filer says so for core: StrandExecutor rebuilds a model's Strand on every serial dispatch -- 4 allocations and 752 of the 1995 bytes per local call #660 and it applies here.

What would change the verdict

Close as parked or invalid if a design pass concludes the deregistration hook cannot preserve the atomicity property without a lock-ordering change — in which case the residual is the price of the invariant, and recording that is more valuable than leaving the ticket open.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW