Skip to content

perf: the fan-out ships a string SPLICE too — negotiated, because its consumers fail silently (#1284) - #1434

Merged
rbuergi merged 1 commit into
mainfrom
perf/1284-fanout-string-splice-wt
Aug 13, 2026
Merged

rbuergi merged 1 commit into
mainfrom
perf/1284-fanout-string-splice-wt

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Closes the owner→subscriber half of the quadratic-streaming class — #1284. The write half shipped in #1414; JsonSynchronizationStream.CreateSingleObjectPatch kept fanning out an RFC 6902 replace carrying the whole new string, once per Sample(100 ms) tick and once more for every viewer and every cross-silo mirror.

Measured

Per subscriber, through the real path end to end — the production differ (PatchExtensions.CreatePatch), the real operation serializer, and the real consumer (DataChangedEvent.UpdateJsonElement → ApplyPatchWithCorrectUnescaping). The two columns are the same code with one negotiated step switched on.

5,000 chars / 50 frames 20,000 chars / 200 frames
before — wire bytes 129,700 2,018,800 (1.93 MB)
after — wire bytes 13,735 43,070 (42.1 kB)
reduction 9.4× 46.9×
before — bytes per answer char 25.9 100.9 (grows 4× with length)
after — bytes per answer char 2.7 2.2 (falls — amortises)

The shape assertion is the one that matters and it is asserted both ways: the baseline column must exhibit the quadratic (per-char cost grows >3× when the answer quadruples) — otherwise the comparison proves nothing — and the after column's per-char cost must fall.

The change

A changed string leaf at or above PatchStringSplice.MinSpliceLength travels as a new operation verb:

{"op":"splice","path":"/content/text",
 "value":{"$sd":[start,removed,"inserted"],"$sdb":[baseLength,"fingerprint"]}}

Same $sd / $sdb markers as the write path — one shape, one meaning — combined into the single value slot an RFC 6902 operation has.

The base is the producer's own cached JsonElement for that subscription, not the update's OldValue. That cache is the document the subscriber holds: the two are kept in lockstep by the frame chain, which resyncs on a lost frame before applying anything. So the fingerprint check on the far side is a proof obligation rather than a gamble, and the compression is a pure post-pass over the finished patch — CreatePatch and every existing operation's bytes are untouched.

No blind splicing. The subscriber applies the splice only when the fingerprint proves its text is byte-identical to the base. On a mismatch it throws StaleStreamStateException, which SynchronizationStream.UpdateStream already answers with a fresh authoritative Full from the owner (storm-gated by _resyncInFlight — one resubscribe per gap, no timer, no retry loop). That is #1414's rule with a strictly better recovery: a resync instead of a NACK.

Why this one is NEGOTIATED, where the write path was not

#1414 could put its marker straight on the wire, and deliberately did: the only consumer is a C# owner, which fails loudly on a shape it does not understand — the merged node stops deserialising, the write is NACKed and never commits. Loud beats silent.

The fan-out has no such property. Its consumers are four hand-rolled appliers this repo's CI does not build, and every one of them fails silently:

consumer on an unknown op
clients/grpc-web/src/jsonPatch.ts:92 default: break — no-op; the text simply stops updating mid-stream
clients/react/src/live/grpcSource.ts:322 default: break — same
clients/react/src/area/pointer.ts:86 no default clause at all — same
clients/python/meshweaver/mesh.py:70 applied as a replace, writing op.get("value") → None into the field
C# SynchronizationStream throws Unknown patch operation — the only loud one

And a browser holds whatever bundle it loaded, so "upgrade both ends together" is not available here the way it is between two pods.

So SubscribeRequest gains AcceptsStringSplice (default false). The owner splices only for a subscriber that declared it; everyone else receives byte-identical bytes to today — no migration, no flag day, no shape they can misread. The marker still rides inside the patch, so a consumer that claims the capability and cannot honour it still fails loudly.

How an old JS client is handled, concretely: it is not spliced to. clients/grpc-web, clients/react and clients/python do not set the flag, so their frames are unchanged and JsonPatchWireFormatTest — the pinned gate standing between a refactor and a silent client break — is untouched and green (366/366). They can opt in later by implementing the verb and setting the flag; nothing about this change obliges them to.

Rolling deploys are safe in both directions: the messaging serializer runs UnmappedMemberHandling.Skip, so a new subscriber against an old owner is simply not spliced to, and an old subscriber against a new owner never sets the field.

Not in scope

Verification

Local, -c Release -warnaserror on every touched project:

suite result
MeshWeaver.Data.Test 368 / 368 (incl. new FanOutStringSpliceTest, 9)
MeshWeaver.Json.Test 366 / 366 — the pinned client wire-format gate
FanOutSpliceNegotiationTest passes — the framework's own subscribes really do declare the capability, asserted on live SubscribeRequests as the owning node hubs receive them

That last one exists because the failure mode of a negotiation is silent in the direction that looks fine: if nobody declared the capability, everyone would keep getting whole values, the measurement would never move, and no test would go red.

🤖 Generated with Claude Code

… consumers fail silently

Closes the owner→subscriber half of #1284. #1414 made the cross-hub WRITE carry a splice;
`JsonSynchronizationStream` kept fanning out an RFC 6902 `replace` with the whole new string,
once per Sample(100 ms) tick and once more for every viewer and every cross-silo mirror.

Measured, per subscriber, through the real differ + serializer + applier:

  5,000 chars /  50 frames:   129,700 B ->  13,735 B   (9.4x)
 20,000 chars / 200 frames: 2,018,800 B ->  43,070 B  (46.9x)
 bytes per answer char:  25.9 -> 100.9 (grows 4x with length)
                     becomes  2.7 ->   2.2 (falls — amortises)

A changed string leaf at or above MinSpliceLength now travels as
{"op":"splice","path":…,"value":{"$sd":[start,removed,"inserted"],"$sdb":[len,"fp"]}}. The
splice is computed against the producer's OWN cached JsonElement for that subscription — which
IS the document the subscriber holds — and the subscriber applies it only when the fingerprint
proves that; otherwise it refuses and takes the existing stale-patch route, RequestFreshSnapshot,
never a blind splice. Same rule as #1414, with a better recovery.

NEGOTIATED, unlike the write path, and that is the whole difference. #1414 could put its marker
straight on the wire because the only consumer is a C# owner that fails LOUDLY — the merged node
stops deserialising, the write is NACKed, nothing commits. The fan-out has no such property: it
is consumed by four hand-rolled appliers this repo's CI does not build (clients/grpc-web, two in
clients/react, clients/python), and every one of them fails SILENTLY — the JS ones treat an
unknown op as a no-op, so the text would just stop updating mid-stream; the Python one applies
any unrecognised op as a replace, writing None into the field. And a browser holds whatever
bundle it loaded, so "upgrade both ends together" is not available.

