From 102f2ca7d5a284929782bca43f1de84f40c746dc Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Sat, 26 Sep 2026 11:26:54 +0800 Subject: [PATCH] [Perf] (MG_Remote/Client): The create window's shipped depth is 6, and its ceiling 8. --- MobileGL/Config.h | 23 ++++++++-- MobileGL/ConfigLoader.cpp | 21 ++++++--- MobileGL/MG_Remote/Client/WireTables.cpp | 46 ++++++++++++++----- MobileGL/MG_Remote/Transport/StreamLink.cpp | 23 +++++++++- MobileGL/MG_Remote/Transport/StreamLink.h | 8 ++++ .../MG_Test/Wire/RemoteClientControls.inc | 31 +++++++------ MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 3 ++ .../Wire/StreamClientPublicationTest.cpp | 8 +++- 8 files changed, 123 insertions(+), 40 deletions(-) diff --git a/MobileGL/Config.h b/MobileGL/Config.h index deb2c225..7b1b13d5 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -572,14 +572,29 @@ namespace MobileGL::MG_Config { // 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 + // 6 IS THE DEFAULT, AND IT IS WHERE THE CURVE FLATTENS - the sweep that says so is the + // one the paragraph above asked for, and it moved the ceiling too (kCreateWindowMax is 8 + // now; the old 4 was derived from a reply pool the window does not run on). Three + // interleaved runs per arm on one binary, load frame, create 4,536 sent in every arm: + // + // window 4 1,503 blocking takes 13.23 s median (spread 11.37-16.29) + // window 5 1,277 11.31 s (10.61-12.12) + // window 6 1,155 10.79 s ( 9.69-10.81) + // window 8 859 10.96 s ( 8.92-11.03) + // + // 4->5 buys 1.92 s, 5->6 buys 0.53 s, 6->8 buys NOTHING (-0.18 s, inside a session noise + // of ~1.5-2 s) - so 6 is the smallest window on the flat part and the extra answers a + // deeper one holds buy no time. AND PAST THIS POINT THE FRAME IS NOT WAIT-BOUND ANY MORE: + // window 8 pays 296 fewer blocking takes than window 6 and the wall clock does not move. + // The store is not the constraint either - its peak is logged as it crosses (StreamLink's + // P12 line) and reads 6..11 against a cap of 24 at every arm from 5 up. What is left is + // the constant ~350-400 creates that block for a reason depth does not reach, and the + // 101 MB / 103k records the SERVER has to apply. 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; + Uint32 CreateWindow = 6; // 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 ee39af03..f5721381 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -412,12 +412,15 @@ 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 2, and it works because a link that declares RetainsReplies + // The create window. DEFAULT 6, 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); + // was inert before, the device numbers, and the sweep that put the default at 6 rather + // than 4 or 8 (the curve flattens at 6: 4->5 buys 1.92 s, 5->6 0.53 s, 6->8 nothing). + // Capped at 8: the retained set is bounded by the window's own depth, the store holds 24 + // declared answers, and the device sweep showed the wall clock still falling at the old + // cap of 4 (see WireTables.cpp's kCreateWindowMax). WireTables.cpp's static_assert and + // StreamLink's crossing log are the checks on this number. + ipc.CreateWindow = QueryEnvUint32("MOBILEGL_IPC_CREATE_WINDOW", 6, 1, 8); // 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 @@ -514,12 +517,16 @@ namespace MobileGL::MG_ConfigLoader { if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; // One line, on the arm where these numbers decide behaviour, because every one of // them is a number a bug report has to quote. + // CREATE-WINDOW IS IN THIS LINE BECAUSE A RUN HAS TO SAY WHICH WINDOW IT USED. It was + // missing while the value was a number nobody varied, and the moment it became one that + // changes the wall clock by seconds (the P12 sweep: 28.3 s at 1 down to 10.8 s at 6) a + // log that does not name it makes every run ambiguous about its own configuration. MGLOG_I("Config: IPC ring=%uMiB stage=%uMiB wire-deferred=%uMiB spin=%uus event-wait=%ums " - "persistent-block=%uKiB " + "persistent-block=%uKiB create-window=%u " "adopt-tier=%u verb-barrier=%u run-ahead=%u present-credit=%u control-timeout=%ums " "cold-start=%ums strict=%d audit=%d role-split-state=%d affinity='%s' surface=%s", ipc.RingMb, ipc.StageMb, ipc.WireDeferredMb, ipc.SpinUs, ipc.EventWaitMs, - ipc.PersistentBlockKb, ipc.AdoptTier, + ipc.PersistentBlockKb, ipc.CreateWindow, ipc.AdoptTier, ipc.VerbBarrier, ipc.RunAhead, ipc.PresentCredit, ipc.ControlTimeoutMs, ipc.ColdStartMs, static_cast(ipc.StrictErrors), static_cast(ipc.Audit), static_cast(ipc.RoleSplitState), ipc.ServerAffinity.c_str(), diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index 948f4626..ae4d268a 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -54,6 +54,9 @@ // P12: kDefaultReplySlotCount, because the create window's ceiling is derived from the pool it // shares rather than from a number written twice (see kCreateWindowMax below). #include +// P12: kRetainedAnswersMax, the store capacity the window's own ceiling is derived from - the +// constant itself rather than a second literal, so the two cannot drift. +#include // P12: LinkMetricsBeginReply/ReplyApplied, so the window's blocking take is visible in by_op 2. #include @@ -392,18 +395,37 @@ namespace MobileGL::MG_Remote::Client { // 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 // other reply traffic. - constexpr Uint32 kCreateWindowMax = 4; - // THE WINDOW MAY NOT OWN MORE THAN HALF THE POOL, and this is where that stops being a - // sentence in a comment. The pool is kDefaultReplySlotCount slots addressed seq % count, so - // an answer is readable only while its slot has not been reused; every other reply-owning - // row - fence, query, and the blocking rows themselves - needs a slot too. The change the - // handoff names as the one worth pinning ("someone raises kCreateWindowMax to 8 and steps on - // the pool") goes red HERE, by name, at compile time, instead of as a wrong picture on a - // device. Raised to 8 it would take the whole pool and leave nothing for the answer the - // blocking rows are waiting on. - static_assert(kCreateWindowMax <= Transport::kDefaultReplySlotCount / 2, - "the create window may hold at most half the reply pool: the other half " - "belongs to the rows that take their answers immediately"); + // (THE NARRATIVE ABOVE PREDATES RETENTION and ends at "the default is 1". It is kept as + // the record of what was tried; what made the window work is the declared-answer store, + // and the default is 2 - Config.h carries the device numbers.) + constexpr Uint32 kCreateWindowMax = 8; + // THE POOL ARGUMENT THAT USED TO PIN THIS AT 4 IS GONE, AND WHY IT IS GONE IS THE ONE + // THING TO RE-CHECK BEFORE RAISING IT AGAIN. It read: the pool is kDefaultReplySlotCount + // slots addressed seq % count, so an answer is readable only while its slot has not been + // reused, and a window owning half of them would starve the rows that take their answers + // immediately - the change the handoff named as the one worth pinning, "someone raises + // kCreateWindowMax to 8 and steps on the pool". That is true of a link whose answers live + // in a SLOT POOL, and the window does not run on one: CreateWindowEffective() is gated on + // ILink::RetainsReplies, which is false for ShmLink, so on the pool-addressed transport + // the window is 1 and holds nothing. On the link where it does run - StreamLink - an + // answer is kept in the declared-answer store until its reader takes it, so depth costs + // STORE ENTRIES, not pool slots. + // + // SO THE SAME RULE IS RE-DERIVED AGAINST THE RIGHT OBJECT: at most half of StreamLink's + // kRetainedCap = 24, the other half being the one blocking row in flight and whatever a + // retired window has stranded. The store's overflow is a named Fatal and never an + // eviction, so exceeding this costs a run rather than a wrong picture - and the store + // logs its own peak as it crosses, which is what a future raise must be argued from. + // + // MEASURED, device load frame, three interleaved runs per arm: the curve had NOT flattened + // at the old cap (1->2 saved 10.29 s, 2->3 3.74 s, 3->4 1.83 s), which is why this + // constant moved at all. + static_assert(kCreateWindowMax <= Transport::StreamLink::kRetainedAnswersMax / 2, + "the create window may hold at most half of StreamLink's declared-answer " + "store: the other half belongs to the one blocking row in flight and to " + "whatever a retired window has stranded. The store's overflow is a named " + "Fatal rather than an eviction, so this bound is about losing a run, not " + "about a wrong picture"); // AND THE ENVIRONMENT MAY NOT RAISE IT EITHER. The configured value is clamped to the // array's own size, so the fixed array below cannot be overrun by a config bound and this // constant having drifted apart. 1 (and anything below it) is the pre-window shape. diff --git a/MobileGL/MG_Remote/Transport/StreamLink.cpp b/MobileGL/MG_Remote/Transport/StreamLink.cpp index c9b8fbd4..579dbf96 100644 --- a/MobileGL/MG_Remote/Transport/StreamLink.cpp +++ b/MobileGL/MG_Remote/Transport/StreamLink.cpp @@ -5,6 +5,9 @@ #include "Doorbell.h" #include "ReplySlot.h" #include "Framing.h" +// P12: the declared-answer store's crossing log. Same include as LinkMetrics.cpp's, rather than +// , because this file is transport-layer and pulls nothing else from the tree. +#include #include #include #include @@ -138,7 +141,8 @@ namespace MobileGL::MG_Remote::Transport { // 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; + // ONE DEFINITION, IN THE HEADER, because the client's window is bounded by this number. + static constexpr std::size_t kRetainedCap = StreamLink::kRetainedAnswersMax; struct StoredReply { std::uint64_t seq = 0; std::int32_t status = 0; @@ -153,6 +157,10 @@ namespace MobileGL::MG_Remote::Transport { std::size_t wantCursor = 0; std::vector retained; std::uint64_t skippedUnwanted = 0, unansweredWanted = 0, retainedTotal = 0; + // P12: the most declared answers this store ever held at once. The cap is an overflow + // Fatal rather than an eviction, so the MAXIMUM is the number that says whether a given + // window depth has room - a sample at teardown answers a different question. + std::uint64_t retainedPeak = 0; std::uint64_t lastProgressSeq = 0; std::chrono::steady_clock::time_point lastProgress = std::chrono::steady_clock::now(); LinkArena records{}, events{}; @@ -391,6 +399,19 @@ namespace MobileGL::MG_Remote::Transport { slot.status = static_cast(static_cast(b)); slot.bytes.assign(payload, payload + size); ++retainedTotal; + // LOGGED AS IT CROSSES EVERY QUARTER OF THE CAP, not sampled: at most + // four lines per run, and they are the ones a "how deep may the window + // go" question is answered from without a rebuild (P12's + // MOBILEGL_IPC_CREATE_WINDOW sweep reads exactly these). + if (retained.size() > retainedPeak) { + retainedPeak = retained.size(); + if (retainedPeak % (kRetainedCap / 4) == 0) { + MGLOG_I("P12 store: peak=%llu of cap=%llu declared answers " + "outstanding; the window's depth is what is spent here", + static_cast(retainedPeak), + static_cast(kRetainedCap)); + } + } } else { // NOBODY DECLARED IT: drop it. This is what keeps the store bounded // against a server that answers records this client never reads. diff --git a/MobileGL/MG_Remote/Transport/StreamLink.h b/MobileGL/MG_Remote/Transport/StreamLink.h index a73d460f..0f8035f2 100644 --- a/MobileGL/MG_Remote/Transport/StreamLink.h +++ b/MobileGL/MG_Remote/Transport/StreamLink.h @@ -38,6 +38,14 @@ namespace MobileGL::MG_Remote::Transport { MobileGLResult ReadReply(std::uint64_t, std::int32_t*, const void**, std::uint64_t*) override; void DeclareReplyRead(std::uint64_t) override; + // THE DECLARED-ANSWER STORE'S CAPACITY, and it is a PUBLIC NUMBER rather than a private + // one because the client's create window is bounded by it: WireTables.cpp's + // kCreateWindowMax static_assert and RemoteClientControls' ceiling case both read it, and + // a bound that is restated in prose in three places is a bound that drifts. The argument + // for the VALUE (why 24, and why overflow is a Fatal and never an eviction) is in the + // .cpp beside the store it sizes. + static constexpr std::size_t kRetainedAnswersMax = 24; + // 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 diff --git a/MobileGL/MG_Test/Wire/RemoteClientControls.inc b/MobileGL/MG_Test/Wire/RemoteClientControls.inc index c58c94ee..c2d8f9e7 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientControls.inc +++ b/MobileGL/MG_Test/Wire/RemoteClientControls.inc @@ -1232,24 +1232,27 @@ TEST(RemoteClientControls, TheWindowRefusesToDeferOnALinkThatCannotRetainAnswers Uint8 pixels[4 * 4 * 4]; std::memset(pixels, 0x21, sizeof(pixels)); // ---- the clamp and the ceiling, before a single record goes out ---- - // THE WINDOW IS OPT-IN AND THE DEFAULT IS OFF (Config.h: the deferral was measured on the - // device and the drain cannot collect it), so both arms of this case SET the knob rather - // than inheriting it - the ON half below is the mechanism being exercised, not a claim - // about what ships. + // BOTH ARMS SET THE KNOB rather than inheriting it: this case is about what the code does + // with a configured window, and not a claim about which value ships (the default is 2 - + // Config.h carries the device numbers). // WHAT THE OPERATOR WROTE IS NOT WHAT THE CODE MAY USE. With the clamp gone a configured // window of 99 is compared against a count that the ring keeps below its own size, so the // fallback never fires and the push index runs off the array. MG_Config::Ipc.CreateWindow = 99; if (ClientCreateWindowEffective() != ClientCreateWindowArraySize()) ::_exit(170); - MG_Config::Ipc.CreateWindow = 4; - if (ClientCreateWindowEffective() != 4u) ::_exit(171); - // AND THE ARRAY IS SIZED FROM THE POOL IT SHARES, not from a second literal: the pool is - // Transport::kDefaultReplySlotCount slots addressed seq % count, and every reply-owning row - // (fence, query, the blocking rows themselves) needs one. Raising the window to the whole - // pool is the change the handoff names, and it is red here as well as at the static_assert - // in WireTables.cpp - deliberately twice, because the compile-time one is the guard and - // this one is the statement of what the guard is for. - if (ClientCreateWindowArraySize() > Transport::kDefaultReplySlotCount / 2) ::_exit(172); + MG_Config::Ipc.CreateWindow = 8; + if (ClientCreateWindowEffective() != 8u) ::_exit(171); + // AND THE ARRAY IS SIZED FROM THE STORE IT SHARES, not from a second literal (P12). This + // line used to read "at most half the REPLY POOL", which was true while an answer was read + // back out of a slot pool addressed seq % slotCount - and the window does not run on such + // a link: CreateWindowEffective() is gated on ILink::RetainsReplies, false for ShmLink, so + // the pool-addressed transport holds a window of 1. On the link where it does run, an + // answer waits in the declared-answer store instead, so the bound that matters is the + // store's - read from the constant rather than restated, for the reason the pool version + // read kDefaultReplySlotCount. WireTables.cpp's static_assert is the compile-time half of + // the same sentence, deliberately twice: that one is the guard, this one is what it is for. + if (ClientCreateWindowArraySize() > Transport::StreamLink::kRetainedAnswersMax / 2) + ::_exit(172); // TWELVE OBJECTS, ONE CREATE EACH, AND THE INVARIANT READ AFTER EVERY ONE. This is the // reading a silent overwrite cannot fake: the count stays at the ceiling while an entry is @@ -1268,7 +1271,7 @@ TEST(RemoteClientControls, TheWindowRefusesToDeferOnALinkThatCannotRetainAnswers }; // ---- THE KNOB IS AS HIGH AS IT GOES, AND THE ROW MUST STILL BLOCK ---- - MG_Config::Ipc.CreateWindow = 4; + MG_Config::Ipc.CreateWindow = ClientCreateWindowArraySize(); const Uint64 recordsBefore = Transport::LinkMetricsRecordsFor(createOp); const Uint64 waitsBefore = Transport::LinkMetricsReplyWaitsFor(createOp); burst(12); diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index 673d1299..13681605 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -37,6 +37,9 @@ #include #include #include +// P12: StreamLink::kRetainedAnswersMax, the store capacity RemoteClientControls' window-ceiling +// case reads instead of restating it. +#include #include #include #include diff --git a/MobileGL/MG_Test/Wire/StreamClientPublicationTest.cpp b/MobileGL/MG_Test/Wire/StreamClientPublicationTest.cpp index d8b7bc77..2d57b77a 100644 --- a/MobileGL/MG_Test/Wire/StreamClientPublicationTest.cpp +++ b/MobileGL/MG_Test/Wire/StreamClientPublicationTest.cpp @@ -262,7 +262,11 @@ namespace { TEST_F(StreamClientWindow, EveryDeferredCreateAnswerComesBackOnTheShippingArm) { const Uint32 savedWindow = MG_Config::Ipc.CreateWindow; - MG_Config::Ipc.CreateWindow = 4; + // THE DEEPEST WINDOW THE CODE SUPPORTS, so the case exercises the ceiling rather than a + // comfortable middle: more deferred answers outstanding means more of the store spent, and + // the crowding below is what has to survive it. (The CAP itself is pinned by + // RemoteClientControls' window-ceiling case; this one pins that the answers come back.) + MG_Config::Ipc.CreateWindow = 8; StartServo(); const std::uint64_t takenBefore = Client::ClientCreateWindowTaken(); constexpr Int32 kCreates = 24; @@ -331,7 +335,7 @@ namespace { EXPECT_LE(pending, Client::ClientCreateWindowEffective()); // AND THE ROW REALLY IS DEFERRING HERE, so the case cannot pass by taking the blocking // path on every create (which is what a link the window refuses to run on looks like). - EXPECT_EQ(Client::ClientCreateWindowEffective(), 4u); + EXPECT_EQ(Client::ClientCreateWindowEffective(), 8u); EXPECT_TRUE(client.LinkRetainsReplies()); MG_Config::Ipc.CreateWindow = savedWindow; }