From 7c822cdc85119823013b2987fd03fcd3d3427840 Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Sat, 26 Sep 2026 01:13:31 +0800 Subject: [PATCH] [Docs] (P12): the no-reply flag reviewed - the arithmetic holds and the flag is cheap, but the predicate has to be "this answer will never be read", not "fire-and-forget" An independent review of the one design door the measurement left open (stop sending the ~51,000 answers nobody reads, so the 8-slot pool stops churning and the create window becomes collectable). Verdict: feasible, worth doing, and the proposal AS WRITTEN would disable the thing it exists to enable. THE CATCH. The windowed create is wantReply=false AND its answer is read - WireTables.cpp's drain reads exactly those seqs. A bit keyed on "fire-and-forget" therefore stops the server answering the creates the window is waiting for: the window goes from "cannot be collected" to "there is nothing to collect", and the readfail counter that is the only way to diagnose it disappears with it. It fails silently, not loudly. The predicate has to be per-record "this answer will never be read" - true of params / respecify-escape / sub-data, false of a windowed create - carried as a separate input from wantReply (willReadReply), with willReadReply && !rowCarriesReplySlot rejected rather than ignored. VERIFIED HERE, not taken on trust (the reviewer's file:line claims, checked): - the flag space is RingRecordFlags (Ring.h:281-285), bits 0-4 taken, BIT 5 FREE, and the header's own table (PipeWire.inc:35-65) says "bit 5 kOptional vs (nothing) no ring counterpart" - so a new ring bit aliases kOptional, which is exactly what the assert at PipeWireCodec.cpp:142-145 refuses; - the three static_asserts that will trip are real and deliberate (PipeWireCodec.cpp:106, :142-145, RingTest.cpp:740); - WireFingerprint mixes payload-layout, catalogue, render-state digests + OpCount, none of which a ring bit moves - so no protocol revision bump is required, and both cross-version directions degrade to today's behaviour. That makes this "must upgrade both ends to take EFFECT", not "must upgrade both ends not to BREAK" - one notch cheaper than handoff-2 section 3.2's protocol surface; - StreamLink.cpp:564-571 is the independent progress flush (>= 64 records or >= 1 ms) and :577 is PostReply piggybacking SendProgress on the reply - so skipping replies coarsens progress granularity. Bounded, but unmeasured. AND IT CORRECTED TWO OF MY OWN NUMBERS. The frame's reply count is 55,903, not 59,673: op 4 ResourceDestroy carries kNone in the catalogue (PipeWire.inc:179) and PostReply Fatals on a row with no kReplySlot, so it never answers - I had added its 3,770 records to the total. The derived figures move with it (12.3 replies per create, 0.65 of a create of survival, 6,988 pool churns), and every copy is fixed here: Config.h, ConfigLoader.cpp, WireTables.cpp's section comment, CONTRACT-P5E.md, CREATE-WINDOW-MEASURED.md. TWO MORE THINGS THE FOLLOW-UP MUST CARRY, both recorded in the note's section 7: - LinkMetricsBeginReply counts ownsReplySlot, so once the flag lands by_op 2 falls to ~0 - and section 1 of the note tells the reader to use by_op 2 to check whether the window engaged. That check becomes a FALSE GREEN; the flag needs a server-side "how many times did this op answer" counter beside it. - the ~112,000 sendmsg/frame the flag removes are on the apply thread, so the flag's own value is a measurable claim that nobody has measured. 172/172 and 13/13 after the comment corrections. --- MobileGL/Config.h | 13 +++-- MobileGL/ConfigLoader.cpp | 2 +- MobileGL/MG_Remote/CONTRACT-P5E.md | 4 +- MobileGL/MG_Remote/Client/WireTables.cpp | 16 ++++-- .../notes/p12/CREATE-WINDOW-MEASURED.md | 54 ++++++++++++++----- 5 files changed, 63 insertions(+), 26 deletions(-) diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 35dc9fe8..ef2e8783 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -551,16 +551,19 @@ namespace MobileGL::MG_Config { // MOBILEGL_IPC_CREATE_WINDOW: how many RESOURCE_CREATE answers may be outstanding before the // client waits for one. 1 = every create waits, which is the shape R-5 shipped with AND THE // DEFAULT, because the deferral was measured on the device and does not work: the reply pool - // is 8 slots and one load frame posts 59,673 replies (create 4,536 + respecify 20,633 + - // params 18,056 + sub-data 12,671 + the rest), so an answer is overwritten after 8 replies - - // 0.61 of a create - and the window's drain can never read one back. All 4,536 creates took + // is 8 slots and one load frame posts 55,903 replies (create 4,536 + respecify 20,633 + + // params 18,056 + sub-data 12,671 + 7), so an answer is overwritten after 8 replies - + // 0.65 of a create - and the window's drain can never read one back. All 4,536 creates took // the blocking path at 4 and at 1 alike (28.64 s both ways). Worse than inert: the // provisional accept latches an object the applier may have refused, and the un-latch that // corrects it lives in that same unreadable drain. WireTables.cpp's section comment has the // counters and the arithmetic, and the gates in RemoteClientControls.inc still pin the // mechanism, which is sound and simply unusable against this much other reply traffic. - // N > 1 is reachable for the experiment that would make it usable (a deeper pool, or a way - // to tell the server not to answer a fire-and-forget row). + // N > 1 is reachable for the experiment that would make it usable (a deeper pool, or a + // per-record "this answer will never be read" bit). NOTE THE PREDICATE: NOT + // "fire-and-forget" - the window's own creates ARE fire-and-forget and the drain reads + // them, so a flag keyed on wantReply would kill the very thing it is meant to enable + // (docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md section 7). Uint32 CreateWindow = 1; // MOBILEGL_IPC_ADOPT_TIER: 2 = emulate (client keeps the shadow and pushes), which // is the only tier P5 implements and the reason persistent-map-push can be non-zero diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index 0784eb68..bd7db550 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -413,7 +413,7 @@ namespace MobileGL::MG_ConfigLoader { ipc.PersistentHashSuppress = QueryEnvUint32("MOBILEGL_IPC_PERSISTENT_HASH_SUPPRESS", 1, 0, 1); ipc.BatchWaits = QueryEnvUint32("MOBILEGL_IPC_BATCH_WAITS", 1, 0, 1); // The create window. DEFAULT 1 - OFF - because the deferral was measured on the device and - // cannot be collected (Config.h has the numbers, WireTables.cpp the arithmetic: 59,673 + // cannot be collected (Config.h has the numbers, WireTables.cpp the arithmetic: 55,903 // replies per frame into 8 pool slots). Capped at 4 rather than the pool's 8 for the reason // the mechanism is sized that way at all. ipc.CreateWindow = QueryEnvUint32("MOBILEGL_IPC_CREATE_WINDOW", 1, 1, 4); diff --git a/MobileGL/MG_Remote/CONTRACT-P5E.md b/MobileGL/MG_Remote/CONTRACT-P5E.md index a57de110..88a28021 100644 --- a/MobileGL/MG_Remote/CONTRACT-P5E.md +++ b/MobileGL/MG_Remote/CONTRACT-P5E.md @@ -231,9 +231,9 @@ handle", and it is what `MGPipeResourceRespecifyWantsItsReply` (`PipeRoute.h`) a does not hold. It was given a WINDOW instead of a rule - `MOBILEGL_IPC_CREATE_WINDOW`, answers deferred and collected in order, a late DECLINE un-latching the object - and **the window is OFF BY DEFAULT because the deferral was measured and cannot be collected**: the reply pool is - `kDefaultReplySlotCount` = 8 slots deep and one load frame posts 59,673 replies (the server + `kDefaultReplySlotCount` = 8 slots deep and one load frame posts 55,903 replies (the server answers every reply-owning record whether or not the client wants the answer), so an answer is - overwritten after 8 replies - 0.61 of a create. On the device all 4,536 creates took the blocking + overwritten after 8 replies - 0.65 of a create. On the device all 4,536 creates took the blocking path at window 4 and at window 1 alike (28.64 s both ways, against the handoff's 31.3 s pre-window baseline on the same device). `WireTables.cpp`'s section comment carries the counters and the arithmetic. The client-visible consequence is worth stating here: with the window ON, the diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index ee49d704..f2e130ce 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -347,13 +347,14 @@ namespace MobileGL::MG_Remote::Client { // slot is reused, and the pool is kDefaultReplySlotCount = 8 slots deep. The frame posts a // reply for EVERY reply-owning record - not only for the rows that wait, because the server // cannot know which those are (ClientSession.cpp:1897-1900: "The server posts either way") - // - and that is 59,673 replies in this one frame: + // - and that is 55,903 replies in this one frame (op 4 ResourceDestroy is NOT counted: its + // catalogue flags are kNone, so PostReply would Fatal on it and it never answers): // // create 4,536 + respecify 20,633 + params 18,056 + sub-data 12,671 + 3,770 + 6 + 1 // - // 8 slots against 59,673 replies is a pool that churns 7,459 times per frame, i.e. 13.2 + // 8 slots against 55,903 replies is a pool that churns 6,988 times per frame, i.e. 12.3 // replies between two creates: an answer posted at create N is overwritten after EIGHT - // replies, which is 0.61 of a create. There is no window depth for which the deferred + // replies, which is 0.65 of a create. There is no window depth for which the deferred // answer still exists when the client comes back for it - the deferral is not slow, it is // UNREADABLE. (A 1-deep "window" is the blocking path, which is what the row does.) // @@ -364,8 +365,13 @@ namespace MobileGL::MG_Remote::Client { // prevent. So the default is 1: the row is the pre-window shape unless someone sets // MOBILEGL_IPC_CREATE_WINDOW explicitly, and the machinery below is kept as the record of // what was tried and what it cost (a fix would have to make a deferred answer outlive - // 59,673 intervening replies, which means a deeper pool or a way for the client to tell - // the server "do not answer this one" - both are wire changes, not tuning). + // 12.3 intervening replies, which means a deeper pool, or NOT sending the ~51,000 answers + // nobody will read - and that second one needs its predicate stated exactly, because + // "fire-and-forget" is the WRONG one: the window's own creates are fire-and-forget and + // the drain above reads them, so a bit keyed on wantReply disables the mechanism it is + // meant to enable. The bit has to mean "this answer will never be read" - which is true + // of respecify/params/sub-data and false of a windowed create. See + // docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md section 7, reviewed). // // The gates that pin it are in MG_Test/Wire/RemoteClientControls.inc, and they still hold: // the window is a coherent mechanism, it is simply unusable on a frame with this much diff --git a/docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md b/docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md index 40b18c79..bce7b5f0 100644 --- a/docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md +++ b/docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md @@ -9,8 +9,8 @@ - **窗口开了等于没开**：同一台设备、同一帧、同一 101,421,396 B staged，`MOBILEGL_IPC_CREATE_WINDOW=4` 与 `=1` 的 create 行都是 **4,536 次等待**，墙钟都是 **28.64 s**（handoff 的 pre-window 基线是 31.3 s）。 临时计数器给出的形态是：`pushed=24 blocked=4,476 took=20 readfail=8,951 pending=4`——**drain 读不回来**。 -- **不是调参问题**：回复池 8 槽，而这一帧发出 **59,673 条带回复的记录**（服务端对每条 kReplySlot 记录都回， - 不管客户端要不要），所以一条答案在 **8 条回复**之后就被覆盖，等于 **0.61 个 create**。任何深度的窗口都读不回来。 +- **不是调参问题**：回复池 8 槽，而这一帧发出 **55,903 条带回复的记录**（服务端对每条 kReplySlot 记录都回， + 不管客户端要不要），所以一条答案在 **8 条回复**之后就被覆盖，等于 **0.65 个 create**。任何深度的窗口都读不回来。 - **顺带修掉一个真 bug**：`StreamLink::ReadReply` 只认「最新一条」（单槽），违背 `ILink::ReadReply(seq)` 的按 seq 读取语义。已改成一个 8 槽环 + 用例。 - **默认关掉**：`MOBILEGL_IPC_CREATE_WINDOW` 默认 1。窗口开着不只是白费，它会让**被拒绝的 create 留在「已发布」状态** @@ -41,25 +41,27 @@ P12diag create=4501 cfg=4 eff=4 suspects=0 refusals=0 pending=4 pushed=24 blocke `pending` 从第 500 条起**一直钉在 4**，`refusals=0`（一条答案都没读到过），`readfail` 约每条 create 涨 2 （非阻塞 poll + 阻塞 drain 各失败一次）。 -## 2. 根因：回复池 8 槽 vs 一帧 59,673 条回复 +## 2. 根因：回复池 8 槽 vs 一帧 55,903 条回复 答案只在**它的槽没被复用**之前可读（`ReplySlotPool`，`kDefaultReplySlotCount = 8`， -`Transport/ReplySlot.h:111`）。而这一帧发回复的记录数是： +`Transport/ReplySlot.h:111`）。而这一帧发回复的记录数是（**只数带 `kReplySlot` 的行**—— +`MG_Pipe/generated/PipeWire.inc:177-179` 的 flags 列，op 4 `ResourceDestroy` 是 `kNone`， +`PostReply` 对它直接 Fatal，从来不回；本文第一版把它的 3,770 条算了进来，审查纠正为下表）： | 行 | 条数 | 为什么回 | |---|---|---| | 2 `ResourceCreate` | 4,536 | 等 | | 3 `ResourceRespecify` | 20,633 | **不等也给**（首用确认后 fire-and-forget） | | 47 `SetTextureParams` | 18,056 | **不等也给** | -| 48 `ResourceSubData` + 4 | 16,441 | **不等也给** | +| 48 `ResourceSubData` | 12,671 | **不等也给** | | 5 / 9 等 | 7 | 等 | -| **合计** | **59,673** | | +| **合计** | **55,903** | | 服务端**无法知道**客户端要不要这条答案——`ClientSession.cpp:1897-1900` 原文：「The server posts either way, -so the wire is unchanged and the row keeps its flag」。于是池子一帧churn **7,459 次**，两条 create 之间平均夹 -**13.2 条回复**，而一条答案只活得下 **8 条**： +so the wire is unchanged and the row keeps its flag」。于是池子一帧 churn **6,988 次**，两条 create 之间平均夹 +**12.3 条回复**，而一条答案只活得下 **8 条**： - 8 / 13.2 = 0.61 个 create + 8 / 12.3 = 0.65 个 create **所以窗口的失败不是「慢」，是「读不到」。** 1 深度的窗口就是阻塞路径本身。 @@ -115,10 +117,36 @@ if (x.replySeq != seq) return ... MOBILEGL_ERR_PROTOCOL_MISMATCH; // 等的是 要让延后的答案活下来，只有两条路，**都是协议面而不是调参**： 1. **加深回复池**：`ReplySlotCount` 在 Hello 里协商（`ClientSession.cpp:957` / `ServerSession.cpp:745`）， - 两端一致即可改。需要的深度 \>= 两条 create 之间的回复数（本帧 13.2）乘以窗口深度，即 4 深度要 ~53 槽； - 而 `ReplyBytes` 固定 16 MiB 时槽会变小，`ReadPixels` 的 `ReplyCanHold` 会先撞墙，所以要一起抬 `ReplyBytes`。 -2. **让客户端能说「这条别回」**：四个 Bool 行（create/respecify/params/sub-data）目前服务端一律回； - 一个 record flag 就能把 59,673 砍到 ~4,543，那时 8 槽够用。这是 wire 改动，两端都要动。 + 两端一致即可改。需要的深度 \>= 两条 create 之间的回复数（本帧 12.3）乘以窗口深度，即 4 深度要 ~49 槽； + 而 `ReplyBytes` 固定 16 MiB 时槽会变小（16 MiB / 64 ≈ 256 KiB），`ReadPixels` 的 `ReplyCanHold` + 会先撞墙，所以要一起抬 `ReplyBytes`（W=4 保 2 MiB 上限约需 128 MiB）。**注意这是 8× 常驻内存**， + 而且评审指出服务端 SEG_REPLY 的实际分配路径**没人读过**——走这条之前先读它。 +2. **让服务端别回那些没人会读的答案**：四个 Bool 行（create/respecify/params/sub-data）目前一律回， + 其中约 51,000 条的答案是**没有人会读**的。砍到 ~4,543 之后两条 create 之间只剩 ~1 条回复，8 槽够用。 + + **判据必须是「这条的答案永远不会被读」，不是「fire-and-forget」。** 这两者今天在 4 个站点里 3 个重合， + **在最重要的那一个上正好相反**：window 延后的那些 create **正是** `wantReply=false`，而 §1 的 drain + 读的就是它们。按字面实现会让窗口从「读不回来」变成「根本没得读」，而且连 `readfail` 这个唯一能诊断它的 + 计数器一起消失——不会崩，只会静默退化。（这条是独立评审抓到的；本页 §7 的第一版写法就是那个错判据。） + + 三件已经核过的事： + + - **位有空位**：`MGPWireRecHeader::Flags` 是链路 framing 空间 `RingRecordFlags`（**不是** `MGPipeCallFlags`）， + `Ring.h:281-285` 用掉 bit 0-4，**bit 5 空**（`PipeWire.inc:35-65` 的对照表自己写着「bit 5 kOptional vs（无）」）。 + - **wire fingerprint 不动**：`WireFingerprint()` 混的是 payload 布局 digest + PipeCalls.def 的 catalogue + digest + 渲染状态 + OpCount，加一个 ring 位三个都不动 ⇒ **不必 bump `MOBILEGL_PROTOCOL_CONTROL_REVISION`**。 + 但 `PipeWireCodec.cpp:106` / `:142-145` 与 `RingTest.cpp:740` 三条 `static_assert` **会红**——那是故意的 + 绊线（新 ring 位正好别名 `kOptional`）。 + - **两个方向都退化成今天的行为**（旧端忽略 bit 5 照旧回包；旧客户端从不设位）⇒ 它**不是** handoff-2 §3.2 + 那种「不升级就会坏」的协议面，而是**「不两端都升就不会生效」**的协议面——便宜一档，客户端可以先落。 + + 还有一处**必须同改**：`LinkMetricsBeginReply` 计的是 `ownsReplySlot`（`ClientSession.cpp:1883`），window + 生效后 **`by_op 2` 会掉到 ~0**，而 §1 正是教人用 `by_op 2` 判断窗口有没有生效——它会变成**假绿**。 + 验证这个提案要另加「服务端为这条 op 回了多少次」的计数。 + + （本节的两条路都经过一次独立评审。评审指出一处**未实测**：`StreamLink::PostReply` 把 `SendProgress()` + 搭在回包上（`StreamLink.cpp:577`），跳过回包会把进度粒度从「每条一次」降到「>= 64 条或 >= 1 ms 一次」 + （`:564-571`）——水位仍独立推进，但 barrier 的平均多等量没人算过。） ## 8. 复算