So SubscribeRequest gains AcceptsStringSplice (default false, skipped by an old peer's
deserializer in both rolling-deploy directions). This assembly sets it on its own subscribes —
the code that asks is the code that folds — and everyone else keeps receiving bytes identical to
today. JsonPatchWireFormatTest is untouched and green: no existing operation's bytes change.

Verified locally, -c Release -warnaserror:
- MeshWeaver.Data.Test 368/368 (incl. new FanOutStringSpliceTest, 9)
- MeshWeaver.Json.Test 366/366 — the pinned client wire-format gate
- FanOutSpliceNegotiationTest — the framework's own subscribes really do declare it, asserted
  on live SubscribeRequests as the owning node hubs receive them

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 13, 2026 14:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends MeshWeaver’s JSON Patch fan-out (owner → subscriber) to support an opt-in splice operation for large string replaces, reducing per-subscriber streaming traffic from quadratic growth (per tick × total text) to incremental updates while keeping legacy clients safe via explicit capability negotiation.

Changes:

  • Add a negotiated SubscribeRequest.AcceptsStringSplice capability and emit splice ops only when the subscriber declares support.
  • Implement fan-out patch compression (PatchStringSplice.Compress) and subscriber-side splice folding with fingerprint validation (refusing stale bases to trigger resync).
  • Add targeted test coverage for correctness + wire-size shape, plus architecture/What’s New documentation updates.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/MeshWeaver.Threading.Test/FanOutSpliceNegotiationTest.cs Verifies framework subscribers declare the splice capability on real SubscribeRequests.
test/MeshWeaver.Data.Test/FanOutStringSpliceTest.cs Pins correctness, negotiation behavior, and wire-cost reduction for streamed text fan-out.
src/MeshWeaver.Utils/Json/PatchOperation.cs Adds OperationType.Splice and supports verb serialization/deserialization.
src/MeshWeaver.Utils/Json/JsonPatch.cs Ensures generic applier refuses splice (to avoid blind apply / silent skip).
src/MeshWeaver.Data/Serialization/PatchStringSplice.cs Encodes/decodes combined splice + base fingerprint shape and compresses patches.
src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs Negotiates capability, compresses fan-out patches, and applies splice with validation.
src/MeshWeaver.Data.Contract/Messages.cs Introduces SubscribeRequest.AcceptsStringSplice contract field with rationale.
src/MeshWeaver.Documentation/Data/Architecture/DataSyncAndCrdt.md Documents negotiated splice behavior in the owner→subscriber pipeline.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-13-streaming-answers-cost-the-viewer-far-less.md Adds a user-facing “What’s New” entry describing the improvement.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/MeshWeaver.Threading.Test/FanOutSpliceNegotiationTest.cs
Comment thread src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs
@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

723 tests   723 ✅  10m 40s ⏱️
 10 suites    0 💤
 10 files      0 ❌

Results for commit 67ec0ef.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   11 files     11 suites   5m 15s ⏱️
1 767 tests 1 662 ✅ 105 💤 0 ❌
2 081 runs  1 976 ✅ 105 💤 0 ❌

Results for commit 67ec0ef.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 909 tests   1 717 ✅  6m 7s ⏱️
   11 suites    192 💤
   11 files        0 ❌

Results for commit 67ec0ef.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 977 tests   1 969 ✅  8m 13s ⏱️
   11 suites      8 💤
   11 files        0 ❌

Results for commit 67ec0ef.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

2 573 tests   2 572 ✅  6m 48s ⏱️
   12 suites      1 💤
   12 files        0 ❌

Results for commit 67ec0ef.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

1 374 tests   1 374 ✅  7m 3s ⏱️
   11 suites      0 💤
   11 files        0 ❌

Results for commit 67ec0ef.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

    66 files      66 suites   44m 8s ⏱️
10 323 tests 10 017 ✅ 306 💤 0 ❌
10 637 runs  10 331 ✅ 306 💤 0 ❌

Results for commit 67ec0ef.

@rbuergi
rbuergi merged commit 5c5e4eb into main Aug 13, 2026
20 checks passed
rbuergi added a commit that referenced this pull request Aug 13, 2026
review follow-up to #1434: a splice's array index is bounds-checked into the resync path
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.

2 participants