From a54ae47ab923caa2b18baf47554514e7cb051c9c Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 21 Sep 2026 08:54:48 -0400 Subject: [PATCH] [MG_Remote] (P6/lk): the pre-flight itemisation, and a correction to c6's regroup design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CONTRACT-P6 §8.5 requires lk to publish its blast radius before it starts, because an earlier draft's ratchet baseline was wrong by a factor of three and a ratchet installed on the wrong number is red on its first CI run for reasons that have nothing to do with a leak. The itemisation is mechanical, per file, with the grep spelling stated: 250 seam-identifier references outside Transport/ across 23 files (127 production, 123 test), and 44 member-access sites that the regroup itself must rewrite, across 9 files - 17 of them in MG_IntegrationTest, which is why lk builds with the integration tests ON. The earlier build did not, and a missed edit there would have been green. THE CORRECTION. Measuring that radius turned up a mistake in c6, landed two commits ago. §8.3 and ILink.h both said lk should promote eventRingFull and eventDropped into the watermark group and called the result single-writer. eventRingFull has TWO writers: the server latches it with store(1) when SEG_EVENT fills (EventRing.h:140) and the CLIENT clears it with an exchange(0) on EVERY DRAIN (:200). Promoting it would put a per-drain read-modify-write by the client on the same cache line as appliedSeq - which the server release-stores every 64 records and the client's spin predicate reads 1e5-1e6 times a frame. That is new false sharing on the hottest line in the system, and it would land in exactly the numbers this package's gate has to hold constant. It is also the same class of error the earlier draft made with submittedSeq, one field over. So LinkProgress is now the four consumer-written watermarks and nothing else - genuinely single-writer - and the two flags stay in the doorbell/generation group, where mixed writers already live beside consumerParked/producerParked, crossing the seam through a separate EventFlags() accessor on its own line. The contract records the correction and its reasoning rather than quietly restating the rule. ALSO CORRECTED, from running the baseline suite: a6's link-experiment report said the silently-empty build stamp had "no log and no gate to say so". That is false. SessionHandshakeTest.cpp:140 asserts BuildStamp[0] != '\0' and :180 asserts that perturbing it changes the fingerprint, and BOTH GO RED in this configuration right now - the worktree's .git holds a Windows path WSL's git cannot resolve, so GIT_COMMIT_HASH_SHORT is "". The finding is therefore stronger, not weaker: the gate that catches it already exists and is already red. What is silent is the BUILD, and hs owes a named configure-time failure rather than a new gate. Baseline for lk, recorded: 3878 tests, two failures, both environmental and understood - the fingerprint one above, and PipeCatalogue.FrontendNeverTakes- AnApplierAddress, which walks up from cwd looking for MobileGL/MG_Impl and so fails for any out-of-source build directory. No behaviour change. The headers are still included by nothing. Co-Authored-By: Claude Opus 5 --- MobileGL/MG_Remote/CONTRACT-P6.md | 19 +++++-- MobileGL/MG_Remote/Transport/ILink.h | 54 +++++++++++++----- MobileGL/MG_Remote/Transport/StreamLink.h | 1 + .../notes/p6/a6-link-experiment.md | 10 +++- docs/Disaggregated/notes/p6/lk-itemisation.md | 55 +++++++++++++++++++ 5 files changed, 119 insertions(+), 20 deletions(-) create mode 100644 docs/Disaggregated/notes/p6/lk-itemisation.md diff --git a/MobileGL/MG_Remote/CONTRACT-P6.md b/MobileGL/MG_Remote/CONTRACT-P6.md index a95aa59d..91383634 100644 --- a/MobileGL/MG_Remote/CONTRACT-P6.md +++ b/MobileGL/MG_Remote/CONTRACT-P6.md @@ -435,10 +435,21 @@ presentAckSerial}`; `eventRingFull`/`eventDropped` sit in a **separate** `aligna (`Ring.h:50`) while the other four are consumer-written — so the group has **mixed writers**, and any "single-writer progress block aliased over the watermark line" is unsound. -`lk` therefore **regroups**: `submittedSeq` moves to the producer's own line (`Ring.h:45-50` states -it is diagnostic and nobody waits on it), `eventRingFull`/`eventDropped` are promoted into the -watermark group, and the group becomes a named `LinkProgress` member with per-field `offsetof` -static_asserts. +`lk` therefore **regroups**: `submittedSeq` moves to the producer's own line, beside `cmdHead`, +which is the other thing the producer writes; what is left — the four consumer-written watermarks — +becomes a named `LinkProgress` member with per-field `offsetof` static_asserts. + +> **Correction, found while `lk` was measuring its own blast radius.** An earlier revision of this +> section, and of `ILink.h`, also promoted `eventRingFull`/`eventDropped` into the watermark group +> and called the result single-writer. **Both were wrong.** `eventRingFull` has two writers: the +> server latches it with `store(1)` when `SEG_EVENT` fills (`Transport/EventRing.h:140`) and the +> **client clears it with an `exchange(0)` on every drain** (`:200`). Promoting it would put a +> per-drain RMW by the client on the same cache line as `appliedSeq` — which the server +> release-stores every 64 records and the client's spin predicate reads 10⁵–10⁶ times per frame. +> That is new false sharing on the hottest line in the system, and it would land in exactly the +> numbers this package's gate has to hold constant. The two flags stay in the doorbell/generation +> group, where mixed writers already live beside `consumerParked`/`producerParked`, and cross the +> seam through a separate `EventFlags()` accessor on its own line. It is free **today** — both peers are the same binary, enforced by the fingerprint — and a wire break the day a second build exists. It is **`lk`'s and not `c6`'s** because the rename blast radius is diff --git a/MobileGL/MG_Remote/Transport/ILink.h b/MobileGL/MG_Remote/Transport/ILink.h index 02af579e..85370e7d 100644 --- a/MobileGL/MG_Remote/Transport/ILink.h +++ b/MobileGL/MG_Remote/Transport/ILink.h @@ -99,26 +99,46 @@ namespace MobileGL::MG_Remote::Transport { // backwards move Fatal (Ring.cpp's AdvanceMonotonic). A stream link changes // only HOW a value arrives, never what it means. // - // WHY THESE SIX AND NOT THE WHOLE PAGE. RingControl's watermark cache line - // today holds {appliedSeq, submittedSeq, retiredSeq, completedFrameSerial, - // presentAckSerial}, and submittedSeq is PRODUCER-written (Ring.h:50, - // "Advanced by the PRODUCER after it publishes") while the other four are - // consumer-written - so the line has MIXED WRITERS and no single-writer - // block can be aliased over it as it stands. eventRingFull and - // eventDropped, which every park predicate on both sides must also see, - // sit a full cache line away in the doorbell/generation group. lk's regroup - // moves submittedSeq out to the producer's own line and promotes the two - // event flags in, so THIS struct becomes a named member of RingControl with - // per-field offsetof static_asserts. Free today, because both peers are the - // same binary and the fingerprint enforces it; a wire break the day a - // second build exists, which is why it is booked now rather than later. + // WHY THESE FOUR AND NOT THE WHOLE PAGE, AND WHY THE EVENT FLAGS ARE NOT + // HERE. RingControl's watermark cache line today holds {appliedSeq, + // submittedSeq, retiredSeq, completedFrameSerial, presentAckSerial}, and + // submittedSeq is PRODUCER-written (Ring.h:50, "Advanced by the PRODUCER + // after it publishes") while the other four are consumer-written - so the + // line has MIXED WRITERS as it stands and no single-writer block can be + // aliased over it. lk's regroup moves submittedSeq out to the producer's + // own line, beside cmdHead, which is the other thing the producer writes; + // what is left IS this struct, and it becomes a named member of + // RingControl with per-field offsetof static_asserts. + // + // THE TWO EVENT FLAGS ARE DELIBERATELY EXCLUDED, and an earlier draft of + // this header had them in and was wrong. eventRingFull has TWO writers: + // the server latches it with store(1) when SEG_EVENT fills + // (EventRing.h:140) and the CLIENT clears it with an exchange(0) on EVERY + // DRAIN (EventRing.h:200). Promoting it here would put a per-drain RMW by + // the client on the same cache line as appliedSeq - which the server + // release-stores every 64 records and the client's spin predicate reads + // 1e5-1e6 times per frame. That is new false sharing on the hottest line + // in the system, and it would land in exactly the numbers lk's gate has to + // hold constant. They stay in the doorbell/generation group, where mixed + // writers already live beside consumerParked/producerParked, and cross the + // seam through EventFlags() instead. + // + // Free to regroup TODAY, because both peers are the same binary and the + // fingerprint enforces it; a wire break the day a second build exists, + // which is why it is booked now rather than later. struct LinkProgress { std::atomic appliedSeq; // records applied std::atomic retiredSeq; // GPU finished std::atomic completedFrameSerial; // releases *RetiredTail / SEG_ADOPT std::atomic presentAckSerial; // returns present credit - std::atomic eventRingFull; // SEG_EVENT full, apply stopped - std::atomic eventDropped; // dropped lossy events + }; + + // The reverse channel's two flags. Separated from LinkProgress for the + // false-sharing reason above, not for a taxonomic one: both roles read + // eventRingFull in a park predicate, and both roles write it. + struct LinkEventFlags { + std::atomic eventRingFull; // server latches, client clears + std::atomic eventDropped; // server-only counter }; // ---- the producer's handle ---------------------------------------------- @@ -197,6 +217,10 @@ namespace MobileGL::MG_Remote::Transport { // an attached link. virtual LinkProgress* Progress() = 0; + // The reverse channel's flags. A separate accessor, and a separate + // cache line, for the reason LinkEventFlags documents. + virtual LinkEventFlags* EventFlags() = 0; + // The command-record arena (producer side) and cursor (consumer side). // Exactly one of the two is valid on a given peer. virtual LinkArena* RecordArena() = 0; diff --git a/MobileGL/MG_Remote/Transport/StreamLink.h b/MobileGL/MG_Remote/Transport/StreamLink.h index 19d53ecb..d47f5a12 100644 --- a/MobileGL/MG_Remote/Transport/StreamLink.h +++ b/MobileGL/MG_Remote/Transport/StreamLink.h @@ -72,6 +72,7 @@ namespace MobileGL::MG_Remote::Transport { LinkCapabilities Capabilities() const override; LinkProgress* Progress() override; + LinkEventFlags* EventFlags() override; LinkArena* RecordArena() override; LinkCursor* RecordCursor() override; diff --git a/docs/Disaggregated/notes/p6/a6-link-experiment.md b/docs/Disaggregated/notes/p6/a6-link-experiment.md index 7798abe4..aabb70a4 100644 --- a/docs/Disaggregated/notes/p6/a6-link-experiment.md +++ b/docs/Disaggregated/notes/p6/a6-link-experiment.md @@ -202,7 +202,15 @@ string(SUBSTRING "${GIT_COMMIT_HASH_FULL}" 0 7 GIT_COMMIT_HASH_SHORT) 设计时**想过**空值——但只想到"空串是一个与真 stamp 不同的输入"。 没想到的是**构建会静默地把所有构建的 stamp 都变成同一个空串**。 于是：**两份来自不同 commit、都在 git 取不到版本的环境里构建的库，ABI 指纹相同**， -而指纹里唯一能看见字段重排的那一项就此失效——没有任何日志、任何门会说一声。 +而指纹里唯一能看见字段重排的那一项就此失效。 + +> **更正（`lk` 跑基线测试时发现）。** 本节原先写的是"没有任何日志、任何门会说一声"。**这句是错的，** +> 门是存在的：`MG_Test/Wire/SessionHandshakeTest.cpp:140` 断言 `inputs.BuildStamp[0] != '\0'`， +> `:180` 断言扰动 `BuildStamp = ""` 必须改变指纹。在这个配置下**两条都实测为红**： +> `Expected: (inputs.BuildStamp[0]) != ('\0'), actual: '\0' vs '\0'`。 +> 所以项目预见到了这个情形，本节的发现因此更强而不是更弱——**不是"没人会注意到"， +> 是"会注意到它的门已经存在，而且现在就是红的"**。真正静默的是**构建期**：CMake 无声地产出空串， +> 红只在跑测试时才出现。`hs` 要补的是构建期的具名失败，不是一道新门。 这不是一个奇异场景。会命中它的至少有：worktree 跨 WSL 构建（已实测）、 源码 tarball 构建、CI 上不带 `.git` 的 checkout、把 repo 拷进去的 Docker 构建。 diff --git a/docs/Disaggregated/notes/p6/lk-itemisation.md b/docs/Disaggregated/notes/p6/lk-itemisation.md new file mode 100644 index 00000000..e3b70847 --- /dev/null +++ b/docs/Disaggregated/notes/p6/lk-itemisation.md @@ -0,0 +1,55 @@ +# lk — 开工前的逐文件清单 +> 头 `0bbe41c2`。`CONTRACT-P6.md` §8.5 要求 `lk` 在开工前发布这份清单，理由是早先一份草案的棘轮基线 +> 差了三倍，而**装在错误数字上的棘轮第一次 CI 就会因为和泄漏无关的理由变红**。 +> 全部计数由 `grep -rn -E` 在本头机械产出，口径写在每节标题下。 + +## A 纯度门棘轮基线：`Transport/` 之外的缝标识符 + +口径：`<标识符>`，排除 `MobileGL/MG_Remote/Transport/` 自身。`lk` 之后这些必须只出现在 `Transport/ShmLink.*` 与传输自己的测试里。 + +| 文件 | 类别 | 合计 | 明细 | +|---|---|---:|---| +| `MG_Test/Wire/SessionTest.cpp` | test | 55 | RingControl×16 ReplySlotPool×10 SessionSegments×9 RingProducer×4 RingConsumer×4 SessionConsumer×4 EventRingConsumer×3 SessionProducer×3 EventRingProducer×2 | +| `MG_Test/Wire/RingTest.cpp` | test | 28 | RingControl×11 RingProducer×9 RingConsumer×8 | +| `MG_Remote/Client/ClientSession.cpp` | prod | 26 | RingControl×7 EventRingConsumer×7 RingProducer×5 SessionProducer×4 ReplySlotPool×2 SessionSegments×1 | +| `MG_Remote/Wire/PipeWireCodec.h` | prod | 19 | RingControl×11 RingProducer×6 RingConsumer×1 SessionConsumer×1 | +| `MG_Test/Wire/PipeWireCodecTest.cpp` | test | 19 | RingProducer×7 RingConsumer×6 RingControl×5 SessionConsumer×1 | +| `MG_Remote/Server/ServerSession.cpp` | prod | 17 | RingControl×4 RingConsumer×4 EventRingProducer×3 SessionConsumer×3 ReplySlotPool×1 SessionSegments×1 SessionProducer×1 | +| `MG_Remote/Client/ClientSession.h` | prod | 14 | RingControl×3 SessionProducer×3 RingProducer×2 ReplySlotPool×2 EventRingConsumer×2 SessionSegments×2 | +| `MG_Remote/Wire/PipeWireCodec.cpp` | prod | 14 | RingControl×6 RingProducer×4 RingConsumer×3 SessionConsumer×1 | +| `MG_Test/Wire/ServerLoopTest.cpp` | test | 13 | RingControl×4 RingProducer×3 EventRingConsumer×2 ReplySlotPool×1 SessionSegments×1 SessionProducer×1 SessionConsumer×1 | +| `MG_Remote/Server/ServerLoop.cpp` | prod | 12 | SessionConsumer×6 RingControl×2 RingConsumer×2 RingProducer×1 EventRingConsumer×1 | +| `MG_Remote/Server/ServerSession.h` | prod | 11 | EventRingProducer×3 RingControl×2 RingConsumer×2 SessionSegments×2 SessionConsumer×2 | +| `MG_Remote/Server/PipeApplier.cpp` | prod | 5 | SessionConsumer×3 RingControl×1 ReplySlotPool×1 | +| `MG_Remote/Server/PipeApplier.h` | prod | 3 | RingControl×2 SessionConsumer×1 | +| `MG_Test/Wire/RemoteClientTest.cpp` | test | 3 | ReplySlotPool×3 | +| `MG_IntegrationTest/Harness/WireLedgerChecks.h` | test | 2 | RingProducer×2 | +| `MG_Util/Metrics/PipeStats.h` | prod | 2 | RingProducer×1 SessionProducer×1 | +| `Config.h` | prod | 1 | RingProducer×1 | +| `MG_IntegrationTest/Harness/SplitRuntimePeek.cpp` | test | 1 | RingProducer×1 | +| `MG_IntegrationTest/Harness/SplitRuntimePeek.h` | test | 1 | RingProducer×1 | +| `MG_Remote/Client/EmitTables.cpp` | prod | 1 | SessionProducer×1 | +| `MG_Remote/Client/EmitTables.h` | prod | 1 | ReplySlotPool×1 | +| `MG_Remote/Client/GpuWritePending.h` | prod | 1 | RingProducer×1 | +| `MG_Test/Wire/InProcessTransportTest.cpp` | test | 1 | RingControl×1 | + +**合计：生产 127 / 测试 123，共 250 处，分布在 23 个文件。** + +## B 重组影响面：`control.<字段>` 成员访问 + +口径：`(control|m_control|ctrl)(->|.)<字段>`，七个字段 = 四个 watermark + `submittedSeq` + 两个 event 标志。 +这是把 watermark 组变成具名 `LinkProgress` 成员后**必须逐处改写**的集合。 + +| 文件 | 类别 | 处数 | +|---|---|---:| +| `MG_Remote/Transport/Ring.cpp` | transport | 14 | +| `MG_Test/Wire/RingTest.cpp` | test | 10 | +| `MG_IntegrationTest/Harness/SplitRuntimePeek.cpp` | test | 5 | +| `MG_Remote/Wire/PipeWireCodec.cpp` | prod | 4 | +| `MG_Remote/Client/ClientSession.cpp` | prod | 2 | +| `MG_Remote/Server/ServerLoop.cpp` | prod | 2 | +| `MG_Test/Wire/PipeWireCodecTest.cpp` | test | 2 | +| `MG_Remote/Server/ServerSession.cpp` | prod | 1 | +| `MG_Test/Wire/SessionTest.cpp` | test | 1 | + +**合计 41 处**（传输内 14 / 生产 9 / 测试 18）。