Repository navigation
fix: share a workspace's local reduced streams instead of minting a hub per call (#1324) - #1345
Conversation
…ub per call Every NodeType recompile retained ~20 MB that survived a full GC (#1324). Heap dumps around the deterministic repro (NodeTypeRecompileAlcLeakTest) name the retainer, and it is not Roslyn: +189 MessageHub instances over three recompiles — 63 per recompile — each with its own Autofac lifetime scope, TypeRegistry and JsonSerializerOptions (~140 KB apiece). Almost all were `sync/{id}` sub-hubs, one per SynchronizationStream. Root cause: Workspace.GetStream(reference) reduced the data-source stream again on EVERY call. CreateReducedStream constructs a SynchronizationStream (which constructs a hosted sync hub) and registers it for disposal on the PARENT stream — the data source's primary stream, which lives as long as the hub. So an uncached GetStream leaks a hub per call. The callers are hot: DataExtensions.HandlePatchDataRequest resolves the target stream that way on every cross-hub write, and MeshNodeStreamHandle does it on every own-node read and own-node Update. A compile writes its NodeType, its _Activity/compile-state and a fresh _Activity/compile-<ts>, hence ~60 hubs per recompile. Remote reduced streams were already cached per (owner, reference, identity). Local ones now are too, keyed on the reference, with the same liveness re-check and the same split: a caller-supplied StreamConfiguration makes the stream caller-specific (client id, subscriber, property bag), so those stay uncached. A faulted Lazy is evicted rather than poisoning the reference for the workspace's life. Measured on the repro: 20 MB -> 9.3 MB retained per recompile, 63 -> 22 hubs per recompile, and the mesh's steady-state hub count 187 -> 48 (the same waste was being paid on ordinary reads and writes, not only on compiles). The guard in NodeTypeRecompileAlcLeakTest drops from 32 MB to 16 MB and records where the remaining ~9 MB lives: Workspace.EvictForPath parks an evicted remote stream without disposing it, so every change event on a subscribed path mints a fresh client/owner sync pair. That is a separate fix; the bound is tightened again with it. MeshWeaver.Data.Test 329/329 and MeshWeaver.Hosting.Monolith.Test 629/629 pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses #1324 by eliminating a major source of managed-memory retention during in-portal NodeType recompiles: repeated creation of local reduced synchronization streams that each mint a hosted sync/{id} sub-hub. It does so by caching workspace-local reduced streams (mirroring the existing remote-stream caching strategy), significantly reducing hub churn and retained memory across recompiles, and updates the leak-guard test and release notes accordingly.
Changes:
- Cache workspace-local reduced streams (when no caller-specific
StreamConfigurationis provided) to avoid creating a newSynchronizationStream/hostedsync/hub per call. - Tighten
NodeTypeRecompileAlcLeakTest’s per-recompile retention ceiling from 32 MB to 16 MB and document the measured retainers. - Add a “What’s New” entry documenting the user-visible impact of the fix (reduced memory growth during rebuilds).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Monolith.Test/NodeTypeRecompileAlcLeakTest.cs | Lowers the retention threshold and documents the identified retainers so the guard matches current reality while still catching regressions. |
| src/MeshWeaver.Data/Workspace.cs | Introduces a local reduced-stream cache keyed by WorkspaceReference and reuses the remote cache’s liveness + fault-eviction pattern to prevent “hub per call” growth. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-13-the-portal-stops-growing-every-time-code-is-rebuilt.md | Adds release notes describing the memory-growth fix and its practical effect on portal stability. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…checkpoint OrphanedStartingExecutionRecoveryTest's first checkpoint asserted `PendingUserMessages.ContainKey(userMsgId)` on the first emission after cold activation. Subscribing is what ACTIVATES the hub, so the recovery under test (roll the orphaned StartingExecution claim back to Idle → the submission watcher drains PendingUserMessages into the round) races the very snapshot that read waits for — both run on the hub the SubscribeRequest just woke, and nothing in the product orders them. It went red on CI once activation got FASTER: sharing the workspace's local reduced streams removes ~40 stream+hub constructions from a cold activation, so the drain now beats the snapshot often enough to matter. The state being asserted is one the fix under test exists to consume, which is not a specification. UserMessageIds is the race-free anchor: the drain moves entries out of PendingUserMessages into IngestedMessageIds, and CommitRoundAndExecute / FoldConsumedInbox explicitly restore UserMessageIds ⊇ IngestedMessageIds. It proves what this checkpoint is for — the cold activation loaded the SEEDED node — and it is strictly stronger against the failure mode that matters, content materialising as a defaulted record, which the old assertion could not tell apart from a won race. Read through ContentAs<T> for the same reason. Nothing is weakened. That the queued message really reaches a round is asserted below and always was: PendingUserMessages empty, Status back to Idle, ExecutionStartedAt cleared, and the Echo agent's reply present in the response cell — plus the revert-proven log pin on the recovery branch itself. Those assertions never even ran on the red build, because the transient check threw first. MeshWeaver.Threading.Test 128/128. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI red on the first run — diagnosed, not retried
It is a race the test was always exposed to, and this change legitimately shifts it. That test seeds This PR removes ~40 stream+hub constructions from a cold activation, so init now finishes sooner and Checked before touching it, rather than assuming a flake:
Fix: the checkpoint now asserts
|
Part of #1324 — the dominant retainer, not all of it (see What remains below), so this deliberately does not auto-close the issue.
The measurement, and what it ruled out
Reproduced the issue's own repro (
NodeTypeRecompileAlcLeakTest) on this machine — 110 MB → 171 MBover three recompiles, ~20 MB each, load contexts steady at 1 — then took
dotnet-dumpheap dumpsaround the loop and diffed
dumpheap -stat.It is not Roslyn. Not one
CSharpCompilation,MetadataReaderor symbol-graph type appears inthe top of the delta. The retained objects are:
MeshWeaver.Messaging.MessageHubAutofac.Core.Lifetime.LifetimeScopeMeshWeaver.Messaging.Serialization.TypeRegistrySystem.Text.Json.JsonSerializerOptions…Serialization.TypeDefinition63 message hubs per recompile, ~140 KB apiece. Dumping the hosted-hub tree by address shows what
they are:
sync/{id}sub-hubs, one perSynchronizationStream, hosted on the NodeType hub, on_Activity/compile-state, on the fresh_Activity/compile-<ts>, and on the cache hub.Root cause
Workspace.GetStream(reference)was not memoized. Every call runsReduceManager.ReduceStream→WorkspaceStreams.CreateReducedStream, which constructs aSynchronizationStream— and aSynchronizationStreamconstructs a hostedsync/{id}hub with itsown Autofac scope,
TypeRegistryandJsonSerializerOptions— then registers itself for disposalon its PARENT (the data source's primary stream), which lives as long as the hub. So an uncached
GetStreamleaks a hub per call, reclaimed only when the whole hub dies.Stack-sampling the streams created during one recompile names the callers:
Nothing about a plain reduce is caller-specific, and remote reduced streams were already cached
per
(owner, reference, identity)— the local twin of that cache simply did not exist.The fix
Cache local reduced streams on the workspace, keyed on the reference, with the same liveness
re-check
GetExternalClientSynchronizationStreamuses and the same split: a caller-suppliedStreamConfiguration(client id, subscriber, property bag) makes the stream caller-specific, sothose stay uncached. A faulted
Lazyis evicted rather than poisoning the reference for theworkspace's life.
Measured after
The steady-state drop is the pleasant surprise: the same waste was being paid on ordinary reads and
writes, not only on compiles.
What remains (honestly)
The remaining ~9 MB is a second retainer with the same shape, and it is the one the issue's own
third comment named:
Workspace.EvictForPathparks the evicted remote stream in_evictedRemoteStreamswithout disposing it, so every change event on a subscribed path mints afresh client/owner
sync/pair (6 on the cache hub + 8 oncompile-stateper compile here) that isreaped only by the 10-minute idle sweep with zero subscribers, or by
Workspace.Dispose. Fixing itneeds a decision about whose lifetime a parked stream belongs to, so it is deliberately not in
this change. The per-compile
_Activity/compile-<ts>node hub is a third, smaller one (~1 hub +6 streams per compile).
The guard therefore moves from 32 MB → 16 MB, not to single digits: a bound nothing can
currently pass is a red test, not a guard. The test comment records the exact remaining retainers
and says to tighten it again with them.
Verification
MeshWeaver.Data.Test— 329/329MeshWeaver.Hosting.Monolith.Test— 629/629 (includesNodeTypeRecompileAlcLeakTestat the new 16 MB bound)dotnet build -c Release -warnaserrorclean for both touched projects, after merging current main🤖 Generated with Claude Code