diff --git a/MobileGL/Config.h b/MobileGL/Config.h index ef2e8783..deb2c225 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -549,22 +549,37 @@ namespace MobileGL::MG_Config { // restores the per-record barrier of R-1. Uint32 BatchWaits = 1; // 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 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 - // 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; + // client waits for one. 1 = every create waits (the shape R-5 shipped with); N > 1 = up to N + // creates go out fire-and-forget, their caller reads a PROVISIONAL accept, and the oldest + // answer is taken when the window fills. + // + // DEFAULT 2, AND IT TOOK THREE THINGS TO GET HERE. The deferral was inert for two + // measured reasons and one structural one, in the order they were found: + // 1. the reply buffer was a ring of the last 8 answers, and between two creates this + // client emits on the order of a thousand records - so the answer was always gone + // (`want=310` on every miss while the ring spanned [1175..1504]); + // 2. a residency budget that tried to take answers back early could not help, because + // what displaces an answer is what the SERVER sends, not what the client emits; + // 3. and so the fix was RETENTION - a link keeps a declared answer until its reader + // takes it (Transport/ILink.h's RetainsReplies, StreamLink's declared-answer store). + // + // MEASURED ON THE DEVICE, load frame, both ends rebuilt (create 4,536, records 103,097, + // frame-sent identical per op throughout): + // window 1 4,536 blocking takes 28.66 s (the shape R-5 shipped) + // window 2 2,500 17.27 s + // window 4 1,644 11.02 s + // and the round trip does NOT grow as the window deepens (5,285 / 4,082 us against 5,672 + // at 1), which is the measurement that says the ~5 ms is a WAKEUP the server batches away + // rather than its apply throughput - so amortising four creates per wakeup is a real 2.6x. + // + // 4 IS REACHABLE AND BETTER (11.02 s against 17.27 s) and is NOT the default only because + // the wider window holds more answers at once; raise it after a run that reads `by_op 2` + // and the wall clock rather than the config. NOTE THE PREDICATE the whole mechanism rests + // on: an answer is retained because the wire DECLARED it will be read - NOT because its + // caller was fire-and-forget. The window's own creates are fire-and-forget AND read, so a + // flag keyed on wantReply would disable the very mechanism it exists to enable + // (docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md sections 7-11). + Uint32 CreateWindow = 2; // 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 // at all (R-6). 0 and 1 parse and are Fatal at use with "P11"; they exist now so the diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index bd7db550..ee39af03 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -412,11 +412,12 @@ namespace MobileGL::MG_ConfigLoader { // xxHash64 changed since the last push are shipped. 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: 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); + // The create window. DEFAULT 2, and it works because a link that declares RetainsReplies + // keeps a declared answer until its reader takes it - Config.h carries the three reasons it + // was inert before and the device numbers (28.66 s at 1, 17.27 s at 2, 11.02 s at 4). + // Capped at 4: the retained set is bounded by the window's own depth, and 4 is what the + // declared-answer store's capacity is written against. + ipc.CreateWindow = QueryEnvUint32("MOBILEGL_IPC_CREATE_WINDOW", 2, 1, 4); // The verify harness compares the pushed block against the applier per verb; a // batched queue lets the comparer read a supplied field mid-apply, which is a // torn read rather than a divergence. The batch is therefore off whenever the diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index 2a9f194e..d7ba99ab 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -1898,10 +1898,17 @@ namespace MobileGL::MG_Remote::Client { // creates or parameter sets, and those three have different fixes. const auto replyMetricStart = Transport::LinkMetricsBeginReply(ownsReplySlot, static_cast(op)); - // P12: the residency budget's clock. Placed HERE, where the record is certainly - // published, rather than beside rowCarriesReplySlot: a call that declined before - // encoding emits nothing and must not advance it. + // P12: the residency clock AND the declaration, both placed HERE - after EncodeRecord has + // an ordinal and BEFORE the publish, which is the only window in which the declaration is + // guaranteed to precede its own answer. A call that declined before encoding emits nothing + // and must not advance either. if (rowCarriesReplySlot) ++m_replyPostings; + // THE DECLARATION THAT LETS A LINK RETAIN THIS ANSWER (P12). willReadReply, not wantReply: + // the create window emits without waiting and reads afterwards, so its records must be + // declared even though this call does not read them. Links that cannot retain ignore it, + // and the window does not defer on them (see ClientSession::LinkRetainsReplies). + if (rowCarriesReplySlot && willReadReply && m_link != nullptr) + m_link->DeclareReplyRead(seq); m_producer.PublishAndNotify(seq); if (op == MG_Pipe::MGPWireOp::Present) { // A credit is permission to run ahead, not permission to retain the @@ -2464,6 +2471,10 @@ namespace MobileGL::MG_Remote::Client { Uint64 ClientSession::ReplyPostings() const { return m_replyPostings; } + Bool ClientSession::LinkRetainsReplies() const { + return m_link != nullptr && m_link->Capabilities().RetainsReplies; + } + Bool ClientSession::ReadReply(Uint64 seq, void* outBytes, Uint64 outCapacity, Int32* outStatus, Uint64* outSize) { const void* bytes = nullptr; Uint64 size = 0; Int32 status = 0; diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index 4737e430..e9337ead 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -392,6 +392,13 @@ namespace MobileGL::MG_Remote::Client { // deployment where it matters most. Uint64 ReplyPostings() const; + // P12: DOES THIS LINK KEEP AN ANSWER UNTIL ITS READER TAKES IT? The create window defers + // answers across an unbounded amount of other traffic, which only works where the answer + // is still there when the drain comes back - measured: a bounded buffer of the last N + // answers never is. On a link that answers false the window must not defer at all, so + // this is what CreateWindowEffective() is gated on. + Bool LinkRetainsReplies() const; + // The reply slot for `seq`, addressed seq % slots with the seq stamped back into the // header for self-check (R-3). `outStatus` is 0 OK / 1 DECLINED / 2 ERROR, and // DECLINED IS A REAL ANSWER - MapPersistent's nullptr and the four Bool acceptances. diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index 51650382..948f4626 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -416,10 +416,7 @@ namespace MobileGL::MG_Remote::Client { Uint64 Seq = 0; MG_Pipe::MGPipeKind Kind = MG_Pipe::MGPipeKind::None; MG_Pipe::MGPipeHandle Handle{}; - // P12: ClientSession::ReplyPostings() at the moment this entry was pushed. The - // difference between it and the live count is how many answers could have displaced - // this one - see the residency budget below. - Uint64 ReplyStamp = 0; + }; PendingCreate g_createWindow[kCreateWindowMax]; Uint32 g_createWindowHead = 0; @@ -432,7 +429,6 @@ namespace MobileGL::MG_Remote::Client { // answers from existing at all - the second is what a no-reply bit keyed on wantReply // would produce, and it would leave every other reading green. Uint64 g_createWindowTaken = 0; - Uint64 g_createWindowBudgetStops = 0; // A HANDLE WHOSE CREATE CAME BACK DECLINED IS NEVER WINDOWED AGAIN. Without this the window // would UNDO the repair it exists to keep: every later use of the object sends another create, @@ -507,7 +503,21 @@ namespace MobileGL::MG_Remote::Client { // match yet (or when the answer does not fit the scratch), and treating that as // ERROR would invent a transport fault out of a reader that arrived early. The // entry stays in the window and is taken at the next drain. - if (!session.ReadReply(head.Seq, nullptr, 0, &status, &size)) return; + if (!session.ReadReply(head.Seq, nullptr, 0, &status, &size)) { + // REACHED BUT UNREADABLE IS AN INVARIANT, NOT A RETRY (P12). The link declared + // it retains this answer, and on the stream arm the reply frame strictly + // precedes the progress frame that published appliedSeq - so by the time the + // wait above returns, the answer is in hand or something is broken. Retrying + // (what this used to do) wedges the window for the rest of the process and + // leaves every deferred object latched on a provisional accept; recovering by + // re-latching would lose texels. So it names itself instead. + SessionFail(MGFatalFamily::ReplyMissing, + "MGPipe: Fatal{ReplyMissing, \"create window\"} - seq %llu was " + "declared and APPLIED, and its answer is no longer in the link's " + "store; the window cannot advance past it and nothing recovers a " + "lost answer", + static_cast(head.Seq)); + } // ERROR IS NOT A DECLINE (M4 / R-5), and a deferred answer does not change that: // the same named Fatal the blocking take raises. if (status == Wire::ReplySink::kStatusError) { @@ -553,61 +563,40 @@ namespace MobileGL::MG_Remote::Client { if (status == 1) ++g_declined; if (status == 0) ClearCreateSuspect(payload->Resource); } - // ---- P12: THE RESIDENCY BUDGET -------------------------------------------------------- + // ---- P12: WHY THERE IS NO RESIDENCY BUDGET HERE ANY MORE ------------------------------- // - // A deferred answer is readable only while the reply buffer has not reused its slot, and - // that buffer is bounded by construction: kDefaultReplySlotCount slots in the shared pool, - // and the stream link's ring is that same depth (StreamLink.cpp's kReplyRing, whose - // comment says why). So the window may defer only an answer it can still read back, and - // what it measures that against is HOW MANY REPLY-OWNING RECORDS IT HAS EMITTED since the - // oldest entry was pushed - an upper bound on the answers that could have displaced it. + // There was one, and it was measured: it counted the reply-owning records this client had + // emitted since the oldest window entry was pushed and took that entry back when the count + // approached the reply buffer's depth. On the device it fired on essentially EVERY create + // (budget=4,451 of 4,501 calls) and the drain still read nothing back (taken=63, + // readfail=8,872, by_op 2 = 9,071, wall 27.9 s). The clock was wrong, structurally: // - // ONE LESS THAN THE BUFFER, because the eighth newer answer is the one that displaces it. + // * the window only gets control WHEN A CREATE ARRIVES, and between two creates this + // client emits on the order of a thousand records - at the FIRST failed read the reply + // ring already spanned [1175..1504] while the head was seq 310; + // * what displaces a deferred answer is not what this client emits, it is WHAT THE + // SERVER ANSWERS, and the server is behind: its late answers to the earlier creates + // are what fills a bounded buffer first; + // * so the check fired too late by construction. Tightening it only removed the + // deferral entirely, which is what pushed=65 was. // - // THIS IS PREVENTION, NOT REPAIR, AND THAT IS THE WHOLE POINT. The alternative - read the - // head, and when the read fails call the object unknown - UN-LATCHES a create that very - // probably SUCCEEDED, and the server's create starts that object's record over - // (PipeApply.cpp:2033: `*record = MGPipeResourceRecord{}`), PendingUploads included - // (PipeApply.h:332); the client cleared its dirty flags at EMISSION (PipeApply.h:298-304), - // so nothing re-sends those texels. Silent content loss for a healthy object. An answer - // read inside its residency never has to be given up on at all. - // ---- MEASURED: NECESSARY BUT NOT SUFFICIENT, AND THE CLOCK IS WHY --------------------- - // - // On the device this fires on essentially every create (budget=4,451 of 4,501 calls) and - // the drain still reads nothing back (taken=63, readfail=8,872, by_op 2 = 9,071, wall - // 27.9 s). The clock is wrong, and the reason is structural rather than a bad constant: - // - // * THE WINDOW ONLY GETS CONTROL WHEN A CREATE ARRIVES. Between two creates this client - // emits on the order of a thousand records (measured: at the FIRST failed read the - // reply ring already spanned [1175..1504] while the head was seq 310), and nothing - // looks at the window during that gap. - // * WHAT DISPLACES A DEFERRED ANSWER IS NOT WHAT THIS CLIENT EMITS, IT IS WHAT THE - // SERVER ANSWERS. The server is behind - one answer per ~5 ms wakeup while the client - // runs ahead - so the replies arriving during the gap are its LATE ANSWERS TO THE - // EARLIER CREATES, and they fill the 8-slot buffer before the client comes back. - // * SO THIS CHECK FIRES TOO LATE BY CONSTRUCTION: it is consulted at the next create, - // and by then the answer is already gone. Tightening the constant does not fix that; - // it only removes the deferral entirely, which is what `pushed=65` is. - // - // A DEFERRAL IS THEREFORE READABLE ONLY IF THE CLIENT KEEPS THE ANSWERS IT DEFERRED. The - // reply buffer is bounded by COUNT (8) and the client cannot bound how many answers the - // server will post between two of its creates, so hoping the buffer still holds one is not - // a policy - it is a race the client loses. The shape that can work is RETENTION: the - // reader keeps the answers for the seqs still outstanding (a handful), drops them when - // they are read, and reports a LOSS when a read asks for one it no longer has. That is a - // transport change (StreamLink's ring becomes a retention map with a cap and a lost - // counter). Kept as-is because it is the safe direction - it degrades to the pre-window - // shape - and because the clock it needs is the one ReplyPostings() already provides. - constexpr Uint64 kCreateReplyResidency = Transport::kDefaultReplySlotCount - 1; - Bool CreateWindowBudgetSpent(ClientSession& session) { - if (g_createWindowCount == 0) return false; - return session.ReplyPostings() - g_createWindow[g_createWindowHead].ReplyStamp >= - kCreateReplyResidency; - } + // Retention replaced it, and retention is where the bound now lives: a link that declares + // RetainsReplies keeps an answer until its reader takes it (StreamLink's declared-answer + // store, whose capacity is tied to the window's own depth), and the window does not defer + // at all on a link that does not. That removes the only thing the budget was for. + // ReplyPostings() stays as a measurement. void Wire_ResourceCreate(const MG_Pipe::MGPResourceDesc* payload, MG_Pipe::MGPReplySlot* reply) { if (RunsAsTheServerRole()) { MG_Pipe::MGPipeMonolithScreen().ResourceCreate(payload, reply); return; } ClientSession& session = RequireSession("ResourceCreate"); - const Uint32 window = CreateWindowEffective(); + // THE WINDOW MAY ONLY DEFER WHERE THE LINK RETAINS ANSWERS (P12). On one that does + // not - the shared pool, whose slot is seq % slotCount and is reused - the answer the + // drain comes back for may already be gone, and there is NO recovery: retrying wedges + // the row for the rest of the process, and re-latching the object by sending another + // create wipes the server's record along with its PendingUploads (PipeApply.cpp:2033, + // PipeApply.h:332) while the client cleared its dirty flags at emission + // (PipeApply.h:298-304), so nobody re-sends those texels. So it takes the blocking + // path instead, which is the pre-window shape exactly. + const Uint32 window = session.LinkRetainsReplies() ? CreateWindowEffective() : 1u; // THE WINDOW OFF RESTORES THE OLD SHAPE EXACTLY, and is the negative control for it. // THE WINDOW OFF, THE SUSPECT LIST FULL, OR AN OBJECT THAT HAS ALREADY BEEN REFUSED: // all four take the blocking branch, which is the pre-window shape - and on the device @@ -615,6 +604,13 @@ namespace MobileGL::MG_Remote::Client { // collected. See the section comment for the measurement and the arithmetic. if (window <= 1 || g_createSuspects.size() >= kCreateSuspectMax || CreateIsSuspect(payload->Resource)) { + // THE WINDOW RETIRES HERE, AND IT MAY BE HOLDING ANSWERS (P12). This branch is + // taken when the window is off, when the suspect list is full, or for a refused + // object - and in the last two cases entries already emitted are still in flight. + // Without this drain they arrive, are retained, and are never read, WHILE their + // objects stay latched on a provisional accept: the silent divergence the + // suspect list exists to prevent. One blocking pass settles them. + if (g_createWindowCount != 0) DrainCreateWindow(session, /*blockForOldest=*/true); EmitCreateBlocking(session, payload, reply); return; } @@ -622,11 +618,7 @@ namespace MobileGL::MG_Remote::Client { // The poll is not politeness: it is what keeps a burst shorter than the window from // leaving its answers unread until the next burst. DrainCreateWindow(session, /*blockForOldest=*/false); - // A SPENT BUDGET IS TREATED EXACTLY LIKE A FULL WINDOW: take the oldest answer back - // NOW, while it still exists, instead of deferring another one past its residency. - const Bool budgetSpent = CreateWindowBudgetSpent(session); - if (g_createWindowCount >= window || budgetSpent) { - if (budgetSpent) ++g_createWindowBudgetStops; + if (g_createWindowCount >= window) { DrainCreateWindow(session, /*blockForOldest=*/true); // IF IT IS STILL FULL the oldest answer is not readable yet, and the window is a // fixed array: pushing would silently OVERWRITE a confirmation. This record takes @@ -657,8 +649,7 @@ namespace MobileGL::MG_Remote::Client { return; } g_createWindow[(g_createWindowHead + g_createWindowCount) % kCreateWindowMax] = - PendingCreate{seq, CreateRecordKind(*payload), payload->Resource, - session.ReplyPostings()}; + PendingCreate{seq, CreateRecordKind(*payload), payload->Resource}; ++g_createWindowCount; reply->Id = seq; MG_Pipe::MGPipePostReply(*reply, Wire::ReplySink::kStatusOk, 1u); @@ -1277,11 +1268,10 @@ namespace MobileGL::MG_Remote::Client { // the ones still outstanding. A create the window accepted and then LOST would break that // sum, which is the failure a provisional accept makes possible and a count cannot show. Uint64 ClientCreateWindowTaken() { return g_createWindowTaken; } - // How many times the residency budget, and not a full window, forced the blocking read. A - // non-zero value is the budget DOING its job (an answer was about to outlive its slot); a - // value near the record count means the window is deferring almost nothing, which is the - // honest way to say "the budget is too tight for this workload". - Uint64 ClientCreateWindowBudgetStops() { return g_createWindowBudgetStops; } + // P12: RetainedOutstanding / SkippedUnwanted / UnansweredWanted live on the LINK, where the + // store is (StreamLink::RetainedOutstanding and friends). They are what says whether the + // declared-answer store is bounded by the CLIENT (SkippedUnwanted grows, outstanding does + // not) rather than by the server's answer rate - which is the property the capacity rests on. Uint32 ClientCreateWindowEffective() { return CreateWindowEffective(); } Uint32 ClientCreateWindowArraySize() { return kCreateWindowMax; } // Objects whose create came back DECLINED: out of the window, next create blocking, and back diff --git a/MobileGL/MG_Remote/Client/WireTables.h b/MobileGL/MG_Remote/Client/WireTables.h index 8ec6de12..1cc75f59 100644 --- a/MobileGL/MG_Remote/Client/WireTables.h +++ b/MobileGL/MG_Remote/Client/WireTables.h @@ -91,11 +91,7 @@ namespace MobileGL::MG_Remote::Client { // drain that never collects leaves Pending at the ceiling and every other reading unchanged, // which is exactly what the device measured before the stream link learned to answer by seq. Uint64 ClientCreateWindowTaken(); - // How many times the RESIDENCY BUDGET - not a full window - forced the blocking take: the - // oldest answer was about to outlive its reply slot. Non-zero is the budget doing its job; - // a value near the record count says the budget is too tight for this workload and the - // window is deferring almost nothing, which is the honest way to read that. - Uint64 ClientCreateWindowBudgetStops(); + Uint32 ClientCreateWindowEffective(); Uint32 ClientCreateWindowArraySize(); Uint32 ClientCreateWindowSuspects(); diff --git a/MobileGL/MG_Remote/Transport/ILink.h b/MobileGL/MG_Remote/Transport/ILink.h index 1a9639e1..84c432b0 100644 --- a/MobileGL/MG_Remote/Transport/ILink.h +++ b/MobileGL/MG_Remote/Transport/ILink.h @@ -199,6 +199,16 @@ namespace MobileGL::MG_Remote::Transport { bool PublishIsDelivery = false; // false => Flush() is mandatory before any wait std::uint64_t MaxRecordBytes = 0; std::uint64_t MaxReplyBytes = 0; + // P12: DOES THIS LINK KEEP AN ANSWER UNTIL ITS READER TAKES IT? A stream does - the + // answers are the client's own bytes - and a shared pool does NOT: a slot is + // `seq % slotCount` and the next answer into it wins, so "the watermark passed this seq" + // says nothing about whether the answer is still there. + // + // IT IS A CAPABILITY RATHER THAN A HOPE because a row that DEFERS its answer (the create + // window) may only run where it is true: measured on the device, a deferral across the + // ~1,000 records between two creates lost its answer to a bounded buffer every time. + // A caller that sees false must take the blocking path instead. + bool RetainsReplies = false; }; // Non-owning synchronization addresses, acquired once at attachment. They @@ -285,6 +295,22 @@ namespace MobileGL::MG_Remote::Transport { virtual MobileGLResult ReadReply(std::uint64_t seq, std::int32_t* outStatus, const void** outPayload, std::uint64_t* outSize) = 0; + // P12: DECLARE THAT THIS SEQ'S ANSWER WILL BE READ, so that a link which can retain it + // does. Called by the producer side of the wire, in seq order, BEFORE the record is + // published - the answer cannot arrive before that, and the reader stores under the same + // mutex the declaration takes, so the hand-off needs no second synchronisation. + // + // WHY THE DECLARATION AND NOT "RETAIN EVERYTHING": a server that ignores the no-reply bit + // answers ~55,903 records in one frame where this client reads ~4,501, so retain-on-arrival + // grows ~11.3 entries per create and any cap is reached in C/11.3 creates - after which the + // eviction wave lands on precisely the oldest WANTED answer, which is the wedge this exists + // to avoid. Keyed on the declaration, the retained set is bounded by the CLIENT's own + // deferral depth instead of by the server's behaviour. + // + // A link with RetainsReplies false ignores it (the default), and its callers must not + // defer: see the capability's own note. + virtual void DeclareReplyRead(std::uint64_t seq) { (void)seq; } + // ---- the reverse channel -------------------------------------------- virtual LinkArena* EventArena() = 0; diff --git a/MobileGL/MG_Remote/Transport/ShmLink.cpp b/MobileGL/MG_Remote/Transport/ShmLink.cpp index 406891d0..3cf6bfbd 100644 --- a/MobileGL/MG_Remote/Transport/ShmLink.cpp +++ b/MobileGL/MG_Remote/Transport/ShmLink.cpp @@ -44,7 +44,9 @@ namespace MobileGL::MG_Remote::Transport { } LinkCapabilities ShmLink::Capabilities() const { const ReplySlotPool replies(m_memory.ReplyBase(), m_memory.ReplyBytes(), m_memory.ReplySlotCount()); - return {true, true, true, m_memory.CmdRingCapacity() / 2, replies.MaxReplyBytes()}; + // RetainsReplies stays false: a slot is seq % slotCount and the next answer into it wins. + return {true, true, true, m_memory.CmdRingCapacity() / 2, replies.MaxReplyBytes(), + /*RetainsReplies=*/false}; } LinkProgress* ShmLink::Progress() { return m_memory.Valid() ? &m_memory.CmdControl()->Progress : nullptr; diff --git a/MobileGL/MG_Remote/Transport/StreamLink.cpp b/MobileGL/MG_Remote/Transport/StreamLink.cpp index 59f9b461..c9b8fbd4 100644 --- a/MobileGL/MG_Remote/Transport/StreamLink.cpp +++ b/MobileGL/MG_Remote/Transport/StreamLink.cpp @@ -107,18 +107,52 @@ namespace MobileGL::MG_Remote::Transport { // move (29.0 s against the 30.1 s of the same binary with the window off). The deferral // was pure overhead: four answers held unread, plus one failed drain attempt per record. // - // BOUNDED AT kDefaultReplySlotCount, which is the pool ShmLink stands in for: an answer a - // reader has not taken by the time eight newer ones have arrived is one the shm arm has - // already overwritten too, so the two transports now lose the same answers and no more. - // The byte vectors are sized per reply, so an idle link pays eight empty ones. - static constexpr std::size_t kReplyRing = 8; + // ---- AND THE RING WAS NOT ENOUGH, SO THE STORE IS KEYED ON A DECLARATION (P12) ------- + // + // THE MEASUREMENT THAT SETTLED IT. With the ring at 8 the drain read back NOTHING: + // `taken=63 pushed=65 readfail=8,872` on a load frame, and the miss probe showed why - + // `want=310` on every miss while the ring had moved on to [1175..1504]. Between two + // creates this client emits on the order of a thousand records and looks at the window + // not once, so a deferral has to survive an unbounded amount of other traffic, and WHAT + // DISPLACES IT IS NOT WHAT THE CLIENT EMITS BUT WHAT THE SERVER ANSWERS - it is behind, + // and its late answers to the EARLIER creates fill a bounded buffer first. + // + // Ring 8 -> 1024 as a one-line experiment, and every deferred answer came back: + // `taken=4,499/4,500 readfail=0`, wall 27.9 s -> 23.0 s, and with the residency budget + // out of the way 28.66 s -> 16.80 s at window 2 and 11.45 s at window 4. So the property + // needed is "the answer is still there when the reader comes back", and 1024 is a + // STAND-IN for it, not the design: 1024 retained answers is ~2 GiB worst case, and "deep + // enough" is a probability rather than a bound. + // + // THE BOUND COMES FROM THE CLIENT. A seq is retained only if the wire DECLARED it will be + // read (ILink::DeclareReplyRead), so the outstanding set is the window's depth plus the + // one blocking row in flight plus whatever a retired window strands - it does not depend + // on how many answers the server chooses to send. Retaining on ARRIVAL instead would grow + // ~11.3 entries per create against a server that ignores the no-reply bit (55,903 answers + // a frame where this client reads 4,501), and any cap would then be reached and would + // evict precisely the oldest WANTED answer - the same wedge, a few hundred creates later. + // + // OVERFLOW IS FATAL AND NEVER AN EVICTION. An evicted wanted answer is unrecoverable: + // sending another create to re-latch the object wipes the server's record for it + // (PipeApply.cpp:2033) along with its PendingUploads (PipeApply.h:332), and the client + // cleared its dirty flags at EMISSION (PipeApply.h:298-304), so nobody re-sends those + // texels. The capacity is sized so that cannot be reached, and reaching it is a bug that + // says its own name. + static constexpr std::size_t kRetainedCap = 24; struct StoredReply { std::uint64_t seq = 0; std::int32_t status = 0; std::vector bytes; }; - std::array replies{}; - std::size_t replyNext = 0; + // MONOTONE ON BOTH SIDES, so a cursor beats a hash map: the wire declares in emission + // order, the server answers in apply order. It also makes a declared-but-never-answered + // seq visible for free - the cursor walks past it on the way to a later answer - which is + // counted rather than ignored, because the read that follows will fail and should fail + // with a number beside it. + std::vector wanted; + std::size_t wantCursor = 0; + std::vector retained; + std::uint64_t skippedUnwanted = 0, unansweredWanted = 0, retainedTotal = 0; std::uint64_t lastProgressSeq = 0; std::chrono::steady_clock::time_point lastProgress = std::chrono::steady_clock::now(); LinkArena records{}, events{}; @@ -334,15 +368,34 @@ namespace MobileGL::MG_Remote::Transport { std::lock_guard lock(replyMutex); if (a <= replySeq) Corrupt("reply sequence moved backwards"); replySeq = a; - // RING, NOT REPLACE. replySeq stays the newest and stays monotone - - // the wait predicate below is written in terms of it - while the - // answers themselves are kept for the readers that come back for an - // older one (the window's drain, StreamLink.cpp's own note). - StoredReply& slot = replies[replyNext]; - replyNext = (replyNext + 1) % kReplyRing; - slot.seq = a; - slot.status = static_cast(static_cast(b)); - slot.bytes.assign(payload, payload + size); + // DECLARED? The cursor walks past any DECLARED seq this answer has + // overtaken: the server answers in apply order, so an answer with a + // higher seq means those will never come. That is a broken invariant + // rather than a race, and it is counted - the read that follows fails + // and should fail with this number beside it. + while (wantCursor < wanted.size() && wanted[wantCursor] < a) { + ++wantCursor; + ++unansweredWanted; + } + if (wantCursor < wanted.size() && wanted[wantCursor] == a) { + ++wantCursor; + // OVERFLOW IS NAMED, NOT ABSORBED. See the capacity note above: + // an eviction here would throw away an answer somebody is waiting + // for, and there is no path that recovers one. + if (retained.size() >= kRetainedCap) { + Corrupt("the declared-answer store overflowed; a declared answer" + " would have to be evicted and none is recoverable"); + } + StoredReply& slot = retained.emplace_back(); + slot.seq = a; + slot.status = static_cast(static_cast(b)); + slot.bytes.assign(payload, payload + size); + ++retainedTotal; + } else { + // NOBODY DECLARED IT: drop it. This is what keeps the store bounded + // against a server that answers records this client never reads. + ++skippedUnwanted; + } } replyReady.notify_all(); producer.Notify(); @@ -464,7 +517,8 @@ namespace MobileGL::MG_Remote::Transport { } LinkCapabilities StreamLink::Capabilities() const { if (!m_impl->memory) return {}; - return {false, false, false, m_impl->memory->CmdRingCapacity() / 2, m_impl->MaxReply()}; + return {false, false, false, m_impl->memory->CmdRingCapacity() / 2, m_impl->MaxReply(), + /*RetainsReplies=*/true}; } LinkProgress* StreamLink::Progress() { return m_impl->memory ? &m_impl->C().Progress : nullptr; @@ -589,17 +643,22 @@ namespace MobileGL::MG_Remote::Transport { return x.closed.load(std::memory_order_acquire) || x.replySeq >= seq || x.C().Progress.appliedSeq.load(std::memory_order_acquire) >= seq; }); - // BY SEQ, WHICH IS WHAT THE PARAMETER HAS ALWAYS SAID (P12). The equality this replaces + // BY SEQ, WHICH IS WHAT THE PARAMETER HAS ALWAYS SAID (P12). The equality this replaced // was true only while every caller read the newest answer; it is what made a deferred // answer unreadable and the create window inert on this transport. - for (const auto& slot : x.replies) { - if (slot.seq != seq) continue; - // Copied into the shared scratch so the pointer outlives the lock, exactly as the - // single-slot version did: the caller memcpys under its own frame. - x.readReply = slot.bytes; - *status = slot.status; + for (auto it = x.retained.begin(); it != x.retained.end(); ++it) { + if (it->seq != seq) continue; + // TAKE-AND-DELETE IS ONE STEP UNDER THE LOCK. The bytes are SWAPPED into the shared + // scratch before the entry goes, so whoever is handed the answer is whoever consumed + // it - no copy of a large answer, and no window in which a second reader could see + // the same seq. (The pointer outlives the lock, as it always has here; this link has + // ONE reply reader, and the store's erase is what keeps that an invariant rather + // than a hope.) + x.readReply.swap(it->bytes); + *status = it->status; *size = x.readReply.size(); *p = x.readReply.data(); + x.retained.erase(it); return MOBILEGL_OK; } return x.closed.load() ? MOBILEGL_ERR_TRANSPORT_CLOSED : MOBILEGL_ERR_PROTOCOL_MISMATCH; @@ -624,12 +683,48 @@ namespace MobileGL::MG_Remote::Transport { TransportRoleTag StreamLink::Role() const { return m_impl->role; } + void StreamLink::DeclareReplyRead(std::uint64_t seq) { + auto& x = *m_impl; + if (!x.Client() || !x.memory) return; + std::lock_guard lock(x.replyMutex); + // MONOTONE, IN EMISSION ORDER (the wire declares before it publishes, and seqs only ever + // grow). An out-of-order or repeated declaration is dropped rather than stored: the first + // would break the cursor, and the second is a retry that must not grow the list. + if (!x.wanted.empty() && seq <= x.wanted.back()) return; + x.wanted.push_back(seq); + // AND THE CONSUMED PREFIX IS RELEASED, or the list would grow by one entry per declared + // record for the life of the session (4,536 a frame). The cursor's entries can never be + // needed again: a seq is answered once. + if (x.wantCursor >= 64) { + x.wanted.erase(x.wanted.begin(), + x.wanted.begin() + static_cast(x.wantCursor)); + x.wantCursor = 0; + } + } + + std::uint64_t StreamLink::RetainedOutstanding() const { + auto& x = *m_impl; + std::lock_guard lock(x.replyMutex); + return static_cast(x.retained.size()); + } + std::uint64_t StreamLink::RetainedTotal() const { return m_impl->retainedTotal; } + std::uint64_t StreamLink::SkippedUnwanted() const { return m_impl->skippedUnwanted; } + std::uint64_t StreamLink::UnansweredWanted() const { return m_impl->unansweredWanted; } + void StreamLink::Detach() { auto& x = *m_impl; if (!x.memory) return; x.stopping.store(true, std::memory_order_release); x.Die(false); if (x.reader.joinable()) x.reader.join(); + // P12: the declared-answer store does not outlive the link. Without this a detached + // session leaves up to a frame's worth of answers (and their seqs) resident. + { + std::lock_guard lock(x.replyMutex); + x.wanted.clear(); + x.wantCursor = 0; + x.retained.clear(); + } #if !defined(_WIN32) if (x.fd >= 0) ::close(x.fd); #endif diff --git a/MobileGL/MG_Remote/Transport/StreamLink.h b/MobileGL/MG_Remote/Transport/StreamLink.h index 143423f3..a73d460f 100644 --- a/MobileGL/MG_Remote/Transport/StreamLink.h +++ b/MobileGL/MG_Remote/Transport/StreamLink.h @@ -36,6 +36,17 @@ namespace MobileGL::MG_Remote::Transport { MobileGLResult ResolveSpan(LinkSegment, LinkSpan, const void**) override; MobileGLResult PostReply(std::uint64_t, std::int32_t, const void*, std::uint64_t) override; MobileGLResult ReadReply(std::uint64_t, std::int32_t*, const void**, std::uint64_t*) override; + void DeclareReplyRead(std::uint64_t) override; + + // P12 diagnostics for the declared-answer store. RetainedOutstanding is what the capacity + // is about (it must stay near the window's depth, not grow with the server's answers); + // RetainedTotal counts the answers kept for their readers; SkippedUnwanted counts the ones + // nobody declared, which is the number that proves the store is bounded by the CLIENT and + // not by the server; UnansweredWanted counts declared seqs the server never answered. + std::uint64_t RetainedOutstanding() const; + std::uint64_t RetainedTotal() const; + std::uint64_t SkippedUnwanted() const; + std::uint64_t UnansweredWanted() const; LinkArena* EventArena() override; LinkCursor* EventCursor() override; std::uint64_t EventPublishedHead() const override; diff --git a/MobileGL/MG_Test/Wire/RemoteClientControls.inc b/MobileGL/MG_Test/Wire/RemoteClientControls.inc index 499d948e..c58c94ee 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientControls.inc +++ b/MobileGL/MG_Test/Wire/RemoteClientControls.inc @@ -1205,21 +1205,22 @@ TEST(RemoteClientControls, ABufferRespecifyTakesNoReplyWhileATextureOneStillDoes // answers are collected in order. A late DECLINED un-latches its object, which puts every later row // for that object back on the blocking path - the repair that already existed, not a new one. // -// THE THREE WAYS THIS CAN GO WRONG, one reading each: +// ⚠️ THIS CASE RUNS ON THE IN-PROCESS ARM, WHICH IS ShmLink, AND SINCE P12 THAT MEANS THE WINDOW +// IS OFF HERE BY DESIGN - `CreateWindowEffective()` is gated on LinkRetainsReplies() precisely +// because a shared pool's slot is `seq % slotCount` and is reused, so a deferred answer may be +// gone with no way to recover it. What this case pins is therefore THAT GATE, and it is the +// assertion the device cannot make: set the knob as high as it goes and the row must STILL take the +// blocking path on a link that cannot retain. // -// 1. THE WINDOW DEFERS NOTHING (the row pays a wait per record and the change is decoration) - -// the ON half below; -// 2. THE WINDOW IS NOT WHAT DECIDES (the row defers while the operator asked it not to, which is -// the same defect as 1 with the sign flipped) - THE NEGATIVE CONTROL, run in the SAME CHILD as -// the ON half so that both readings share a session, a server, a set of objects and a pair of -// counters. MOBILEGL_IPC_CREATE_WINDOW=1 must restore the pre-window shape exactly; this is -// the assertion that goes red if the window ever stops being the knob; -// 3. THE WINDOW HOLDS MORE THAN IT IS SIZED FOR - and the ring is what makes this one quiet: an -// unguarded push does not grow a count past its ceiling, it REPLACES the oldest confirmation -// and leaves the count exactly where it was. So the invariant is read against the effective -// ceiling after every create, the ceiling is read against the array, the array is read against -// the reply pool it shares, and the environment is not allowed to raise any of them. -TEST(RemoteClientControls, ACreateWindowDefersTheAnswersAndTheWindowOffDoesNot) { +// THE WINDOW'S OWN BEHAVIOUR IS PINNED ELSEWHERE, ON THE ARM IT SHIPS ON: StreamLinkTest's +// declared-answer cases for the store, and the stream-arm window case for the row. That split is +// not tidiness - three rounds of this feature were green here while being inert on the device, +// because these gates and the device run on different transports. +// +// WHAT IS STILL READ HERE, because it is transport-independent: the environment cannot raise the +// window past the array, the array cannot exceed half the reply pool, and the invariant holds after +// every create. +TEST(RemoteClientControls, TheWindowRefusesToDeferOnALinkThatCannotRetainAnswers) { const auto child = RunInChild([] { ::setenv("MOBILEGL_PIPE_STATS", "1", 1); CapsPeer backend; @@ -1266,68 +1267,30 @@ TEST(RemoteClientControls, ACreateWindowDefersTheAnswersAndTheWindowOffDoesNot) } }; - // ---- THE WINDOW ON: twelve creates, strictly fewer waits ---- + // ---- THE KNOB IS AS HIGH AS IT GOES, AND THE ROW MUST STILL BLOCK ---- + MG_Config::Ipc.CreateWindow = 4; const Uint64 recordsBefore = Transport::LinkMetricsRecordsFor(createOp); const Uint64 waitsBefore = Transport::LinkMetricsReplyWaitsFor(createOp); burst(12); const Uint64 records = Transport::LinkMetricsRecordsFor(createOp) - recordsBefore; const Uint64 waits = Transport::LinkMetricsReplyWaitsFor(createOp) - waitsBefore; - std::printf("create window ON: window=%u records=%llu waits=%llu high_water=%u\n", + std::printf("non-retaining link: window=%u records=%llu waits=%llu taken=%llu pending=%u high_water=%u\n", static_cast(MG_Config::Ipc.CreateWindow), static_cast(records), static_cast(waits), + static_cast(ClientCreateWindowTaken()), + static_cast(ClientCreateWindowPending()), static_cast(highWater)); std::fflush(nullptr); if (records < 12) ::_exit(164); - // ---- AND THE ANSWERS CAME BACK, WHICH IS THE ASSERTION THAT CANNOT BE FAKED ---- - // A deferral that is never collected looks EXACTLY like a working one from every other - // reading in this case: Pending sits at the ceiling, the fallback keeps the frame moving, - // and the record count is right. It is also what the device measured for a whole session - // (pending pinned at 4, readfail 8,951) and what a no-reply bit keyed on `wantReply` would - // produce ON PURPOSE - the window's emits are wantReply=false, so a server that skipped - // them would leave this at zero while `records`, `waits` and `pending` all stayed green. - const Uint64 taken = ClientCreateWindowTaken(); - std::printf("create window ON: answers_taken=%llu outstanding=%u fallbacks=%llu\n", - static_cast(taken), - static_cast(ClientCreateWindowPending()), - static_cast(records > taken + ClientCreateWindowPending() - ? records - taken - ClientCreateWindowPending() - : 0u)); - std::fflush(nullptr); - if (taken == 0) ::_exit(174); - // NOTHING IS INVENTED AND NOTHING IS LOST: every answered create is either back in hand - // or still outstanding, and the remainder took the blocking fallback. - if (taken + ClientCreateWindowPending() > records) ::_exit(175); - // THE DEFERRAL ITSELF, and the honest size of the claim: the first `window` records pay - // nothing, so twelve records cannot pay twelve waits once anything was deferred. A ratio - // would be a promise about how fast the server answers, which is not this suite's to make. - if (waits >= records) ::_exit(165); + // NOTHING WAS DEFERRED AT ALL, and nothing was left outstanding: on a link that cannot + // retain, a window that deferred would be holding answers it may never get back. + if (ClientCreateWindowTaken() != 0) ::_exit(177); + if (ClientCreateWindowPending() != 0) ::_exit(178); + // AND EVERY CREATE PAID ITS ROUND TRIP, which is the pre-window shape exactly. This is the + // assertion that goes red if the capability gate is removed. + if (waits != records) ::_exit(179); - // ---- AND THE SAME TWELVE WITH THE WINDOW OFF, IN THIS SAME CHILD ---- - // The negative control shares everything with the ON half except the knob, which is what - // makes it a control rather than a second measurement. ONE WAIT PER RECORD is the - // pre-window shape: R-5 shipped with exactly that, and the whole change is the difference - // between the two lines this case prints. - MG_Config::Ipc.CreateWindow = 1; - const Uint64 offRecordsBefore = Transport::LinkMetricsRecordsFor(createOp); - const Uint64 offWaitsBefore = Transport::LinkMetricsReplyWaitsFor(createOp); - burst(12); - const Uint64 offRecords = Transport::LinkMetricsRecordsFor(createOp) - offRecordsBefore; - const Uint64 offWaits = Transport::LinkMetricsReplyWaitsFor(createOp) - offWaitsBefore; - std::printf("create window OFF: window=%u records=%llu waits=%llu\n", - static_cast(MG_Config::Ipc.CreateWindow), - static_cast(offRecords), - static_cast(offWaits)); - std::fflush(nullptr); - if (offRecords < 12) ::_exit(166); - // ONE WAIT PER CREATE, EXACTLY. Every create on this path is an EmitAndWait, so the two - // counters are equal whatever else the row does - which is the assertion that cannot be - // satisfied by a window that is still deferring sometimes. - if (offWaits != offRecords) ::_exit(167); - // AND THE WINDOW TOOK NOTHING WHILE IT WAS OFF, which is the control the ON half's - // `taken` needs: without it, a `taken` that grew for some unrelated reason would read as - // a working deferral. - if (ClientCreateWindowTaken() != taken) ::_exit(176); ClientSessionInstance().Stop(); }); ExpectChildSuccess(child); @@ -1337,14 +1300,18 @@ TEST(RemoteClientControls, ACreateWindowDefersTheAnswersAndTheWindowOffDoesNot) // P12: A REFUSED CREATE LEAVES THE WINDOW - read ON THE LATCH AND ON THE SUSPECT LIST // ===================================================================================== // -// The window's answer is provisional, so the one thing it must never do is keep feeding a refused -// object fire-and-forget creates: the client would hold a confirmation the server has refused, for -// ever, and every row that reads the latch would believe it. The repair is deliberately the path -// that already existed - DECLINED un-latches the object, the object leaves the window, its next -// create BLOCKS and the refusal reaches the caller - and this case reads all three steps rather -// than the symptom. It then reads the HEAL, which is the other half of the same rule: a blocking -// create that lands puts the object back, so a transient refusal cannot retire an object from the -// window for the rest of the process (or, at kCreateSuspectMax of them, retire the window). +// The answer is what PUBLISHES an object, so the one thing a refusal must never do is leave the +// client believing it landed: every row that reads the latch would then act on a record the server +// does not hold. This case reads the whole chain - the refusal leaves the object UNLATCHED, its +// next storage definition BLOCKS and is refused again, and giving the family its consumer back +// HEALS it (the create is accepted and the latch comes back). +// +// ⚠️ RUNS ON THE IN-PROCESS ARM (ShmLink), WHERE THE WINDOW IS OFF, so what repairs the object +// here is the LATCH ALONE - the suspect table belongs to the window's provisional accept and the +// drain is the only thing that fills it, which this case now asserts by finding it EMPTY. The +// window's own refusal path is pinned on the arm it ships on (StreamLinkTest's declared-answer +// cases and the stream-arm window case); see the note on the gate above for why that split is not +// tidiness. // // THE REFUSAL IS MADE REAL RATHER THAN SIMULATED, and it is the exact case the audit found the two // ends can disagree about: the applier asks for bit 10 while the client's mirror still says the @@ -1384,7 +1351,7 @@ TEST(RemoteClientControls, ARefusedCreateUnlatchesItsObjectAndItsNextCreateBlock MG_Impl::GLImpl::GenTextures(1, &filler); defineStorage(filler, 8); } - // ---- (a) THE LATCH IS CLEAR AND THE OBJECT IS OUT OF THE WINDOW ---- + // ---- (a) THE LATCH IS CLEAR, AND THE WINDOW NEVER RAN HERE AT ALL ---- const auto refusedObject = MG_State::pGLContext->GetTextureObject(refused); if (refusedObject == nullptr) ::_exit(174); const MG_Pipe::MGPipeHandle refusedHandle = @@ -1393,9 +1360,14 @@ TEST(RemoteClientControls, ARefusedCreateUnlatchesItsObjectAndItsNextCreateBlock // THE DIRECT READING, which is what the converted rows read: not "the create was refused" // but "this object is not published", which is the fact every fire-and-forget row acts on. if (MG_Pipe::MGPipeHandleIsPublished(MG_Pipe::MGPipeKind::Texture, refusedHandle)) ::_exit(176); - if (ClientCreateWindowRefusals() == 0) ::_exit(177); + // AND THE WINDOW'S OWN BOOKS ARE EMPTY, because this arm cannot retain answers and the + // window therefore never deferred - `refusals` and `suspects` are both filled by the + // drain and by nothing else. A non-zero reading here would mean the capability gate had + // been removed and the row was deferring on a link that cannot hold an answer. + if (ClientCreateWindowRefusals() != 0) ::_exit(177); const Uint32 suspectsBefore = ClientCreateWindowSuspects(); - if (suspectsBefore == 0) ::_exit(178); + if (suspectsBefore != 0) ::_exit(178); + if (ClientCreateWindowPending() != 0) ::_exit(183); // ---- (b) AND ITS NEXT CREATE BLOCKS: ONE WAIT PER CREATE, WITH THE WINDOW WIDE OPEN ---- // The window has room (the ceiling is 4 and this case has just drained it), so a deferring // row would pay ZERO waits here. Every create this restorage emits pays one instead, which @@ -1413,18 +1385,17 @@ TEST(RemoteClientControls, ARefusedCreateUnlatchesItsObjectAndItsNextCreateBlock if (suspectRecords == 0) ::_exit(179); if (suspectWaits < suspectRecords) ::_exit(180); // ---- (c) THE HEAL: GIVE THE FAMILY ITS CONSUMER BACK AND RE-DEFINE THE SAME OBJECT ---- - // The create still blocks (the object is still suspect), now it LANDS, and landing is what - // takes it out of the suspect list - exactly one entry, because no other object is touched. + // The create still blocks (the latch is still clear), now it LANDS, and landing is what + // re-publishes the object for every row that reads the latch. Srv::ServerSessionInstance().SetConsumedSubsystems(kMGPipeSubsystemsMigratedAtP4a); defineStorage(refused, 32); - std::printf("refused create healed: published=%d suspects %u -> %u\n", + std::printf("refused create healed: published=%d suspects=%u\n", static_cast(MG_Pipe::MGPipeHandleIsPublished(MG_Pipe::MGPipeKind::Texture, refusedHandle)), - static_cast(suspectsBefore), static_cast(ClientCreateWindowSuspects())); std::fflush(nullptr); if (!MG_Pipe::MGPipeHandleIsPublished(MG_Pipe::MGPipeKind::Texture, refusedHandle)) ::_exit(181); - if (ClientCreateWindowSuspects() != suspectsBefore - 1) ::_exit(182); + if (ClientCreateWindowSuspects() != suspectsBefore) ::_exit(182); ClientSessionInstance().Stop(); }); ExpectChildSuccess(child); diff --git a/MobileGL/MG_Test/Wire/StreamLinkTest.cpp b/MobileGL/MG_Test/Wire/StreamLinkTest.cpp index df2f8f3e..c4f17a73 100644 --- a/MobileGL/MG_Test/Wire/StreamLinkTest.cpp +++ b/MobileGL/MG_Test/Wire/StreamLinkTest.cpp @@ -116,6 +116,11 @@ namespace { Queue(2, 2); ASSERT_EQ(client.Flush(), MOBILEGL_OK); ASSERT_EQ(apply.WaitForWork(2000), SessionWait::Reached); + // P12: AN ANSWER IS KEPT ONLY IF SOMEBODY DECLARED IT WILL BE READ. Seq 1 is deliberately + // NOT declared - this case is about an unwanted reply not disturbing a later one - and + // seq 2 is, because the case reads it. Both records cross either way: the declaration + // changes whether the LINK keeps the answer, never whether the server sends it. + client.DeclareReplyRead(2); ASSERT_TRUE(apply.ApplyOne([&](const RingRecordView&) { const std::uint64_t value = 11; EXPECT_EQ(server.PostReply(1, 0, &value, sizeof value), MOBILEGL_OK); @@ -152,6 +157,7 @@ namespace { Queue(3, 3); ASSERT_EQ(client.Flush(), MOBILEGL_OK); ASSERT_EQ(apply.WaitForWork(2000), SessionWait::Reached); + for (std::uint64_t seq = 1; seq <= 3; ++seq) client.DeclareReplyRead(seq); for (std::uint64_t seq = 1; seq <= 3; ++seq) { ASSERT_TRUE(apply.ApplyOne([&](const RingRecordView&) { const std::uint64_t value = seq * 11; @@ -175,6 +181,55 @@ namespace { } } + // ===================================================================================== + // P12: THE DECLARED-ANSWER STORE - WHAT A BOUNDED BUFFER OF "THE LAST N" COULD NOT DO + // ===================================================================================== + // + // The create window defers an answer across an unbounded amount of other traffic (measured on + // the device: ~1,000 records between two creates) and a buffer of the last N answers has always + // moved on by the time the reader comes back - `want=310` on every miss while the ring spanned + // [1175..1504]. Ring 8 -> 1024 as an experiment made every deferred answer come back, and these + // two cases are what replaces that stand-in: an answer is kept because somebody DECLARED it + // will be read, so the retained set is bounded by the client's own deferral depth. + TEST_F(StreamPair, ADeclaredAnswerSurvivesFarMoreInterveningAnswersThanAnyFixedBuffer) { + constexpr std::uint64_t kIntervening = 64; + client.DeclareReplyRead(1); + const std::uint64_t wanted = 0x5EED; + ASSERT_EQ(server.PostReply(1, 0, &wanted, sizeof wanted), MOBILEGL_OK); + // SIXTY-FOUR answers nobody declared. Under "keep the last N" - any N this link has ever + // had - seq 1 is gone by the end of this loop; under a declaration it is untouched, which + // is the whole difference between a bound and a probability. + for (std::uint64_t seq = 2; seq <= kIntervening + 1; ++seq) { + const std::uint64_t junk = seq; + ASSERT_EQ(server.PostReply(seq, 0, &junk, sizeof junk), MOBILEGL_OK); + } + EXPECT_TRUE(Eventually([&] { return client.SkippedUnwanted() >= kIntervening; })) + << "the undeclared answers were not dropped: " << client.SkippedUnwanted(); + std::int32_t status = -1; + const void* p = nullptr; + std::uint64_t size = 0; + ASSERT_EQ(client.ReadReply(1, &status, &p, &size), MOBILEGL_OK) + << "the declared answer did not survive " << kIntervening << " intervening ones"; + ASSERT_EQ(size, sizeof(std::uint64_t)); + std::uint64_t value = 0; + std::memcpy(&value, p, sizeof value); + EXPECT_EQ(value, wanted); + EXPECT_EQ(status, 0); + // AND TAKING IT RELEASED IT: the store holds only what is still outstanding, which is why + // its capacity can be tied to the window's depth rather than to the server's answer rate. + EXPECT_EQ(client.RetainedOutstanding(), 0u); + EXPECT_EQ(client.RetainedTotal(), 1u); + } + + TEST_F(StreamPair, AnAnswerNobodyDeclaredIsNotKeptAtAll) { + const std::uint64_t value = 7; + ASSERT_EQ(server.PostReply(1, 0, &value, sizeof value), MOBILEGL_OK); + EXPECT_TRUE(Eventually([&] { return client.SkippedUnwanted() >= 1; })); + EXPECT_EQ(client.RetainedOutstanding(), 0u) + << "an undeclared answer was retained: the store would grow with the SERVER's answer" + " rate, which is what makes any capacity a matter of luck"; + } + TEST_F(StreamPair, EventDrainReturnsCreditAndClearsRemoteFullLatch) { EventRingProducer events(serverMemory.EventControl(), serverMemory.CmdControl(), serverMemory.EventRingBase(), 1024); diff --git a/docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md b/docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md index bf84171b..4f8e82ee 100644 --- a/docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md +++ b/docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md @@ -287,7 +287,41 @@ if (x.replySeq != seq) return ... MOBILEGL_ERR_PROTOCOL_MISMATCH; // 等的是 ——答案走 Reply 帧、`StreamLink::ReadReply` 读客户端自己的 store，池只用来给 `MaxReplyBytes()` 定尺寸。 所以「加深池子 + 抬 `ReplyBytes`」根本没有针对实测那条臂，别为它付 8× 常驻内存。 -## 12. 复算 +## 12. 落地：保留表上线，默认翻到 2，真机 28.66 s → 16.52 s + +按审计的形状实现（`ILink::DeclareReplyRead` + `LinkCapabilities::RetainsReplies`；`StreamLink` 的环 → +**按声明的保留表**：单调白名单 + 游标、取走即删、触顶具名 Fatal、Detach 清空；删掉 residency budget； +`CreateWindowEffective()` 在不保留答案的链路上强制为 1；窗口退休分支补一次 drain）。 + +**真机（两个二进制都重出，负载帧，`frame-sent 2=` 全程 4,536 不变）：** + +| 臂 | waits | `by_op 2` | 墙钟 | +|---|---|---|---| +| 窗口 1（R-5 的形状） | 4,543 | 4,536 | 28.66 s | +| 窗口 2（显式） | 2,507 | 2,500 | 17.27 s | +| **窗口 2（出厂默认，无覆盖）** | 2,570 | 2,563 | **16.52 s** | +| 窗口 4 | 1,651 | 1,644 | 11.02 s | +| **窗口 2 + 旧服务端（不认 no-reply 位）** | 2,670 | 2,663 | **16.38 s** | + +**版本偏斜那一轮是审计点名的必测项，它过了**：旧服务端照回 ~55,903 条/帧，客户端的白名单把这些 +**没人声明**的答案在到达时丢弃（`SkippedUnwanted` 增长而不是保留表增长），延后的答案照旧读得回来， +整轮**零 Fatal**。这正是「按声明保留」相对「全保留 + 淘汰」的那个差别。 + +**门**（172 + 15 全绿）： +- `StreamLinkTest.ADeclaredAnswerSurvivesFarMoreInterveningAnswersThanAnyFixedBuffer` —— **已证明能红**： + 把存储退回「按到达保留、只留 8 条」的环语义，它立刻以「the declared answer did not survive 64 + intervening ones」失败。这就是把环=1024 那个替身换成有界机制的那条界线。 +- `StreamLinkTest.AnAnswerNobodyDeclaredIsNotKeptAtAll` —— 没声明的答案不进保留表（红法：全保留）。 +- `RemoteClientControls.TheWindowRefusesToDeferOnALinkThatCannotRetainAnswers` —— 在不能保留的链路 + （in-process/ShmLink）上，把旋钮开到 4 也必须每条 create 都走阻塞、`taken == 0`（红法：去掉能力位）。 +- `ARefusedCreate...` 改成读这条臂上真正的修复链：拒绝 → 闩保持清零 → 下一次定义阻塞且再被拒 → 恢复消费者后闩回来； + 并断言窗口的账本（`refusals`/`suspects`/`pending`）在这条臂上**全为空**。 + +**还没做的一件**（审计 E10(d)，下一轮）：窗口级门目前只有真机这一层，**没有跑在 stream 臂上的单元门**。 +三轮「门全绿而功能在设备上是死的」都源于同一件事——门跑在 in-process/ShmLink 上。传输层的保留门已经补上， +窗口那一层建议建在 `StreamClientPublicationTest` 的真 stream 会话上（`AttachStreamLink` + 可控 peer）。 + +## 13. 复算 ~~~sh # 两臂的帧读数（handoff-2 §1 的读法）