diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index 24b918aa..03a7b752 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -649,6 +649,7 @@ namespace MobileGL::MG_Remote::Client { // 4. and ONLY NOW may anything an emitter owns be released: a var-tail still // referenced by an unapplied record is a use-after-free the join is what prevents. LogMemory("teardown"); + LogWireLedger(); m_producer.Detach(); m_encoder = Wire::PipeWireEncoder(); m_events = Transport::EventRingConsumer(); @@ -900,6 +901,45 @@ namespace MobileGL::MG_Remote::Client { Transport::LogRoleMemory(phase, SampleMemory()); } + // R-10's AND R-9's numbers IN EVERY SPLIT PRIVATE LOG, not only in the lanes that set + // MOBILEGL_PIPE_STATS=1. + // + // WHY IT IS HERE AND NOT ONLY ON THE STATS LINE. `MGPipe stats:` is an opt-in channel: two + // ctest entries out of 21 set MOBILEGL_PIPE_STATS, and neither the retrace lanes nor the + // 19 ordinary split entries do. R-10's proof obligation is about THE PHASE, not about the + // two counting lanes - "no record on the reduced path comes near half the ring" has to be + // readable from any split run that happened, which is what ID-53's per-entry private log + // is for. One line per session teardown costs nothing and cannot be missed. + // + // IT IS ALSO WHERE THE PROOF FAILS SOFTLY. A record ABOVE the cap already aborts on the + // spot with Fatal{RingOverrun} (PipeWireCodec.cpp), so this line's job is the other half: + // a maximum that is merely CLOSE to the cap is not a crash and would otherwise be + // invisible until the day a workload crossed it. The percentage is printed for exactly + // that reason, and R-10 names the integrator as the person who decides between early + // chunking and a bigger default ring when it climbs. + void ClientSession::LogWireLedger() const { + const Uint64 maxRecord = m_encoder.MaxRecordBytesSeen(); + const Uint64 cap = m_encoder.MaxRecordBytesCap(); + // Integer permille rather than a float: this file has no and a "%.1f" of a + // ratio nobody can reproduce by hand is worse than two integers. + const Uint64 permille = cap != 0 ? (maxRecord * 1000ull) / cap : 0ull; + MGLOG_I("MG_Remote client: wire ledger: maxrec=%llu maxrecop=%s cap=%llu (%llu.%llu%% of " + "RingProducer::MaxRecordBytes, half of a %llu byte SEG_CMD) cmdbytes=%llu " + "ringwraps=%llu ringpads=%llu " + "ringwaits=%llu emitseq=%llu - R-10's proof obligation and R-9's producer " + "readings, published from the session that produced them", + static_cast(maxRecord), m_encoder.MaxRecordOpName(), + static_cast(cap), + static_cast(permille / 10), + static_cast(permille % 10), + static_cast(cap * 2), + static_cast(m_encoder.CmdBytesWritten()), + static_cast(m_encoder.CmdWraps()), + static_cast(m_encoder.CmdWrapPads()), + static_cast(m_encoder.StageReclaimWaits()), + static_cast(m_encoder.EmitSeq())); + } + #undef MGP5_C0_STUB } // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index 2f971f9c..be165629 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -200,6 +200,9 @@ namespace MobileGL::MG_Remote::Client { // Peak-RSS accounting for t1 (RoleMemory.h). Transport::RoleMemorySample SampleMemory() const; void LogMemory(const char* phase) const; + // R-10's maximum record bytes and R-9's wrap/wait counts, at teardown, in whatever log + // this process writes. See the definition for why it is not only on the stats line. + void LogWireLedger() const; private: Wire::PipeWireEncoder m_encoder; diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 39fba630..431475ba 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include @@ -64,27 +65,85 @@ namespace MobileGL::MG_Remote::Client { Bool g_dropClearEmission = false; Uint64 g_droppedClearEmissions = 0; + Bool g_dropDrawEmission = false; + Uint64 g_droppedDrawEmissions = 0; + Uint64 g_presentOrdinal = 0; + Uint64 g_publishedMaxRecordBytes = 0; // E2's control has to be armable from OUTSIDE the process that runs the replay, because - // the statement it makes is about a trace lane and not about a unit case: "drop one - // Clear emission and OpenRA's SSIM falls below 0.99". A recompile would make the control + // the statement it makes is about a trace lane and not about a unit case: "drop an + // emission and OpenRA's SSIM falls below 0.99". A recompile would make the control // arm against source text, which is ID-22(a)'s defect. // // READ WITH getenv RATHER THAN THROUGH MG_Config, DELIBERATELY AND TEMPORARILY. Config.h - // is c0's and a new MOBILEGL_IPC_* knob goes through the integrator; this is a - // NEGATIVE-CONTROL switch no operator may ever set, and it announces itself at warning - // level every time it arms so it cannot be on by accident. Flagged for adoption into - // IpcTable if the integrator wants it there. - Bool ReadDropClearFromEnvironment() { - const char* value = std::getenv("MOBILEGL_IPC_E2_DROP_CLEAR"); - const Bool armed = value != nullptr && value[0] == '1' && value[1] == '\0'; - if (armed) { - MGLOG_W("MG_Remote client: MOBILEGL_IPC_E2_DROP_CLEAR=1 - E2's NEGATIVE CONTROL is " - "armed and every glClear will be DROPPED on the wire. This arm is expected " - "to fail its SSIM threshold; a lane that stays green with it set is not " - "going through the wire at all"); + // is c0's and a new MOBILEGL_IPC_* knob goes through the integrator; these are + // NEGATIVE-CONTROL switches no operator may ever set, and they announce themselves at + // warning level every time they arm so they cannot be on by accident. Flagged for + // adoption into IpcTable if the integrator wants them there. + Bool ReadControlKnob(const char* name) { + const char* value = std::getenv(name); + return value != nullptr && value[0] == '1' && value[1] == '\0'; + } + + // ---- WHY THERE ARE TWO OF THESE KNOBS, measured rather than assumed ----------------- + // + // MOBILEGL_IPC_E2_DROP_CLEAR came first and it does exactly what it says: every glClear + // stops at the client and no Clear record reaches the ring. It STILL COULD NOT TURN THE + // E2 RETRACE RED (joint-v1.md 3: SSIM 1.000000, mismatchPixels=0 with the knob armed and + // the arming WARN in the library's own log). That is not a broken knob, it is OpenRA: + // `apitrace dump` of openra.trace over the 31249 replayed calls counts 30 glClear, 30 + // glXSwapBuffers and 788 glDrawArrays, and the final frame issues its clear at call + // 30197 and then covers the surface four times over with a terrain layer + // (`glDrawArrays(GL_TRIANGLES, first=56064, count=16128)` x4, under a scissor of + // -24,-24,688x528 over a 640x480 surface) before the snapshot at 31249. A frame that + // overdraws every pixel it clears has a picture that does not depend on the clear, so + // "drop the clear" is a control whose observable is invisible to THIS trace - R-16's + // exact defect, a gate that cannot go red for its own reason. + // + // MOBILEGL_IPC_E2_DROP_DRAW is the honest form of the same statement for a trace lane: + // drop every DrawVbo record and the picture can only be the clear colour. It is the + // control that makes "the wire carried the frame" falsifiable, because the thing it + // removes is the thing the golden is made of. + // + // DROP_CLEAR IS KEPT rather than retired: it drops a real record, it now publishes the + // count it dropped (below), and a scenario whose picture DOES depend on its clear - + // ClearThenReadPixelsScenario is the reduced path's target A - is where it is + // observable. What it is no longer allowed to be is E2's retrace control. + void ArmControlKnobs() { + g_dropClearEmission = ReadControlKnob("MOBILEGL_IPC_E2_DROP_CLEAR"); + g_dropDrawEmission = ReadControlKnob("MOBILEGL_IPC_E2_DROP_DRAW"); + if (g_dropClearEmission) { + MGLOG_W("MG_Remote client: MOBILEGL_IPC_E2_DROP_CLEAR=1 - a NEGATIVE CONTROL is " + "armed and every glClear will be DROPPED on the wire. It is observable " + "only where the picture depends on the clear: OpenRA overdraws its whole " + "surface every frame, so this knob does NOT redden the E2 retrace " + "(measured, joint-v1.md 3) - MOBILEGL_IPC_E2_DROP_DRAW is the one that " + "does. The dropped count is published on the 'E2 control armed' line"); } - return armed; + if (g_dropDrawEmission) { + MGLOG_W("MG_Remote client: MOBILEGL_IPC_E2_DROP_DRAW=1 - E2's NEGATIVE CONTROL is " + "armed and every DrawVbo record will be DROPPED on the wire. The surface " + "can then only carry the clear colour, so this arm is expected to fail its " + "SSIM threshold; a lane that stays green with it set is not going through " + "the wire at all"); + } + } + + // THE CONTROL'S OWN EVIDENCE LINE, and it is emitted per frame rather than at teardown + // on purpose: a retrace that is killed by its own timeout, or whose library never runs + // MobileGL::Destroy, would leave a teardown-only line absent and the control would then + // have to accept a bare threshold failure - which is the thing R-16 forbids. One line + // per Present, only while a knob is armed, is bounded by the frame count and present + // whatever happens afterwards. + void LogE2ControlLine(Uint64 frameOrdinal) { + if (!g_dropClearEmission && !g_dropDrawEmission) return; + MGLOG_W("MGPipe: E2 control armed - drop-draw=%d drop-clear=%d, %llu records dropped " + "on the wire (draw=%llu clear=%llu), frame %llu", + g_dropDrawEmission ? 1 : 0, g_dropClearEmission ? 1 : 0, + static_cast(g_droppedDrawEmissions + g_droppedClearEmissions), + static_cast(g_droppedDrawEmissions), + static_cast(g_droppedClearEmissions), + static_cast(frameOrdinal)); } // ID-49's two halves, in one place so the emitter and its control read the same @@ -150,9 +209,11 @@ namespace MobileGL::MG_Remote::Client { BeforeReadOnlyVerb(); if (g_dropClearEmission) { - // E2's negative control. Everything above still ran, so the only difference + // A negative control. Everything above still ran, so the only difference // between this arm and the live one is the record - which is exactly the - // statement "the picture comes from the wire" that E2 exists to prove. + // statement "the picture comes from the wire" that E2 exists to prove. Its + // OBSERVABILITY is a property of the workload, not of this branch: see + // ArmControlKnobs for the measurement that took E2's retrace off this knob. ++g_droppedClearEmissions; return; } @@ -175,6 +236,16 @@ namespace MobileGL::MG_Remote::Client { ClientSession& session = RequireSession("DrawArrays"); BeforeDrawVerb(); + if (g_dropDrawEmission) { + // E2's LOAD-BEARING negative control. Same shape as the clear drop and the same + // rule: everything above still ran - the persistent-map push and the GPU-write + // mark walk both happened - so the ONLY difference from the live arm is that + // this frame's geometry never crossed the ring. A lane that still matches its + // golden with this armed did not get its picture from the wire. + ++g_droppedDrawEmissions; + return; + } + MG_Pipe::MGPDrawInfo info{}; info.Mode = static_cast(mode); info.IndexSize = 0; // arrays @@ -409,6 +480,50 @@ namespace MobileGL::MG_Remote::Client { ClientSession& session = RequireSession("Present"); BeforeReadOnlyVerb(); + // ---- R-10's and R-9's readings, published BEFORE the present record ------------ + // + // THE FRAME BOUNDARY IS THE RIGHT PLACE and the per-record path is the wrong one: + // the encoder keeps all five as run totals, so this is five relaxed stores per + // frame rather than five per record. Guarded by Enabled() like every other counting + // site in the tree, so the cost with MOBILEGL_PIPE_STATS unset is a global load and + // a predicted branch. + // + // AND IT IS BEFORE THE EmitAndWait BELOW, WHICH IS NOT A DETAIL. PipeStats::OnPresent + // is called by the SERVER's Present - i.e. from inside the apply of the very record + // this function is about to emit - so a publish placed after it lands one frame + // late, and the FIRST summary line of every run then reads `maxrec=0 maxcap=0`. + // Measured that way once: a zero that means "not published yet" is printed in the + // same shape as a zero that means "nothing crossed", and the second one is a real + // defect (an emit table that fell through to the driver). The Present record is 24 + // bytes and cannot be the maximum, so nothing is lost by reading one record early. + if (MG_Util::PipeStats::Enabled()) { + const Wire::PipeWireEncoder& encoder = session.Encoder(); + using MG_Util::PipeStats::Gauge; + MG_Util::PipeStats::PublishGauge(Gauge::MaxRecordBytes, encoder.MaxRecordBytesSeen()); + MG_Util::PipeStats::PublishGauge(Gauge::MaxRecordBytesCap, encoder.MaxRecordBytesCap()); + MG_Util::PipeStats::PublishGauge(Gauge::RingWraps, encoder.CmdWraps()); + MG_Util::PipeStats::PublishGauge(Gauge::RingWrapPads, encoder.CmdWrapPads()); + MG_Util::PipeStats::PublishGauge(Gauge::RingWaits, encoder.StageReclaimWaits()); + + // AND THE ROW, WHENEVER THE MAXIMUM MOVES. The summary line can carry the + // number but not the name - MG_Util is below MG_Remote and has no WireOpName - + // and the name is the actionable half: R-10 makes the integrator choose between + // early chunking and a bigger ring, and that is a decision about a record + // FAMILY. ClientSession::Stop prints the same pair at teardown, but a trace + // replay never reaches it (measured: the OpenRA lane's library log ends mid-run + // with no teardown line at all), so a stats-enabled run would otherwise publish + // a size with no row. Emitted only when the maximum actually grows, so it is + // bounded by the number of distinct maxima - five or six in a whole replay. + if (encoder.MaxRecordBytesSeen() > g_publishedMaxRecordBytes) { + g_publishedMaxRecordBytes = encoder.MaxRecordBytesSeen(); + MGLOG_I("MGPipe: wire ledger: new maximum record - maxrec=%llu " + "maxrecop=%s cap=%llu (R-10's proof obligation; P5 does not chunk)", + static_cast(g_publishedMaxRecordBytes), + encoder.MaxRecordOpName(), + static_cast(encoder.MaxRecordBytesCap())); + } + } + MG_Pipe::MGPPresent record{}; // FrameSerial 0 = "the server stamps its own". P5 has no client-side present credit // (MOBILEGL_IPC_PRESENT_CREDIT is P6's), so a client-minted serial would be a second @@ -423,6 +538,9 @@ namespace MobileGL::MG_Remote::Client { // keyed on pActiveBackendObject.get() (Core.cpp:34) and that pointer never changes // under split. session.PumpControlPlane(); + + ++g_presentOrdinal; + LogE2ControlLine(g_presentOrdinal); } // ============================================================================= @@ -599,7 +717,7 @@ namespace MobileGL::MG_Remote::Client { "the three classes no longer partition the 71 slots"); MG_Backend::GlobalBackendFunctionsTable BuildRemoteEmitTable() { - g_dropClearEmission = ReadDropClearFromEnvironment(); + ArmControlKnobs(); MG_Backend::GlobalBackendFunctionsTable table{}; // ---- class C first, so that a slot forgotten below stays Fatal rather than null. @@ -653,6 +771,9 @@ namespace MobileGL::MG_Remote::Client { void SetDropClearEmissionForNegativeControl(Bool drop) { g_dropClearEmission = drop; } Uint64 DroppedClearEmissions() { return g_droppedClearEmissions; } + void SetDropDrawEmissionForNegativeControl(Bool drop) { g_dropDrawEmission = drop; } + Uint64 DroppedDrawEmissions() { return g_droppedDrawEmissions; } + Bool ReadbackPackStateIsTightForTest(GLsizei width, Uint64 bytesPerPixel, const PixelStoreParameters& pack) { return ReadbackPackStateIsTight(width, bytesPerPixel, pack); diff --git a/MobileGL/MG_Remote/Client/EmitTables.h b/MobileGL/MG_Remote/Client/EmitTables.h index cb791e46..6a75dc88 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.h +++ b/MobileGL/MG_Remote/Client/EmitTables.h @@ -102,19 +102,27 @@ namespace MobileGL::MG_Remote::Client { Uint32 LocallyAnsweredSlotCount(); // class A - answered from the caps mirror, R-15 Uint32 UnmigratedSlotCount(); // class C - Fatal{UnmigratedVerb} - // THE E2 NEGATIVE CONTROL (t1's debt against c1, BRIEF §7). When set, the Clear emitter + // THE E2 NEGATIVE CONTROLS (t1's debt against c1, BRIEF §7). When set, the named emitter // SKIPS its record - it still runs the pre-verb hooks and still returns - so a replay that - // is really going through the wire loses one clear per frame and its SSIM falls below the - // 0.99 threshold, while a replay that fell through to the driver is unaffected. It is a - // function rather than a knob in Config.h for two reasons: the control has to be settable - // from a test process that has already started, and a knob would be a - // MOBILEGL_IPC_-shaped name for something no operator may ever set. + // is really going through the wire loses that verb while a replay that fell through to the + // driver is unaffected. They are functions rather than knobs in Config.h for two reasons: + // a control has to be settable from a test process that has already started, and a knob + // would be a MOBILEGL_IPC_-shaped name for something no operator may ever set. // - // Emissions actually skipped, so the control can assert that it DID something rather than + // Emissions actually skipped, so a control can assert that it DID something rather than // that a picture changed - a control that silently never fired is the third shape of R-16's // "a gate that cannot go red for its own reason". + // + // WHICH ONE E2'S RETRACE USES, and it is not the clear. Measured on the joint head: with + // MOBILEGL_IPC_E2_DROP_CLEAR=1 armed and its WARN in the library's own log, the OpenRA + // retrace under inproc still scored ssim=1.000000 / mismatchPixels=0 (joint-v1.md §3), + // because OpenRA covers every pixel it clears before the snapshot. Dropping the DRAWS is + // the control whose observable the golden is actually made of. See EmitTables.cpp's + // ArmControlKnobs for the trace census that settled it. void SetDropClearEmissionForNegativeControl(Bool drop); Uint64 DroppedClearEmissions(); + void SetDropDrawEmissionForNegativeControl(Bool drop); + Uint64 DroppedDrawEmissions(); // ID-47. The CLIENT refuses a readback whose answer would not fit a reply slot, BEFORE it // emits the record, and names the read. THE REFUSAL ITSELF IS s1's - diff --git a/MobileGL/MG_Remote/Client/PersistentMapTracker.cpp b/MobileGL/MG_Remote/Client/PersistentMapTracker.cpp index d3cda550..252d9a9b 100644 --- a/MobileGL/MG_Remote/Client/PersistentMapTracker.cpp +++ b/MobileGL/MG_Remote/Client/PersistentMapTracker.cpp @@ -85,7 +85,27 @@ namespace MobileGL::MG_Remote::Client { // here would make the control green for the wrong reason - it has to disable the // push, so that PersistentCoherentMapScenario draws the last uploaded bytes and goes // red exactly the way an unpushed map does. - if (blockBytes == 0) return; + // + // AND IT SAYS SO, ONCE. Until now this was a silent `return`, so E3(a)'s red could + // only ever be the scenario's pixel assertion and the control had no way to tell "the + // push was disabled" apart from "the push was never armed, or never reached, or the + // knob never got here" (joint-v1.md §3: "There is no Fatal for block size zero"; + // ID-65 assigns the line to x2). The control now requires BOTH: the pixel red AND + // this line in the entry's own private log. It is MGLOG_W and not a Fatal because 0 + // is a legal configured value whose whole purpose is to keep running with the push + // off; aborting here would turn every E3(a) entry into a subprocess abort and take + // the pixel evidence with it. + if (blockBytes == 0) { + if (!m_blockZeroAnnounced) { + m_blockZeroAnnounced = true; + MGLOG_W("MGPipe: persistent-map push disabled - MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 " + "is exit gate E3(a)'s NEGATIVE CONTROL, not 'unlimited': a live " + "persistent WRITE mapping's dirty blocks are NOT being pushed, so the " + "server draws whatever bytes last crossed by some other route. A lane " + "that stays green with this set is not getting its pixels from the push"); + } + return; + } const auto range = buffer.GetMappedRange(); const Uint64 begin = static_cast(range.start); diff --git a/MobileGL/MG_Remote/Client/PersistentMapTracker.h b/MobileGL/MG_Remote/Client/PersistentMapTracker.h index c8326a73..cffca073 100644 --- a/MobileGL/MG_Remote/Client/PersistentMapTracker.h +++ b/MobileGL/MG_Remote/Client/PersistentMapTracker.h @@ -96,6 +96,7 @@ namespace MobileGL::MG_Remote::Client { void ResetCountersForTest() { m_blocksPushed = 0; m_bytesPushed = 0; + m_blockZeroAnnounced = false; } void ClearForTest() { m_livePersistentMaps.clear(); @@ -111,6 +112,11 @@ namespace MobileGL::MG_Remote::Client { UnorderedMap m_livePersistentMaps; Uint64 m_blocksPushed = 0; Uint64 m_bytesPushed = 0; + // E3(a)'s diagnostic is emitted ONCE per process. PushBlocksFor runs at every validate + // point of every member, so an unlatched MGLOG_W would be one line per draw per mapping + // - and a control that has to grep a log cannot tell a message that fired from a + // message that flooded. + Bool m_blockZeroAnnounced = false; }; // What the client's emit table calls immediately BEFORE emitting any verb that can read a diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 1f901c27..6085d7f6 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -770,6 +770,15 @@ namespace MobileGL::MG_Remote::Wire { // One try at reclaiming what the server has already retired. A second failure // means the bytes genuinely do not fit, which R-10 says P5 does not chunk and // must instead prove it never needs to. + // + // AND THIS IS P5'S ONE REAL BACK-PRESSURE EVENT, so it is counted here and + // published as `ringwaits=`. Reaching this line means the producer could not + // place a blob until the CONSUMER had retired earlier ones - the producer's + // progress depended on retiredSeq, which is exactly what R-9's "batching may + // only delay a watermark" is about. Exit gate E3(e)'s small-ring lane exists + // to make it happen at least once; a lane that never reaches it has a ring + // that is small only in its environment block. + ++m_stageReclaimWaits; ReclaimStagedBytes(); } } @@ -889,8 +898,48 @@ namespace MobileGL::MG_Remote::Wire { if ((callFlags & static_cast(kHasBlob)) != 0) ringFlags |= Transport::kRecHasBlob; if ((callFlags & static_cast(kVarTail)) != 0) ringFlags |= Transport::kRecVarTail; + // BOTH WRAP READINGS ARE TAKEN FROM THE PRODUCER'S OWN CURSOR, not from a flag Reserve + // does not return. The cursor is a monotonic byte count and the ring is indexed + // `cursor & mask`, so `cursor / capacity` is the number of times the byte area has been + // reused - and Reserve advances the cursor by `total` for a contiguous record and by + // `spaceToEnd + total` when it had to lay a kRecPad filler down to the boundary first + // (Ring.cpp). Everything below is exact arithmetic over those two facts, which keeps + // both counters in the encoder - where R-10's maximum already lives - rather than + // adding members to a Transport class the wire package does not own. + // + // AND THEY ARE TWO COUNTERS BECAUSE THEY ARE TWO EVENTS, which one measurement made + // unmissable: a workload whose records repeat at a uniform stride that DIVIDES the + // capacity lands on the boundary exactly, every time, for ever. Driving 1310824 bytes + // of clears and draws through a 1 MiB SEG_CMD produced ZERO pads - the ring went round + // once and a half and never straddled - so "did a pad happen" is NOT the question "did + // this ring wrap", and a lane that asked the first one while meaning the second would + // have gone red for a property of its own arithmetic. + // + // m_cmdWraps the head crossed a multiple of the capacity: the ring went round. + // Guaranteed once more bytes are written than the ring holds, which + // is what makes it something exit gate E3(e) can ASSERT. + // m_cmdWrapPads a kRecPad filler was laid because a record would have straddled + // the boundary. R-9's "a pad does not advance seq, both sides skip + // it and count again" is about THIS one, and it is RECORDED rather + // than asserted, because whether it ever happens is a property of + // the record sizes and not of the ring. + const Uint64 headBeforeReserve = m_cmd->LocalHead(); void* slot = m_cmd->Reserve(static_cast(op), ringFlags, total - sizeof(MGPWireRecHeader)); + if (slot != nullptr) { + const Uint64 headAfterReserve = m_cmd->LocalHead(); + if ((headAfterReserve - headBeforeReserve) > total) { + ++m_cmdWrapPads; + } + const Uint64 capacity = m_cmd->Capacity(); + if (capacity != 0) { + // A record is capped at capacity/2 and its pad at capacity/2 too, so one + // Reserve can cross at most one boundary; the subtraction is still written as + // a difference of quotients rather than as a Bool, because that stays correct + // if the cap ever changes. + m_cmdWraps += (headAfterReserve / capacity) - (headBeforeReserve / capacity); + } + } if (slot == nullptr) { // The ring is full, not the record too big - Reserve refuses an oversized record // above, and we already proved this one is not. The caller publishes, waits for @@ -967,6 +1016,13 @@ namespace MobileGL::MG_Remote::Wire { if (total > m_maxRecordBytes) { m_maxRecordBytes = total; + // WHICH ROW IT WAS, not just how big. R-10 makes the integrator choose between early + // chunking and a bigger default ring when the maximum climbs, and that choice is + // about a specific record family - a var-tail whose length the GL limits bound, or + // one the emitter has to split itself. A number with no row attached leaves the + // reader to guess which, and the guess in c1-v2.md §10 was SetGlobalConstants while + // the measured answer on the reduced path is a different row entirely. + m_maxRecordOp = op; } ++m_emitSeq; // The stage mark: where SEG_STAGE stood once everything this record names had been @@ -1077,6 +1133,29 @@ namespace MobileGL::MG_Remote::Wire { Uint64 PipeWireEncoder::MaxRecordBytesSeen() const { return m_maxRecordBytes; } + const char* PipeWireEncoder::MaxRecordOpName() const { + return m_maxRecordOp == MG_Pipe::MGPWireOp::kOpCount ? "none" : WireOpName(m_maxRecordOp); + } + + Uint64 PipeWireEncoder::MaxRecordBytesCap() const { + // Read from the ring, not recomputed from MOBILEGL_IPC_RING_MB: the number the proof + // has to hold against is the capacity this process's producer actually got, and the + // two differ the moment a session clamps or rounds the configured size. + return (m_cmd != nullptr && m_cmd->Valid()) ? m_cmd->MaxRecordBytes() : 0; + } + + Uint64 PipeWireEncoder::CmdWraps() const { return m_cmdWraps; } + + Uint64 PipeWireEncoder::CmdWrapPads() const { return m_cmdWrapPads; } + + Uint64 PipeWireEncoder::StageReclaimWaits() const { return m_stageReclaimWaits; } + + Uint64 PipeWireEncoder::CmdBytesWritten() const { + // LocalHead(), not RingControl::head: the producer's own cursor includes records + // reserved but not yet published, and this number is about what the PRODUCER wrote. + return (m_cmd != nullptr && m_cmd->Valid()) ? m_cmd->LocalHead() : 0; + } + // --------------------------------------------------------------------------------- // Decoder // --------------------------------------------------------------------------------- diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index ea2360bb..acbf108f 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -276,6 +276,54 @@ namespace MobileGL::MG_Remote::Wire { // R-10's proof obligation: the largest single record this encoder has written. Uint64 MaxRecordBytesSeen() const; + // The op whose record set that maximum, by name, or "none" before any record. Published + // beside the number so R-10's integrator decision names a row rather than a size. + const char* MaxRecordOpName() const; + + // THE CAP THAT NUMBER IS PROVED AGAINST, read from the ring rather than recomputed. + // RingProducer::MaxRecordBytes() == Capacity()/2, and Capacity() is + // MOBILEGL_IPC_RING_MB. Published beside MaxRecordBytesSeen() so a reader never has to + // multiply an environment variable to know whether the proof holds - which is the one + // arithmetic step between "4 MiB" and "half of the ring this process actually got". + // 0 when this encoder has no command ring (a default-constructed one). + Uint64 MaxRecordBytesCap() const; + + // ---- R-9's producer readings, and why they are three rather than one --------------- + // + // `CmdWraps()` counts SEG_CMD going ROUND: the number of times the producer's monotonic + // head crossed a multiple of the ring capacity and the byte area was reused from the + // start. It is what exit gate E3(e)'s small-ring lane asserts, because it is the one + // that is GUARANTEED once a workload writes more bytes than the ring holds, and + // therefore the one a lane can be red for not reaching. + // + // `CmdWrapPads()` counts the kRecPad fillers Reserve lays when a record would have + // STRADDLED that boundary. R-9's last clause - "a pad record does not advance seq, both + // sides must skip it and count again" - is about this one, and it is RECORDED rather + // than asserted: measured, a stream of clears and draws repeats at a stride that + // divides a power-of-two capacity exactly, so 1310824 bytes through a 1 MiB SEG_CMD + // produced one and a half trips round the ring and ZERO pads. A gate written against + // this number would have been red for the arithmetic of the record catalogue rather + // than for anything about the ring. + // + // `StageReclaimWaits()` counts every SEG_STAGE allocation that did not fit until the + // encoder reclaimed the runs the server had already retired - i.e. every time the + // producer's progress depended on the consumer's retiredSeq. That is the honest + // back-pressure reading in P5, and the reason the command ring has none: the verb + // barrier makes EmitAndWait wait for appliedSeq after EVERY record (R-1), so at most + // one record is ever in flight on SEG_CMD and a full command ring is not a wait but a + // Fatal{RingOverrun} (ClientSession.cpp). Publishing a "command ring waits" counter + // that can only ever be zero-or-dead is the decoration this file's counters are not. + Uint64 CmdWraps() const; + Uint64 CmdWrapPads() const; + Uint64 StageReclaimWaits() const; + + // Bytes this encoder has ever written into SEG_CMD, pad fillers included: the + // producer's monotonic head cursor. It is the DENOMINATOR the wrap count only means + // anything against - "0 wraps" is a defect when the run pushed more bytes than the ring + // holds and a tautology when it pushed fewer, and only this number tells those apart. + // It is also how E3(e)'s lane knows when it has driven enough work, without guessing a + // record size. 0 when there is no command ring. + Uint64 CmdBytesWritten() const; private: // {the record's seq, the SEG_STAGE cursor just past everything that record named}. @@ -307,6 +355,10 @@ namespace MobileGL::MG_Remote::Wire { SegmentTable* m_segments = nullptr; Uint64 m_emitSeq = kInvalidSeq; Uint64 m_maxRecordBytes = 0; + MG_Pipe::MGPWireOp m_maxRecordOp = MG_Pipe::MGPWireOp::kOpCount; + Uint64 m_cmdWraps = 0; + Uint64 m_cmdWrapPads = 0; + Uint64 m_stageReclaimWaits = 0; Vector m_stageMarks; SizeT m_stageMarkFront = 0; Uint8* m_stageBase = nullptr; diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 4e1b8508..2dd13843 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1161,6 +1161,74 @@ TEST_F(PipeWireCodecTest, MaxRecordBytesSeenStaysFarBelowHalfTheRing) { // MGPFramebufferState at 304, so a record only ever grows through its TAIL - which is why // the counter is on the encoder and not a constant. EXPECT_LT(8u + sizeof(MGPFramebufferState), wire.Cmd().MaxRecordBytes()); + + // AND THE CAP THE PROOF IS AGAINST IS PUBLISHED BY THE ENCODER ITSELF. Everything outside + // MG_Remote - the summary line's `maxrec=`/`maxcap=`, the session's teardown ledger, the + // integration lanes' Harness/WireLedgerChecks - compares against MaxRecordBytesCap() rather + // than against MOBILEGL_IPC_RING_MB / 2, because the cap moved twice in this phase without + // the environment variable changing (see the paragraph above). If those two ever disagree, + // every published `maxrec` percentage is measured against the wrong denominator, and this + // is the case that says so. + EXPECT_EQ(wire.Encoder().MaxRecordBytesCap(), wire.Cmd().MaxRecordBytes()); + EXPECT_EQ(wire.Encoder().MaxRecordBytesCap(), Wire2::kCmdBytes / 2); +} + +TEST_F(PipeWireCodecTest, TheCommandRingWrapCountIsWhatTheHeadActuallyDid) { + // R-9's wrap reading, and the distinction exit gate E3(e) turned out to depend on. + // + // `CmdWraps()` is the head crossing a multiple of the capacity - the ring going ROUND - + // and it is guaranteed once more bytes are written than the ring holds. `CmdWrapPads()` is + // the narrower event: a record that would have STRADDLED the boundary and needed a kRecPad + // filler, which is the case R-9's "a pad does not advance seq, both sides skip it and + // count again" is about. + // + // THEY ARE NOT THE SAME NUMBER, and assuming they were is what this case exists to + // prevent. A stream of identically sized records whose stride divides a power-of-two + // capacity lands on the boundary EXACTLY every time and never straddles it: measured in + // the split lane, 1310824 bytes of clears and draws through a 1 MiB SEG_CMD produced one + // wrap and ZERO pads. The first cut of E3(e)'s assertion read the pad count and went red + // for that arithmetic rather than for anything about the ring. + Wire2 wire; + EXPECT_EQ(wire.Encoder().CmdWraps(), 0u); + EXPECT_EQ(wire.Encoder().CmdWrapPads(), 0u); + EXPECT_EQ(wire.Encoder().CmdBytesWritten(), 0u); + + // The stride is MEASURED rather than computed from sizeof: Reserve rounds the header plus + // payload up to 8, and a case that restated that arithmetic would be asserting its own + // copy of Ring.cpp rather than what the producer did. + MGPBindRenderState bind{}; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindRenderState, &bind, sizeof(bind)), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + const Uint64 stride = wire.Encoder().CmdBytesWritten(); + ASSERT_GT(stride, 0u); + EXPECT_EQ(wire.Encoder().CmdWraps(), 0u) << "one record cannot have taken the ring round"; + + // One trip round and a little more, draining after every record so the producer never meets + // its own tail. This is the same shape the split lane has under the verb barrier: one + // record in flight at a time, the ring recycled behind it - so a wrap here is a wrap + // there, and not an artefact of a backed-up queue. + const Uint64 records = (Wire2::kCmdBytes / stride) + 3; + for (Uint64 i = 1; i < records; ++i) { + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindRenderState, &bind, sizeof(bind)), + kInvalidSeq) + << "the ring refused record " << i << " of " << records; + ASSERT_TRUE(wire.PumpOne(&applied)); + } + + EXPECT_GT(wire.Encoder().CmdBytesWritten(), Wire2::kCmdBytes); + EXPECT_EQ(wire.Encoder().CmdWraps(), 1u); + + // AND THE PAD COUNT IS A DIFFERENT NUMBER. With this record the stride is 24 bytes and the + // ring is 65536, which leaves a 16-byte remainder: exactly one record per trip finds fewer + // than 24 bytes to the boundary and gets a kRecPad filler. Change the stride to one that + // DIVIDES the capacity and the same trip produces no pad at all - measured in the split + // lane, where 1310824 bytes of clears and draws through a 1 MiB SEG_CMD reported + // ringwraps=1 ringpads=0. That is why exit gate E3(e) asserts the WRAP and only records + // the pad: a gate on the pad count would be a gate on the sizes in the record catalogue. + EXPECT_EQ(Wire2::kCmdBytes % stride, 16u) << "stride " << stride; + EXPECT_EQ(wire.Encoder().CmdWrapPads(), 1u); } TEST_F(PipeWireCodecTest, ABigProgramArchiveDoesNotGrowItsRecordAtAll) { @@ -1609,6 +1677,35 @@ TEST_F(PipeWireCodecTest, ANonZeroSizeWithNoSegmentIsFatal) { EXPECT_NE(r.Log.find("with no segment"), std::string::npos) << r.Log; } +TEST_F(PipeWireCodecTest, ARecordLargerThanHalfTheRingIsFatalRingOverrun) { + // R-10's PROOF OBLIGATION, FAILING ON PURPOSE - the red-once for everything the phase + // publishes as `maxrec=`. P5 does no chunking: a record above + // RingProducer::MaxRecordBytes() == Capacity()/2 must abort by name at the ENCODER, on the + // producing side, rather than becoming a nullptr from Reserve that some caller reads as + // "the ring is full, wait" - which on an EMPTY ring would be a wait that never ends. + // + // The oversized record is a REAL one from the catalogue with a long var-tail, not a forged + // header: MGPDrawInfo declares NumDraws and the encoder cross-checks the tail against the + // layout that number implies, so this is the shape a genuine emitter bug would take. + const ChildResult r = RunInChild([] { + Wire2 wire; + MGPDrawInfo info{}; + // Just over half the ring. Wire2's SEG_CMD is 64 KiB, so the cap is 32 KiB. + const Uint32 draws = + static_cast(((Wire2::kCmdBytes / 2) / sizeof(MGPDrawRange)) + 8); + info.NumDraws = draws; + std::vector ranges(draws); + (void)wire.Encoder().EncodeRecord(MGPWireOp::DrawVbo, &info, sizeof(info), ranges.data(), + ranges.size() * sizeof(MGPDrawRange)); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("Fatal{RingOverrun,"), std::string::npos) << r.Log; + EXPECT_NE(r.Log.find("exceeds RingProducer::MaxRecordBytes()"), std::string::npos) << r.Log; + // The diagnostic has to name R-10 and the decision it forces, because the person reading it + // has to choose between early chunking and a bigger ring and neither is a local fix. + EXPECT_NE(r.Log.find("does not chunk (R-10)"), std::string::npos) << r.Log; +} + TEST_F(PipeWireCodecTest, ARunThatLeavesItsSegmentIsFatal) { // R-2 arm 4. const ChildResult r = RunInChild([] { diff --git a/MobileGL/MG_Util/Metrics/PipeStats.cpp b/MobileGL/MG_Util/Metrics/PipeStats.cpp index 181c0c19..a988db8a 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.cpp +++ b/MobileGL/MG_Util/Metrics/PipeStats.cpp @@ -153,6 +153,13 @@ namespace MobileGL::MG_Util::PipeStats { Uint64 g_windowBaseGateMiss[kGateCount] = {}; Uint64 g_windowBaseFrames = 0; Bool g_shutdownDone = false; +#if MOBILEGL_PIPE_PUSH + // The gauges' storage. Relaxed atomics like every other counter here: the publisher is + // the GL thread at a frame boundary and the reader is whoever formats the line, which + // under split can be the apply thread. + Counter g_gauges[static_cast(Gauge::Count)] = {}; + constexpr Uint32 kGaugeCount = static_cast(Gauge::Count); +#endif // Frames per summary line, latched by Init() from MOBILEGL_PIPE_STATS_PERIOD. Uint64 g_summaryPeriod = kDefaultSummaryFramePeriod; @@ -258,6 +265,11 @@ namespace MobileGL::MG_Util::PipeStats { } g_frameCount.store(0, std::memory_order_relaxed); g_windowBaseFrames = 0; +#if MOBILEGL_PIPE_PUSH + for (Uint32 i = 0; i < kGaugeCount; ++i) { + g_gauges[i].store(0, std::memory_order_relaxed); + } +#endif } void EmitSummaryLine() { @@ -329,6 +341,19 @@ namespace MobileGL::MG_Util::PipeStats { Bump(g_totalCalls[index], count); } +#if MOBILEGL_PIPE_PUSH + // A STORE, NOT A BUMP, and the difference is the whole reason these are a separate kind. + // The publisher hands over its OWN run total (a maximum, or a count it has been keeping + // since the session opened), so accumulating deltas here would double every reading; and a + // maximum is not additive at all. Publishing the same value twice is a no-op, which is + // what makes it safe to call at every frame boundary. + void PublishGauge(Gauge gauge, Uint64 value) { + g_gauges[static_cast(gauge)].store(value, std::memory_order_relaxed); + } + + Uint64 GaugeValue(Gauge gauge) { return Read(g_gauges[static_cast(gauge)]); } +#endif + void CountGate(Gate gate, Bool hit) { const Uint32 index = static_cast(gate); if (hit) { @@ -489,6 +514,22 @@ namespace MobileGL::MG_Util::PipeStats { // tracks the draw count is a pull inside a loop, and one that tracks the frame count is // a pull per verb. Zero in every monolith lane by construction. line += " rsp=" + std::to_string(calls[static_cast(CallClass::ResidualPulls)]); + // P5's three wire gauges, and THEY ARE RUN TOTALS on a line whose every other field is + // a window - see the Gauge enum for the argument. `maxrec` is R-10's proof obligation + // (BRIEF 8 item 3): the largest single record this run wrote, in BYTES, beside the cap + // it has to stay under so nobody has to multiply MOBILEGL_IPC_RING_MB by hand. `maxcap` + // reads 0 when this process has no wire producer, which is what a monolith lane prints + // and is NOT the same statement as "the cap is zero". + // + // ringwraps / ringwaits are R-9's: SEG_CMD wrap pads and SEG_STAGE waits on retiredSeq. + // A small-ring lane whose ringwraps stays 0 ran the default lane's workload under a + // different environment block, which is exactly what exit gate E3(e) was recorded as + // NOT having proved (joint-v1.md 6). + line += " maxrec=" + std::to_string(Read(g_gauges[static_cast(Gauge::MaxRecordBytes)])); + line += " maxcap=" + std::to_string(Read(g_gauges[static_cast(Gauge::MaxRecordBytesCap)])); + line += " ringwraps=" + std::to_string(Read(g_gauges[static_cast(Gauge::RingWraps)])); + line += " ringpads=" + std::to_string(Read(g_gauges[static_cast(Gauge::RingWrapPads)])); + line += " ringwaits=" + std::to_string(Read(g_gauges[static_cast(Gauge::RingWaits)])); #endif line += "] gates["; for (Uint32 i = 0; i < kGateCount; ++i) { @@ -544,6 +585,20 @@ namespace MobileGL::MG_Util::PipeStats { ", \"miss\": " + std::to_string(Read(g_totalGateMiss[i])) + "}"; json += (i + 1 == kGateCount) ? "\n" : ",\n"; } +#if MOBILEGL_PIPE_PUSH + // The gauges, under their long names. Run totals here as on the summary line. + json += " },\n \"wire\": {\n"; + json += " \"max-record-bytes\": " + + std::to_string(Read(g_gauges[static_cast(Gauge::MaxRecordBytes)])) + ",\n"; + json += " \"max-record-bytes-cap\": " + + std::to_string(Read(g_gauges[static_cast(Gauge::MaxRecordBytesCap)])) + ",\n"; + json += " \"ring-wraps\": " + + std::to_string(Read(g_gauges[static_cast(Gauge::RingWraps)])) + ",\n"; + json += " \"ring-wrap-pads\": " + + std::to_string(Read(g_gauges[static_cast(Gauge::RingWrapPads)])) + ",\n"; + json += " \"ring-waits\": " + + std::to_string(Read(g_gauges[static_cast(Gauge::RingWaits)])) + "\n"; +#endif json += " },\n \"cmd-bytes-per-draw-histogram\": ["; for (Uint32 i = 0; i < kPayloadHistogramBuckets; ++i) { if (i != 0) { diff --git a/MobileGL/MG_Util/Metrics/PipeStats.h b/MobileGL/MG_Util/Metrics/PipeStats.h index 79932baa..bf7bce97 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.h +++ b/MobileGL/MG_Util/Metrics/PipeStats.h @@ -183,6 +183,58 @@ namespace MobileGL::MG_Util::PipeStats { Count }; +#if MOBILEGL_PIPE_PUSH + // P5's GAUGES, and they are a THIRD KIND of counter rather than three more CallClass rows. + // + // A ByteClass and a CallClass are SUMS this module owns and a call site increments. These + // three are neither: they are the wire producer's own running readings - a MAXIMUM and two + // RUN TOTALS that live on MG_Remote's encoder, which this module cannot see and must not + // link against (MG_Util is below MG_Remote, and the pull build has no MG_Remote at all). + // The owner publishes its current value at the frame boundary and this module prints the + // last one it was given. Summing them here would be wrong twice: a maximum is not additive, + // and the encoder already holds the run total, so adding deltas would double-count. + // + // THEY ARE RUN TOTALS ON A WINDOWED LINE, deliberately and against the file's own habit. + // Everything else on the summary line covers "since the previous line" because a run total + // over a workload whose shape changes hides the number P2 wants. These three are the + // opposite: "the largest record this run ever wrote" and "did the ring ever wrap" are + // questions about the RUN, and a windowed maximum would read 0 in every window that did not + // happen to contain the biggest record - which is the shape of a proof obligation that + // cannot fail. The label says so in the line itself (`maxrec=` is bytes, not bytes/frame). + // + // PUSH-ONLY for the reason every counter added since P2 is: the pull build must stay + // symbol-identical (gate G1), and a gauge whose only publisher is MG_Remote could never + // leave zero there. + enum class Gauge : Uint32 { + // R-10's PROOF OBLIGATION. The largest single record the wire encoder has written, in + // bytes, and the cap it must stay under - RingProducer::MaxRecordBytes() == + // MOBILEGL_IPC_RING_MB / 2. P5 does no chunking and has to prove it needs none; before + // this pair existed the only consumers of PipeWireEncoder::MaxRecordBytesSeen() were + // codec unit tests, so BRIEF 8 item 3 had no measurement from any real workload + // (joint-v1.md 5, "Maximum record bytes: NO MEASUREMENT"). + MaxRecordBytes = 0, + MaxRecordBytesCap, + // R-9's three producer readings. `RingWraps` is SEG_CMD going ROUND - the head crossing + // a multiple of the capacity - which is the event exit gate E3(e)'s small-ring lane + // asserts, because it is guaranteed once the workload writes more bytes than the ring + // holds. `RingWrapPads` is the kRecPad fillers laid when a record would have STRADDLED + // that boundary, which is R-9's "a pad does not advance seq" path and is RECORDED, not + // asserted: a uniform record stride over a power-of-two ring lands on the boundary + // exactly and never straddles it (measured). `RingWaits` is SEG_STAGE allocations that + // had to wait on the consumer's retiredSeq. See PipeWireCodec.h for why the command + // ring contributes no wait count while the verb barrier is armed. + RingWraps, + RingWrapPads, + RingWaits, + Count + }; + + // Publishes the owner's current reading. Cheap and unconditional on the caller's side: + // the call sites are per-frame, not per-record. + void PublishGauge(Gauge gauge, Uint64 value); + Uint64 GaugeValue(Gauge gauge); +#endif + // Memo gates. Each is a place where a backend decides "nothing moved, skip the work". // Hit == the gate short-circuited; Miss == it fell through and did the work. The six // are exactly the ones section 2.3.1 tabulates.