From 6e6b86e592e2aa1c67e3b83d60c7d6a7d3032b42 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 07:58:06 -0400 Subject: [PATCH] [MG_Remote, MG_Pipe, MG_Impl, MG_Backend] (Disaggregated): P5e ra - the wait rule, the present credit and server-owned gPipeInputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CONTRACT-P5E §2.3. EmitAndWaitTails now switches on the generated WaitClass column when RunAheadArmed(): kWaitNone and kWaitPresent return at the publish, kWaitApplied waits and then releases the fill pins, and a reply row is unchanged. It REPLACES the BatchWaits skip rather than layering on it - the batch rule reads the call CLASS and this one reads what an apply READS, and the server's MGPipeBarriered reads the second, so two rules layered is two halves of the wire disagreeing about whether the client is parked. - §1: RunAheadArmed() = barrierArmed && Ipc.RunAhead && Caps().HasCap(kCapRunAheadApply), latched at the first caps adoption after Start and demotable but never promotable - a record already published without a wait cannot be un-published. A knob set against a server that does not publish the bit logs once and runs lockstep. - §2.4: MGPPresent::FrameSerial is minted client-side, 1-based, by AcquirePresentCredit, which pays the credit BEFORE the encode (a client that encoded first would hold a SEG_CMD reservation across the park). ServerVerbSink::OnPresent returns one credit per swap through ServerSession::ReturnPresentCredit, which has had no production caller since v1 wrote it; serial 0 on a run-ahead server is Fatal{ProtocolCorruption, "Present.FrameSerial"}. The stats line and the wire ledger gain credit-waits. - §2.5, the drains and the forced waits: every wait exit drains SEG_EVENT (the applied wait, the present-ack wait, the cmd-space wait and - through a hook the session lends the encoder - the stage bell); glFinish becomes WaitForApplyToCatchUp, since "the commands issued so far have completed" is a promise a queue nobody waits on can break; every Server* EGL forwarder waits first, because the control mailbox is pumped BETWEEN drain batches and a make-current landing between two run-ahead records is not a wrong pixel but a wrong everything. ServerSwapEGLBuffers is not on that list: present is the swap. - §2.5 / ruling 15 (ID-93): resource_subdata's BUFFER half stops waiting. The row keeps kReplySlot, the server still posts, and the slot is never read - which is what makes the 64 KB persistent-map push fire-and-forget instead of a hidden round trip per block. MGPipeSubDataWantsItsReply is the one predicate, read by the route and by the emit table, so the two cannot disagree about whether a wait is owed. The texture half keeps its reply: DrainTextureSubData clears a level's dirty flag on the accepted answer. - §3: gPipeInputs becomes server-role memory for unbarriered records. The tracker walk and the emitters still run - they produce records - but the serial bump, the verb stamp, the server-stamp withdrawal and the whole of step 4 do not, and MGPipeNoteFrontendMutation / MGPipeLeaveVerb return instead of writing. `fillOwed` is a separate name from `barriered` on purpose: the red-once is that one line. - §3.3: MGPipeInputUnfreshRead's counter becomes an UNCONDITIONAL abort under an unbarriered record, knob or no knob, because the value is torn or stale by construction and there is nothing for a counter to count. The marker gains its reason so the lane's allowlist can tell the two apart. - §3.5: RefusePipeInputsTouchWhileApplierOwnsIt is re-derived - under run-ahead the rule is "not at all unless you are the barriered fill", and the BatchWaits early return goes with the argument that justified it. - §2.3's ReleaseFillPins drops the four O-class SharedPtr rows once a barriered apply returns: under lockstep the next fill overwrote them within a verb, under run-ahead a pinned VAO can make the apply thread its last owner a frame later. - §2.7 / ruling 13 (ID-91): the deferred-destroy queue stays live, and an enqueue from an UNBARRIERED apply is now a named finding - loud once, Fatal under strict. - NAMED DEVIATION from §3.1: the identity/liveness pair is still written on every verb. It is not a field of the residual model - no ownership row owns it, no record carries it, no server stamp can answer it - it is "does this process still have a GL context", which only this thread knows and every null-context guard reads. - Inert until the integration commit: RunAheadArmed() is false while kMGPipeP5eRunAheadReady is, so every branch above answers as it did before. --- MobileGL/MG_Backend/MGPipe/PipeInputs.cpp | 38 +- .../MG_Impl/GLImpl/Exporting/Definitions.cpp | 13 +- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 259 ++++++++++++-- MobileGL/MG_Pipe/PipeMutation.h | 30 ++ MobileGL/MG_Pipe/PipeRoute.h | 31 +- .../MG_Remote/Client/BackendObject_Remote.cpp | 26 ++ MobileGL/MG_Remote/Client/ClientSession.cpp | 326 +++++++++++++++++- MobileGL/MG_Remote/Client/ClientSession.h | 89 ++++- MobileGL/MG_Remote/Client/EmitTables.cpp | 18 +- MobileGL/MG_Remote/Client/WireTables.cpp | 30 +- MobileGL/MG_Remote/Server/PipeApplier.cpp | 38 ++ MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 10 +- MobileGL/MG_Remote/Wire/PipeWireCodec.h | 17 + 13 files changed, 867 insertions(+), 58 deletions(-) diff --git a/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp b/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp index 939c03b5..bc8f9741 100644 --- a/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp +++ b/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp @@ -58,22 +58,50 @@ namespace MobileGL::MG_Pipe { // existing filter on Fatal{UnmigratedPipeInput still matches, plus the class and the // phase that retires it - a strict abort that did not say which phase owes the answer // would leave the reader exactly where the gate found them. - [[noreturn]] void StrictBarrierPullFatal(MGPipeInputField field, MGPipeVerb verb) { + // P5e (ra): `why` is the second half of the marker, because the lane's allowlist step + // reads this line and the two reasons mean different things to it. "MOBILEGL_IPC_ + // STRICT_ERRORS=1" is the operator asking for a real, ordered value to be loud; + // "UNBARRIERED" is the value not existing (§3.3). The `@` prefix and the + // Fatal{UnmigratedPipeInput tag are unchanged, so every filter written since P5c still + // matches. + [[noreturn]] void StrictBarrierPullFatal(MGPipeInputField field, MGPipeVerb verb, + const char* why) { const SizeT index = static_cast(field); - MGLOG_F("MGPipe: Fatal{UnmigratedPipeInput, \"%s@%s\"} [BARRIER-PULLED, " - "MOBILEGL_IPC_STRICT_ERRORS=1, retires in %s]", - kMGPipeInputFieldNames[index], MGPipeVerbName(verb), kMGPipeFieldRetiringPhase[index]); + MGLOG_F("MGPipe: Fatal{UnmigratedPipeInput, \"%s@%s\"} [BARRIER-PULLED, %s, " + "retires in %s]", + kMGPipeInputFieldNames[index], MGPipeVerbName(verb), why, + kMGPipeFieldRetiringPhase[index]); std::abort(); } // One place decides what a BARRIER-PULLED read does, so the field accessors and the // seven sticky forwards cannot drift apart on it. + // + // ---- P5e (ra), CONTRACT-P5E §3.3: THE DETECTOR IS UNCONDITIONAL UNDER AN UNBARRIERED + // RECORD, AND THAT IS WHAT MAKES THE STRICT LANE A GATE ------------------------------- + // + // The knob exists because under LOCKSTEP a pulled row is a real, ordered, fresh value: + // the client filled it and then parked, so "count it and carry on" is an honest + // measurement of remaining debt and MOBILEGL_IPC_STRICT_ERRORS is the operator asking + // for the debt to be loud instead. None of that survives run-ahead. With the client + // running ahead of this apply, the row was either never filled for this verb (§3.1 + // skips the fill) or is being overwritten by a verb two frames later - so the value is + // torn or stale BY CONSTRUCTION and there is nothing for a counter to count. A + // "count it" arm here would be a wrong picture with a number beside it. + // + // rsp is therefore 0 on unbarriered records in every scenario summary (§7's pin): a + // non-zero count would be an abort that did not fire. void CountBarrierPull(MGPipeInputField field, MGPipeVerb verb) { + if (!MGPipeApplierCurrentRecordIsBarriered()) { + StrictBarrierPullFatal(field, verb, "UNBARRIERED, the client did not fill it"); + } ++g_residualPulls; if (MG_Util::PipeStats::Enabled()) { MG_Util::PipeStats::AddCalls(MG_Util::PipeStats::CallClass::ResidualPulls, 1); } - if (MG_Config::Ipc.StrictErrors) StrictBarrierPullFatal(field, verb); + if (MG_Config::Ipc.StrictErrors) { + StrictBarrierPullFatal(field, verb, "MOBILEGL_IPC_STRICT_ERRORS=1"); + } } // The verb's OWN may-read table (FillPoints.def, kMGPipeClassFieldMask). The stamp diff --git a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp index ed94936a..2275a89d 100644 --- a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp +++ b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp @@ -108,7 +108,18 @@ DECLARE_GL_FUNCTION_HEAD(void, DrawArrays, GLenum mode, GLint first, GLsizei cou DECLARE_GL_FUNCTION_HEAD(void, DrawElements, GLenum mode, GLsizei count, GLenum type, const void* indices) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawElements, mode, count, type, indices) DECLARE_GL_FUNCTION_HEAD(void, Enable, GLenum cap) DECLARE_GL_FUNCTION_END_NO_RETURN(void, Enable, cap) DECLARE_GL_FUNCTION_HEAD(void, EnableVertexAttribArray, GLuint index) DECLARE_GL_FUNCTION_END_NO_RETURN(void, EnableVertexAttribArray, index) -MOBILEGL_GL_API void glFinish() { MGLOG_D("Implementing function: %s(...)", __FUNCTION__); } +// P5e (ra), CONTRACT-P5E §2.5. STILL NOTHING ON THE WIRE - Flush and Finish have no record +// (ARCHITECTURE §11) and this stays a no-op on every monolith build. What it is not any more +// is a no-op on a RUN-AHEAD client: "the commands issued so far have completed" is a promise a +// queue the client never waits on can break, and this is the only place an application can ask +// for it. Under lockstep the client had already waited out every command it issued, so the +// call really did have nothing to do; MGPipeClientFinish answers false-cheap there too. +MOBILEGL_GL_API void glFinish() { + MGLOG_D("Implementing function: %s(...)", __FUNCTION__); +#if MOBILEGL_BUILD_DISAGGREGATED + MobileGL::MG_Pipe::MGPipeClientFinish(); +#endif +} MOBILEGL_GL_API void glFlush() { MGLOG_D("Implementing function: %s(...)", __FUNCTION__); } DECLARE_GL_FUNCTION_HEAD(void, FramebufferRenderbuffer, GLenum target, GLenum attachment, GLenum renderbuffertarget, GLuint renderbuffer) DECLARE_GL_FUNCTION_END_NO_RETURN(void, FramebufferRenderbuffer, target, attachment, renderbuffertarget, renderbuffer) DECLARE_GL_FUNCTION_HEAD(void, FramebufferTexture2D, GLenum target, GLenum attachment, GLenum textarget, GLuint texture, GLint level) DECLARE_GL_FUNCTION_END_NO_RETURN(void, FramebufferTexture2D, target, attachment, textarget, texture, level) diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index fe0999ef..34d23700 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -85,6 +85,20 @@ namespace MobileGL::MG_Pipe { return inputs.m_currentVertexAttribute; } +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (ra, CONTRACT-P5E §2.3). THE FOUR O-CLASS ROWS, AND ONLY THEY: these are the only + // members of this block that own a frontend object rather than point into one, so they + // are the only ones through which the apply thread can become a last owner. The raw + // pointer rows (the binding-slot bases) are borrowed from a live GLContext and own + // nothing, so releasing them would buy no lifetime and lose the monolith's arms. + static void ReleaseObjectPins(PipeInputs& inputs) { + inputs.m_boundVertexArray.reset(); + inputs.m_programForDispatch.reset(); + inputs.m_programForDraw.reset(); + inputs.m_transformFeedbackProgram.reset(); + } +#endif + static void CopyField(PipeInputs& dst, GLContext& ctx, MGPipeInputField field) { using F = MGPipeInputField; using MG_State::GLState::BufferBindPointTargets; @@ -330,6 +344,64 @@ namespace MobileGL::MG_Pipe { namespace { GLContext* LiveContext() { return MG_State::pGLContext.get(); } +#if MOBILEGL_BUILD_DISAGGREGATED + // ---- P5e (ra): the fill decision (CONTRACT-P5E §3.1) ----------------------------- + // + // Is this process a run-ahead CLIENT right now? Three questions, cheapest first, and + // the last one is the latch ClientSession took at its first caps adoption - so this is + // one pointer test and one bool load on the verb path once the first two are constant. + Bool ClientRunsAhead() { + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return false; + if (MG_Remote::Client::RunsAsTheServerRole()) return false; + const MG_Remote::Client::ClientSession* session = MG_Remote::Client::ClientSession::Active(); + return session != nullptr && session->RunAheadArmed(); + } + + // THE WIRE OP A VERB BOUNDARY BECOMES, which is the join MGPipeVerbForWireOp draws in + // the other direction. Built by walking the op space once at compile time rather than + // written out, because a second hand-written table is a second thing to forget a row + // in - and the generator already refuses a verb-shaped op with no row. + constexpr MGPWireOp WireOpForVerb(MGPipeVerb verb) { + for (SizeT i = 0; i < static_cast(MGPWireOp::kOpCount); ++i) { + const auto op = static_cast(i); + if (MGPipeVerbForWireOp(op) == verb) return op; + } + return MGPWireOp::kOpCount; + } + + // CONTRACT-P5E §2.1's predicate, ASKED AT THE VALIDATE POINT - which is before the + // record exists, so it is asked of the verb and the live context rather than of the + // payload. The two halves must agree with MGPipeBarriered(op, payload, applierState), + // which is what the server computes, so each clause is the same clause: + // + // 1. the static WaitClass column - the same generated table, same op; + // 2. kCtxVerb inside an open XFB span - ctx.IsTransformFeedbackActive() here, + // the applied MGPContextValues mirror there, and set_context_values precedes the + // verb on the ring, so the two read the same value (§2.1); + // 3. a draw carrying kDrawClientArrays - NOT asked here, because under run-ahead + // such a draw never reaches a record at all: vi refuses it on this very thread in + // EmitDrawRecord's array arm (§5.1, ruling 2). Asking it here would be a second + // spelling of vi's "does any enabled attribute lack a buffer" walk, and two + // spellings of an escalation is precisely what ruling 3 replaced. + // + // AN OP WITH NO VERB ROW ANSWERS BARRIERED. A verb the join does not know is one this + // file cannot reason about, and the safe answer - fill it, wait for it - is also the + // pre-P5e answer. + Bool ClientVerbIsBarriered(MGPipeVerb verb, GLContext& ctx) { + const MGPWireOp op = WireOpForVerb(verb); + if (op == MGPWireOp::kOpCount) return true; + if (MGPipeWaitClassFor(op) != kWaitNone) return true; + if (MGPipeCallClassFor(op) == kCtxVerb && ctx.IsTransformFeedbackActive()) return true; + return false; + } + + // Whether the LAST fill actually happened, i.e. whether the rows in the block describe + // the verb in flight. Read by MGPipeNoteFrontendMutation, which refreshes one field of + // that fill: with no fill behind it there is nothing to refresh and the write would be + // the role violation §3.5 names. + Bool g_lastFillWasBarriered = true; +#endif + template const SharedPtr& NullShared() { static const SharedPtr null; @@ -546,10 +618,18 @@ namespace MobileGL::MG_Pipe { void MGPipeNoteFrontendMutation(MGPipeInputField field) { PipeInputs& inputs = gPipeInputs; #if MOBILEGL_BUILD_DISAGGREGATED + // P5e (ra, CONTRACT-P5E §3.1): THIS REFRESHES ONE FIELD OF THE LAST FILL, so with no + // fill behind it there is nothing to refresh. Under run-ahead the last verb may have + // been unbarriered, in which case the block describes an older verb the server is no + // longer being asked about and a write here would be a GL-thread touch of server-role + // memory. It returns instead - the same answer, one line earlier, as the class-mask + // test below gives for a field the verb never pushed. + if (!g_lastFillWasBarriered) return; // P5c (gt, layer 2): the single-field refresh is a client write into gPipeInputs too - - // same gate as the fill. + // same gate as the fill, and it makes the same claim the fill made: the rows it is + // touching belong to a record this thread is parked behind (or will park behind). MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( - "MGPipeNoteFrontendMutation"); + "MGPipeNoteFrontendMutation", /*isBarrieredFill=*/true); #endif auto* ctx = LiveContext(); if (ctx == nullptr) return; @@ -999,6 +1079,29 @@ namespace MobileGL::MG_Pipe { Bool MGPipeDeferDestroyAndFreeIfOnApplyThread(MGPipeKind kind, Uint64 lifetimeId) { if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return false; if (!MG_Remote::Client::RunsAsTheServerRole()) return false; + // P5e (ra), CONTRACT-P5E §2.7 / ruling 13. The queue stays - a BARRIERED record's apply + // may still pin a frontend object (its fill's O-class rows, XFB's targets), so the + // apply thread can still be a last owner and this is still the belt that keeps that + // death off the client's allocator. But an enqueue from an UNBARRIERED record is a + // FINDING, not a service: rule F says such an apply names no client memory at all, so + // a SharedPtr it could be the last owner of means some site is still pinning a + // frontend object across a record and the migration this phase believes it finished is + // not finished. Named once with the kind so the site is findable, and Fatal under + // strict so the lane owns the red rather than a log nobody reads. + if (!MG_Pipe::MGPipeApplierCurrentRecordIsBarriered()) { + if (MG_Config::Ipc.StrictErrors) { + MGLOG_F("MGPipe: Fatal{RoleViolation, \"deferred-destroy\"} - an UNBARRIERED " + "apply was the last owner of a frontend object of kind %u (lifetime " + "%llu). CONTRACT-P5E rule F says an unbarriered apply reads no client " + "memory, so nothing it touched should have been a SharedPtr at all", + static_cast(kind), static_cast(lifetimeId)); + std::abort(); + } + MGLOG_E_ONCE("MGPipe: an UNBARRIERED apply deferred the destruction of a frontend " + "object of kind %u - CONTRACT-P5E §2.7's finding: some apply-thread " + "site still holds a frontend SharedPtr across a record", + static_cast(kind)); + } { std::lock_guard lock(g_deferredDestroyMutex); g_deferredDestroys.push_back(MGPipeDeferredDestroy{kind, lifetimeId}); @@ -1007,6 +1110,30 @@ namespace MobileGL::MG_Pipe { return true; } + // P5e (ra), CONTRACT-P5E §2.3. Declared in PipeMutation.h, where its WHY is argued. + // + // NO GUARD ON THE CALLER'S BEHALF: ClientSession calls this only after a barriered apply + // has returned on a run-ahead session, and adding a second "is run-ahead armed" test here + // would be a copy of a decision that belongs on the other side. What this side owns is + // WHICH rows go, and that answer is ReleaseObjectPins' four. + // + // THE STAMPS ARE LEFT ALONE, deliberately. A released row reads back as null, and a + // BARRIERED verb's fill re-copies it before that verb's apply can pull it (§3.1 skips the + // fill only for UNBARRIERED records); an unbarriered apply may not read it at all, and + // §3.3's detector is what says so by name. Clearing the stamps as well would trade that + // named abort for the poison Fatal, which names the field but not the rule it broke. + void MGPipeReleaseResidualFillPins() { MGPipeFillAccess::ReleaseObjectPins(gPipeInputs); } + + // P5e (ra), CONTRACT-P5E §2.5. Declared in PipeMutation.h; see there for why it is not a + // ClientSession call at the GL entry point. + void MGPipeClientFinish() { + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; + if (MG_Remote::Client::RunsAsTheServerRole()) return; + if (MG_Remote::Client::ClientSession* session = MG_Remote::Client::ClientSession::Active()) { + session->Finish(); + } + } + void MGPipeDrainDeferredDestroys() { if (g_deferredDestroyCount.load(std::memory_order_acquire) == 0) return; Vector drained; @@ -1955,9 +2082,20 @@ namespace MobileGL::MG_Pipe { void MGPipeLeaveVerb() { PipeInputs& inputs = gPipeInputs; #if MOBILEGL_BUILD_DISAGGREGATED - // Same layer-2 gate as the fill: the serial bump and the verb reset below are writes - // into gPipeInputs (gt). - MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt("MGPipeLeaveVerb"); + // P5e (ra, §3.1): a verb whose fill was skipped has no stamps of this thread's to + // retire, and the bump below would move a serial the SERVER's stamp owns. Leave + // returns, and the tracker's base-instance clear - which is frontend state, not block + // state - still runs at the bottom. + if (g_lastFillWasBarriered) { + // Same layer-2 gate as the fill: the serial bump and the verb reset below are + // writes into gPipeInputs (gt). + MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( + "MGPipeLeaveVerb", /*isBarrieredFill=*/true); + } + if (!g_lastFillWasBarriered) { + MGPipeTrackerInstance().ClearPendingBaseInstance(); + return; + } #endif #if MOBILEGL_PIPE_POISON // Same bump the next fill would make, without a verb to fill from: no field is @@ -2686,6 +2824,18 @@ namespace MobileGL::MG_Pipe { // Did the applier reproduce it? Byte for byte, over the attributes this call // named - anything less would be a mirror that disagrees with the frontend in a // window no gate looks at. + // + // P5e (ra, CONTRACT-P5E §3.4): NOT UNDER RUN-AHEAD. The mirror is the APPLIER's + // copy of this record, and set_vertex_attrib_defaults is a kWaitNone row - so + // under run-ahead this thread has not waited for the apply and the read races it, + // and the repair below (a CopyField straight into the block) is precisely the + // GL-thread write §3.5 refuses. The client's own authority is `resolved` and + // `g_attribDefaultLastHeader`, which is what it just published; there is nothing + // the mirror could add that the wire does not already carry. A build that ever + // needs the repair arm again has to earn it with a barriered row. +#if MOBILEGL_BUILD_DISAGGREGATED + if (ClientRunsAhead()) return sizeof(MGPVertexAttribDefaults) + header.Count * sizeof(MGPAttribValue); +#endif const auto* mirror = MGPipeFillAccess::VertexAttribDefaultsOf(gPipeInputs); Bool reproduced = true; for (SizeT i = 0; i < kAttribs && reproduced; ++i) { @@ -3004,13 +3154,20 @@ namespace MobileGL::MG_Pipe { void MGPipeValidateForVerb(MGPipeVerb verb) { PipeInputs& inputs = gPipeInputs; #if MOBILEGL_BUILD_DISAGGREGATED - // P5c (gt, CONTRACT-P5C §6 layer 2): the residual fill is THE client-side write into - // gPipeInputs, and it is legal only because it runs before the record is published - - // under an armed barrier the apply thread's in-applier flag is provably down here. If - // that ever stops being true this is the check that says so, rather than the applier - // reading a half-written fill. - MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( - "MGPipeValidateForVerb"); + // ---- P5e (ra), CONTRACT-P5E §3: WHO OWNS gPipeInputs FOR THIS VERB ---------------- + // + // Under run-ahead the block is SERVER-ROLE MEMORY for an unbarriered record: the + // client does not fill it, does not stamp it and does not withdraw the server's stamp, + // because it will not be parked while the apply runs and every one of those writes + // would race the applier's own reads. What still runs, unchanged, is the tracker walk + // and the emitters (steps 2 and 3): those PRODUCE RECORDS, which is the whole of what + // an unbarriered verb is allowed to hand the server. + // + // `fillOwed` is deliberately a separate name from `barriered`, and that is what makes + // the red-once one line: setting it to `true` restores the old behaviour (a fill for + // every verb) and the guard below then fires Fatal{RoleViolation, "gPipeInputs"} on + // the first unbarriered verb, by name. + const Bool runAhead = ClientRunsAhead(); #endif ParsePoisonOmissionKnob(); #if MOBILEGL_PIPE_VERIFY @@ -3023,24 +3180,64 @@ namespace MobileGL::MG_Pipe { "(configure with -DMOBILEGL_PIPE_VERIFY=ON)"); } #endif + auto* ctx = LiveContext(); + // THE IDENTITY / LIVENESS PAIR IS WRITTEN ON EVERY VERB, run-ahead or not, and that is + // a NAMED DEVIATION from CONTRACT-P5E §3.1's list (see the report): `m_live` and + // `m_contextIdentity` are not FIELDS of the residual model - no FieldOwnership.def row + // owns them, no applier record carries them, and no server stamp can answer them - + // they are "does this process still have a GL context", which every null-context guard + // in the backend reads and which only this thread can know. Two stores, no freshness + // and no stamp, so an unbarriered apply reading them reads a fact about the client's + // process rather than a value the wire owes it. + MGPipeFillAccess::SetIdentity(inputs, ctx); +#if MOBILEGL_BUILD_DISAGGREGATED + // §2.1's predicate, client-side (see ClientVerbIsBarriered). A null context makes the + // verb a no-op below anyway, so it answers barriered and the old path runs whole. + const Bool barriered = !runAhead || ctx == nullptr || ClientVerbIsBarriered(verb, *ctx); + // THE ONE LINE THE RED-ONCE FLIPS: `= true` here is "fill for every verb", the + // pre-P5e behaviour, and it turns the guard below into the abort §3.5 names. + const Bool fillOwed = barriered; + g_lastFillWasBarriered = fillOwed; + if (fillOwed) { + // P5c (gt, CONTRACT-P5C §6 layer 2) / P5e §3.5: the residual fill is THE + // client-side write into gPipeInputs. Under lockstep it is legal because it runs + // before the record is published - the apply thread's in-applier flag is provably + // down here. Under run-ahead it is legal because this record is BARRIERED: this + // thread is about to park behind it. The second argument is which of the two + // claims the caller is making, and a fill that made the second one falsely is the + // named abort. + MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( + "MGPipeValidateForVerb", /*isBarrieredFill=*/barriered); + } +#else + constexpr Bool fillOwed = true; +#endif + if (fillOwed) { #if MOBILEGL_PIPE_POISON - MGPipeFilledState& filled = MGPipeFillAccess::Filled(inputs); - // Starts at 1: FilledGen == 0 is "never filled", and MGPipeInputFieldIsFresh refuses - // it on both branches, so a read before this first bump is - // Fatal{UnmigratedPipeInput, "@"} rather than default storage. - ++filled.CurrentVerbSerial; + // Starts at 1: FilledGen == 0 is "never filled", and MGPipeInputFieldIsFresh + // refuses it on both branches, so a read before this first bump is + // Fatal{UnmigratedPipeInput, "@"} rather than default storage. + ++MGPipeFillAccess::Filled(inputs).CurrentVerbSerial; #endif #if MOBILEGL_BUILD_DISAGGREGATED - // The client is filling, so whatever the server stamped at its last verb boundary is - // withdrawn: the stamps below are the CLIENT's again and a stale read is a defect, not - // a residual pull. Disarming here rather than at the end of the applier's work is what - // makes the arming flag say "the current stamps are the server's" no matter which of - // the two roles ran last. - MGPipeServerClearVerbBoundary(); + // The client is filling, so whatever the server stamped at its last verb boundary + // is withdrawn: the stamps below are the CLIENT's again and a stale read is a + // defect, not a residual pull. Disarming here rather than at the end of the + // applier's work is what makes the arming flag say "the current stamps are the + // server's" no matter which of the two roles ran last. + // + // P5e (§3.2): under run-ahead this runs only inside a barriered fill, which is the + // whole of the E note's hazard (b.4) - a GL thread withdrawing the SERVER's stamp + // while the apply thread is inside a record that depends on it. For an unbarriered + // verb the stamp is not touched at all, and the applier's own LeaveApplier is then + // its only writer. + MGPipeServerClearVerbBoundary(); +#endif + MGPipeFillAccess::SetVerb(inputs, verb); + } +#if MOBILEGL_PIPE_POISON + MGPipeFilledState& filled = MGPipeFillAccess::Filled(inputs); #endif - MGPipeFillAccess::SetVerb(inputs, verb); - auto* ctx = LiveContext(); - MGPipeFillAccess::SetIdentity(inputs, ctx); if (ctx == nullptr) { // The pending base instance belongs to THIS verb, and this exit skips step 3's // clear, so it has to make the same promise here: a base-instanced draw with no @@ -3286,12 +3483,20 @@ namespace MobileGL::MG_Pipe { // does read. The two halves of P5c rv's gate still read the ONE `contextValuesWireLive` // computed at the top of this function - it is the memo's key AND its argument - so they // cannot disagree about who supplies the eight value-class fields. + // + // P5e (ra, §3.1): AND IT DOES NOT RUN AT ALL FOR AN UNBARRIERED RECORD. Every write + // below - the CopyField and the FilledGen stamp alike - is a GL-thread write into a + // block the apply thread is about to read without this thread being parked, which is + // exactly what rule F forbids. The server does not go without an answer: it stamps its + // own boundary and §3.3's detector turns any field it still needs from here into + // Fatal{UnmigratedPipeInput, "@"} by name, which is what makes the strict + // lane a gate rather than a count. const Bool applierDerives = ApplierDerivesRenderStateFields(); // BY VALUE, NOT BY REFERENCE: the memo's storage is rebuilt in place when the key moves, // and the walk below holds this across 63 iterations. const MGPipeFieldMask supplied = SuppliedFieldMask(pushMask, applierDerives, contextValuesWireLive); - for (SizeT i = 0; i < kMGPipeInputFieldCount; ++i) { + for (SizeT i = 0; fillOwed && i < kMGPipeInputFieldCount; ++i) { const auto field = static_cast(i); if (!MGPipeFieldMaskHas(mask, field)) continue; #if MOBILEGL_PIPE_POISON diff --git a/MobileGL/MG_Pipe/PipeMutation.h b/MobileGL/MG_Pipe/PipeMutation.h index d367a4bf..c9f6fb25 100644 --- a/MobileGL/MG_Pipe/PipeMutation.h +++ b/MobileGL/MG_Pipe/PipeMutation.h @@ -231,8 +231,38 @@ namespace MobileGL::MG_Pipe { // BeforeReadOnlyVerb) through MGPipeDrainDeferredDestroys. A delayed free is safe: // slots are plentiful, reuse is delayed rather than corrupted, and the replay runs the // helper itself, so every helper's own three-step order is untouched. + // P5e (ra), CONTRACT-P5E §2.7 / ruling 13 (ID-91): THE QUEUE STAYS LIVE, AND AN + // UNBARRIERED ENQUEUE IS A FINDING. After the family packages land, an unbarriered apply + // holds no frontend SharedPtr at all - every object is resolved from a handle - so the + // only apply thread that can still be a last owner is one inside a BARRIERED record (a + // fill's O-class rows, XFB's pinned targets). An enqueue from an unbarriered record + // therefore means some site still pins a frontend object across a record, which is a + // migration this phase believes it finished: it logs once with the kind, and is Fatal + // under MOBILEGL_IPC_STRICT_ERRORS so the strict lane owns the red. The queue itself is + // NOT removed - it is the belt that keeps the barriered case from corrupting the + // allocator while the finding is being read. Bool MGPipeDeferDestroyAndFreeIfOnApplyThread(MGPipeKind kind, Uint64 lifetimeId); void MGPipeDrainDeferredDestroys(); + + // P5e (ra), CONTRACT-P5E §2.3: drop the four O-class SharedPtr rows the residual fill left + // in gPipeInputs (the bound VAO and the three programs, PipeInputs.h's O block). Called by + // ClientSession on the GL thread once a BARRIERED apply has returned - the one instant + // under run-ahead at which the applier is provably idle and this block has no other + // reader. Under lockstep nothing calls it: the next verb's fill overwrites those rows + // within a verb, and dropping them early would only cost a re-pin. + // + // It is HERE rather than in ClientSession because the rows are PipeInputs' private storage + // and MGPipeFillAccess - the client-side door to it - lives in MG_Impl (PipeFill.cpp). + // MG_Remote says when; MG_Impl says what. + void MGPipeReleaseResidualFillPins(); + + // P5e (ra), CONTRACT-P5E §2.5: glFinish's whole body under a transport. Declared here + // rather than on ClientSession so that MG_Impl's GL entry points - which are BELOW + // MG_Remote - can reach it without learning that a session exists; the implementation in + // PipeFill.cpp is one forward to ClientSession::Finish. A no-op with no live session, on a + // monolith transport, and on a lockstep one (there the client waited out every command it + // issued before it could reach the call). + void MGPipeClientFinish(); #endif void MGPipeEmitResourceCreate(MG_State::GLState::BufferObject& buffer); diff --git a/MobileGL/MG_Pipe/PipeRoute.h b/MobileGL/MG_Pipe/PipeRoute.h index 7e794bf9..63f4b0d5 100644 --- a/MobileGL/MG_Pipe/PipeRoute.h +++ b/MobileGL/MG_Pipe/PipeRoute.h @@ -267,12 +267,41 @@ namespace MobileGL::MG_Pipe { } // ---- resources --------------------------------------------------------------------- + + // P5e (ra), CONTRACT-P5E §2.5 / ruling 15 (ID-93): DOES THIS SUB-DATA RECORD WANT ITS + // ANSWER? One predicate, two callers - this route and the wire emit table - because the + // route cannot pass a flag through the thunk (the call table's signature is the + // catalogue's) and two hand-written copies of "is this the buffer half" is exactly the + // drift that would make the client wait for an answer the emitter told the server not to + // bother with, or read a slot that was never posted. + // + // THE BUFFER HALF DOES NOT: its Bool is discarded at the only call site there is + // (MGPipeEmitResourceSubData), and because the row carries kReplySlot the client used to + // wait for it anyway - one full round trip per 64 KB persistent-map block, whose answer + // nobody looked at. THE TEXTURE HALF DOES: DrainTextureSubData clears the level's dirty + // flag on an ACCEPTED reply, so its answer is load-bearing (D-D5; making that half + // fire-and-forget is a trailing item, not this package's). + // + // The test is MGPSubData::Target == kMGPipeResourceTargetBuffer, whole field, which is the + // invariant MGPipeTypes.h asserts beside the packer and which the applier's own + // SubDataNamesABuffer already reads. + inline Bool MGPipeSubDataWantsItsReply(const MGPSubData& record) { + return record.Target != kMGPipeResourceTargetBuffer; + } + inline Bool MGPipeRouteResourceSubData(const MGPSubData& record, const void* bytes, Uint64 byteCount, const MGPSubRegion* regions = nullptr) { + // THE SLOT IS STILL MINTED AND STILL TAKEN, even for the buffer half. The row keeps + // kReplySlot on the wire, the server still posts, and the mailbox's "every minted slot + // is taken" rule is what the next row's ReplyOverrun Fatal is looking for - so the + // half that does not WAIT still has to collect. What changes is that under run-ahead + // the emit table publishes and returns, and the answer it collects is the emitter's + // accept-by-construction rather than the server's. MGPReplySlot reply = MGPipeMintReplySlot(); MGP_ResourceSubData(&record, bytes, byteCount, regions, record.RegionCount, &reply); - return MGPipeTakeReplyBool(reply, "resource_subdata"); + const Bool accepted = MGPipeTakeReplyBool(reply, "resource_subdata"); + return MGPipeSubDataWantsItsReply(record) ? accepted : true; } inline void MGPipeRouteBufferSubDataResident(const MGPSubData& record, const void* bytes, Uint64 byteCount) { diff --git a/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp b/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp index bf1193e0..c7a739aa 100644 --- a/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp +++ b/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp @@ -172,6 +172,27 @@ namespace MobileGL::MG_Remote::Client { return CapsMirrorInstance().Backend(); } + namespace { + // P5e (ra), CONTRACT-P5E §2.5: THE WAIT BEFORE EVERY `Server*` EGL FORWARDER. + // + // These calls do not travel on SEG_CMD. They go through ServerLoop's control mailbox, + // which is pumped BETWEEN drain batches (ServerLoop.cpp's PumpControlRequest, above + // DrainRing) - so a make-current, a surface creation or a resize can land between two + // records the client published and never waited for. Under lockstep that was + // impossible: the client had waited out every record it issued before it could reach + // this line. Under run-ahead it is one queued frame wide, and a context switch applied + // in the middle of another context's draws is not a wrong pixel, it is a wrong + // everything. + // + // ServerSwapEGLBuffers is deliberately NOT on this list, and its own virtual says why: + // present IS the swap and it travels as a record, in order, with its own credit. + void WaitForApplyBeforeEglForwarder(const char* forwarder) { + if (ClientSession* session = ClientSession::Active()) { + session->WaitForApplyToCatchUp(forwarder); + } + } + } // namespace + // ---- the nine EGL lifecycle virtuals --------------------------------------------------- // // FORWARD FIRST, THEN RUN THE BASE. The server has to own the context before the client's @@ -180,6 +201,7 @@ namespace MobileGL::MG_Remote::Client { // InitCapabilities has run. Bool BackendObject_Remote::InitializeEGLDisplay(EGLDisplay dpy, EGLint* major, EGLint* minor) { + WaitForApplyBeforeEglForwarder("InitializeEGLDisplay"); if (!Server::ServerInitializeEGLDisplay(dpy, major, minor)) return false; return MG_Backend::BackendObject::InitializeEGLDisplay(dpy, major, minor); } @@ -188,6 +210,7 @@ namespace MobileGL::MG_Remote::Client { const MG_Backend::WindowHandle& handle) { // The handle first: the server's backend has to know which window it is about to make // a surface for, and ServerSetWindowHandle is the only way to tell it. + WaitForApplyBeforeEglForwarder("CreateEGLWindowSurface"); Server::ServerSetWindowHandle(handle); if (!Server::ServerCreateEGLWindowSurface(surface, handle)) return false; // The server's surface init published the default framebuffer's shape as a @@ -202,6 +225,7 @@ namespace MobileGL::MG_Remote::Client { } Bool BackendObject_Remote::ResizeEGLWindowSurface(EGLSurface surface, Uint32 width, Uint32 height) { + WaitForApplyBeforeEglForwarder("ResizeEGLWindowSurface"); if (!Server::ServerResizeEGLWindowSurface(surface, width, height)) return false; // A resize re-creates the server's swapchain, which re-posts the surface-changed // event - same drain, same reason as CreateEGLWindowSurface. @@ -210,6 +234,7 @@ namespace MobileGL::MG_Remote::Client { } Bool BackendObject_Remote::CreateEGLPbufferSurface(EGLSurface surface, EGLint width, EGLint height) { + WaitForApplyBeforeEglForwarder("CreateEGLPbufferSurface"); if (!Server::ServerCreateEGLPbufferSurface(surface, width, height)) return false; // Same drain as the window surface: InitPbufferSurface publishes the default // framebuffer's depth/stencil format on SEG_EVENT from inside this very RPC. @@ -223,6 +248,7 @@ namespace MobileGL::MG_Remote::Client { // class latches "the surface is initialised" and calls InitCapabilities, because // InitCapabilities' answer comes from a snapshot the server can only publish once its // own InitCapabilities has run - and ServerMakeEGLCurrent is what publishes it. + WaitForApplyBeforeEglForwarder("MakeEGLCurrent"); if (!Server::ServerMakeEGLCurrent(dpy, draw, read, ctx)) return false; if (!MG_Backend::BackendObject::MakeEGLCurrent(dpy, draw, read, ctx)) return false; diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index bd749387..b1332132 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -139,7 +139,12 @@ namespace MobileGL::MG_Remote::Client { return timeoutNs / 1000000 + (timeoutNs % 1000000 != 0) + kBarrierTimeoutMs; } + // Defined below; declared here because the applied wait has to be able to drain + // SEG_EVENT while it is parked (P5e §2.6). + Uint32 DrainEventRing(Transport::EventRingConsumer& events); + Transport::SessionWait WaitForAppliedBudget(Transport::SessionProducer& producer, + Transport::EventRingConsumer& events, Uint64 seq, Uint64 remainingMs) { // The transport takes Uint32 milliseconds with UINT32_MAX meaning forever. // Keep even the largest GL timeout finite, using bounded chunks and preserving @@ -148,7 +153,23 @@ namespace MobileGL::MG_Remote::Client { for (;;) { const Uint32 chunkMs = remainingMs > kMaxFiniteWaitMs ? kMaxFiniteWaitMs : static_cast(remainingMs); - const auto wait = producer.WaitForApplied(seq, chunkMs); + // P5e (ra, CONTRACT-P5E §2.6): THE PARK BREAKS ON TWO THINGS. A server that + // ran out of SEG_EVENT stops producing and therefore stops applying, so a + // waiter that could only be woken by appliedSeq would wait for a watermark + // nothing is going to move - and it is holding the only drain there is. The + // second exit is that case: drain, clear the latch, ring the server, park + // again. `remainingMs` is deliberately NOT charged for it; a wake that did + // the queue's work is not the caller's budget being spent. + const auto wait = producer.WaitForAppliedOrEventBacklog(seq, chunkMs); + if (wait == Transport::SessionWait::Reached) { + Transport::RingControl* control = producer.Control(); + if (control == nullptr || + Transport::Watermark::Reached(control->appliedSeq, seq)) { + return wait; + } + DrainEventRing(events); + continue; + } if (wait != Transport::SessionWait::TimedOut || remainingMs <= chunkMs) return wait; remainingMs -= chunkMs; } @@ -592,6 +613,9 @@ namespace MobileGL::MG_Remote::Client { m_events = Transport::EventRingConsumer(m_shm.EventControl(), control, m_shm.EventRingBase(), m_shm.EventRingCapacity(), m_shm.EventSegmentBase()); + // P5e (ra, §2.6): the drain's other half. A server parked on a full SEG_EVENT is woken + // by the drain that emptied it, not by the next thing this client happens to publish. + m_events.SetServerDoorbell(&m_clientTransport->PeerDoorbell()); if (!m_replies.Valid() || !m_events.Valid()) { Stop(); return MOBILEGL_ERR_INVALID_ARGUMENT; @@ -629,6 +653,10 @@ namespace MobileGL::MG_Remote::Client { // half-wired shape this session exists not to have. m_encoder = Wire::PipeWireEncoder(control, &m_cmd, nullptr, &m_segments); m_encoder.SetStageRetirementDoorbell(m_producer.SelfDoorbell()); + // P5e (ra, §2.6): the stage bell is a client wait and every client wait drains. The + // encoder owns the park; the session owns SEG_EVENT, so the session lends it a drain. + m_encoder.SetStageWaitHook( + +[](void* self) { DrainEventRing(static_cast(self)->Events()); }, this); // ---- 7. the first CapsSnapshot, if the server had a backend to publish one from. // ONE DRAIN, ONE ADOPTER (c1): PumpControlPlane below is the only thing in the client @@ -814,7 +842,8 @@ namespace MobileGL::MG_Remote::Client { Uint64 ClientSession::EmitAndWaitTails(MG_Pipe::MGPWireOp op, const void* payload, Uint64 payloadBytes, const Wire::WireTail* tails, Uint32 tailCount, void* replyOut, Uint64 replyBytes, - Int32* statusOut, Uint64* replySizeOut) { + Int32* statusOut, Uint64* replySizeOut, + Bool wantReply) { Uint64 varTailBytes = 0; for (Uint32 i = 0; i < tailCount; ++i) varTailBytes += tails[i].Size; if (statusOut != nullptr) *statusOut = Wire::ReplySink::kStatusError; @@ -863,6 +892,12 @@ namespace MobileGL::MG_Remote::Client { wait == Transport::SessionWait::ShutDown ? "shutdown" : "timeout"); std::abort(); } + // P5e (ra, §2.6): EVERY WAIT EXIT DRAINS. The cmd-space wait is one of the two + // safety nets under run-ahead (the stage bell is the other), and a client parked + // here without draining is the other half of the flow-control deadlock: the server + // stops applying on a full event ring, so retiredSeq stops moving, so this wait + // never completes. One drain on the way out closes it. + DrainEventRing(m_events); seq = m_encoder.EncodeRecord(op, payload, payloadBytes, tails, tailCount); if (seq == Wire::kInvalidSeq) { MGLOG_F("MGPipe: Fatal{RingOverrun, \"SEG_CMD\"} - %s refused after sufficient " @@ -880,8 +915,63 @@ namespace MobileGL::MG_Remote::Client { // (R-16 / ID-31) - fourteen rows now, because the four Bool acceptance entry points // gained the flag. Asking the catalogue rather than the caller is what stops a caller // that forgot to pass a buffer from silently turning an answer into a guess. - const Bool ownsReplySlot = + // + // P5e (ra) ADDS ONE CALLER-SIDE CONJUNCT AND NO SECOND SOURCE OF TRUTH: `wantReply`. + // The catalogue still says which rows CARRY a slot; a caller that passes false says it + // will not read this one (resource_subdata's buffer half, §2.5 - the only such caller). + // The server posts either way, so the wire is unchanged and the row keeps its flag. + const Bool rowCarriesReplySlot = (MG_Pipe::MGPipeCallFlagsFor(op) & static_cast(MG_Pipe::kReplySlot)) != 0; + const Bool ownsReplySlot = rowCarriesReplySlot && wantReply; + + // ---- THE P5e WAIT RULE (CONTRACT-P5E §2.3) --------------------------------------- + // + // IT STANDS BEFORE THE BATCH SKIP AND REPLACES IT WHOLE, rather than adding a clause to + // it. The batch skip is a rule about CALL CLASSES ("a set/CSO/object record carries its + // whole value in the payload"); the wait class is a rule about WHAT AN APPLY READS, and + // it is generated from the same PipeCalls.def row the class comes from + // (MGPipeWaitClassFor, c0e). Two rules layered would mean a row could be skipped by the + // class rule and waited by the column, and the column is the one the SERVER's + // MGPipeBarriered reads - so the two halves of the wire would disagree about whether + // the client is parked behind this record, which is the single fact §4.4's exemptions + // and §3.3's detector both rest on. + // + // WITH RUN-AHEAD DISARMED NOTHING BELOW RUNS and the file's original shape continues - + // Magma, a monolith transport, MOBILEGL_IPC_RUN_AHEAD=0 and every build before the + // integration commit take the old path byte for byte, which is what makes RUN_AHEAD=0 + // a pure wait-rule A/B on identical server code (§5.8). + if (RunAheadArmed() && !ownsReplySlot) { + MG_Pipe::MGPipeWaitClass waitClass = MG_Pipe::MGPipeWaitClassFor(op); + // §2.5's named relaxation, and the ONLY place a static column answer is overridden: + // a kWaitReply row whose caller does not want the answer has nothing to wait FOR. + // It is spelled here rather than in the column because the column is per-op and + // this is per-call - the texture half of the same row still waits. + if (!wantReply && waitClass == MG_Pipe::kWaitReply) waitClass = MG_Pipe::kWaitNone; + switch (waitClass) { + case MG_Pipe::kWaitNone: + // Published and gone. Rule F is what makes this legal: the apply of an + // unbarriered record names no client memory at all (§3, §4, §5). + return seq; + case MG_Pipe::kWaitPresent: + // The credit wait already ran, in EmitPresent, BEFORE the encode (§2.4). There + // is nothing left to do here and waiting would turn the credit into a barrier. + return seq; + case MG_Pipe::kWaitApplied: + // §2.2's barriered rows: applier_reset, generate_mipmap (until tx2 lands its + // VerbMipRes arm), set_storage_block_binding, the five XFB span verbs and the + // CopyTex endpoint. Each still pulls a row or probes a registry, and the + // client's wait is exactly what keeps P5C's semantics legal for it (ruling 4). + // Falls through to the wait below. + break; + case MG_Pipe::kWaitReply: + case MG_Pipe::kWaitClassCount: + // Unreachable: `ownsReplySlot` is false here, and gen_pipe refuses a kWaitReply + // row without a slot and a kReplySlot row that does not wait (c0e's three + // negative controls). Treated as a wait rather than as a skip, because the + // wrong answer to "does this hang" is the recoverable one. + break; + } + } // MOBILEGL_IPC_BATCH_WAITS (default 1): a value-class record is published WITHOUT // waiting for its own apply. The barrier's one load-bearing reader is the backend @@ -912,7 +1002,13 @@ namespace MobileGL::MG_Remote::Client { // (set_texture_params, resource_subdata, resource_readback and get_texture_image // own reply slots and wait anyway; the resource_* transfers and object_death resolve // by handle). - if (!ownsReplySlot && MG_Config::Ipc.BatchWaits != 0 && + // + // AND IT IS DISARMED WHOLE UNDER RUN-AHEAD (P5e §2.3): the wait class above is the + // rule there, and a second skip rule reading a different column is how the two halves + // of the wire end up disagreeing about whether the client is parked. Written as a + // conjunct here rather than left to "the classes happen not to overlap", because the + // classes overlapping is one PipeCalls.def row away at any time. + if (!ownsReplySlot && !RunAheadArmed() && MG_Config::Ipc.BatchWaits != 0 && op != MG_Pipe::MGPWireOp::GenerateMipmap) { const MG_Pipe::MGPipeCallClass callClass = MG_Pipe::MGPipeCallClassFor(op); if (callClass == MG_Pipe::kCtxState || callClass == MG_Pipe::kCtxCso || @@ -931,7 +1027,8 @@ namespace MobileGL::MG_Remote::Client { const BarrierWaitScope waiting; const Uint64 waitBudgetMs = AppliedWaitBudgetMs(op, payload); - const Transport::SessionWait wait = WaitForAppliedBudget(m_producer, seq, waitBudgetMs); + const Transport::SessionWait wait = + WaitForAppliedBudget(m_producer, m_events, seq, waitBudgetMs); if (wait == Transport::SessionWait::ShutDown) { // The doorbell died: the server went away. The only thing that returns from a // kWaitForever park, and therefore the only way a client blocked in the barrier @@ -965,7 +1062,17 @@ namespace MobileGL::MG_Remote::Client { // clears when the writeback lands, and the writeback lands on this ring. DrainEventRing(m_events); - if (!ownsReplySlot) return seq; + if (!ownsReplySlot) { + // P5e (ra, §2.3). A barriered record's apply has returned and this thread is the + // only runnable one, so the four O-class SharedPtr rows the last fill left in + // gPipeInputs can go. Under lockstep they were harmless - the next verb's fill + // overwrote them - but under run-ahead the next fill may be a frame away, and a + // frontend VAO or program whose LAST owner is that block is one the apply thread + // can end up destroying (§2.7's deferred-destroy hazard). Released here rather + // than at the fill, because here is where the applier is provably idle. + if (RunAheadArmed()) ReleaseFillPins(); + return seq; + } // THE SAME WAIT, NOT A SECOND ONE. appliedSeq >= seq already means the server wrote // this record's answer, because it writes the slot before it advances the watermark. @@ -1000,6 +1107,166 @@ namespace MobileGL::MG_Remote::Client { Bool ClientSession::BarrierArmed() const { return m_barrierArmed; } + // ---- P5e (ra): the run-ahead latch and the waits it leaves behind --------------------- + + Bool ClientSession::RunAheadArmed() const { return m_runAheadArmed; } + // THE PRODUCER'S NUMBER, NOT A SECOND ONE. SessionProducer records it inside + // PublishAndNotify, which is the only place a record becomes published at all; a copy kept + // here would be a second thing to forget to write on a path that publishes without waiting + // - which, after P5e, is most of them. + Uint64 ClientSession::LastPublishedSeq() const { return m_producer.LastPublishedSeq(); } + Uint64 ClientSession::PresentCreditWaits() const { return m_presentCreditWaits; } + + void ClientSession::LatchRunAheadFromCaps() { +#if MOBILEGL_BUILD_DISAGGREGATED + // THE CONJUNCTION, IN THE CONTRACT'S OWN ORDER (§1). The caps read is last because it + // is the only one that costs a mirror load, and because it is the only one whose answer + // can change: the other two are process configuration. + const Bool armed = m_barrierArmed && MG_Config::Ipc.RunAhead != 0 && + Caps().HasCap(MG_Pipe::kCapRunAheadApply); + if (!m_runAheadLatched) { + m_runAheadLatched = true; + m_runAheadArmed = armed; + if (armed) { + MGLOG_I("MG_Remote client: run-ahead ARMED - the server publishes " + "kCapRunAheadApply, so an unbarriered record is published and not " + "waited for (CONTRACT-P5E §2.3), paced by a present credit of %u", + MG_Config::Ipc.PresentCredit); + } else if (MG_Config::Ipc.RunAhead != 0 && m_barrierArmed) { + // THE KNOB IS NOT A SWITCH ON ITS OWN, and an operator who set it has to be + // told that, or an A/B arm that measured nothing looks like an A/B arm that + // measured no difference (Config.h's own argument for parsing it everywhere). + MGLOG_W_ONCE("MG_Remote client: run-ahead requested, server does not publish " + "kCapRunAheadApply - running lockstep. This is Magma always, and " + "Espryt until the P5e integration commit flips " + "kMGPipeP5eRunAheadReady"); + } + return; + } + // A LATER SNAPSHOT MAY ONLY TURN IT OFF. Promotion would mean the records already + // published without a wait were published against a rule this server never agreed to, + // and nothing can un-publish them; demotion only adds waits, so it is safe and is + // exactly what a server that lost its run-ahead backend needs. + if (!armed && m_runAheadArmed) { + m_runAheadArmed = false; + MGLOG_W("MG_Remote client: run-ahead DISARMED by a later caps snapshot - every " + "record from here waits as it did before P5e"); + } +#endif + } + + void ClientSession::ReleaseFillPins() { +#if MOBILEGL_BUILD_DISAGGREGATED + // The block's own door: PipeInputs' O-class rows are private and MG_Remote has no + // business knowing which four they are. MG_Impl owns the fill, so MG_Impl owns the + // un-fill (PipeFill.cpp), and this side only says WHEN. + MG_Pipe::MGPipeReleaseResidualFillPins(); +#endif + } + + void ClientSession::WaitForApplyToCatchUp(const char* why) { +#if MOBILEGL_BUILD_DISAGGREGATED + // A NO-OP WITHOUT RUN-AHEAD, and that is not an optimisation: under lockstep the + // client already waited for every record it published, so appliedSeq is at + // LastPublishedSeq by construction and a second wait would only add a park on a + // watermark that is already reached. + if (!m_runAheadArmed || !m_started) return; + const Uint64 target = m_producer.LastPublishedSeq(); + if (target == 0) return; + const BarrierWaitScope waiting; + const Transport::SessionWait wait = + WaitForAppliedBudget(m_producer, m_events, target, kBarrierTimeoutMs); + if (wait == Transport::SessionWait::ShutDown) { + // The doorbell died: teardown, not a fault - the same answer EmitAndWaitTails + // gives, and for the same reason. + MGLOG_E("MG_Remote client: the forced wait for %s woke on a dead doorbell; the " + "server is gone", why); + return; + } + if (wait != Transport::SessionWait::Reached) { + MGLOG_F("MGPipe: Fatal{BarrierTimeout, \"%s\"} - a forced run-ahead wait did not " + "reach appliedSeq %llu within %u ms. The forced waits of CONTRACT-P5E §2.5 " + "are the points at which the client must be in step again, so a wedge here " + "is a wedge in the queue and not a slow frame", + why, static_cast(target), kBarrierTimeoutMs); + std::abort(); + } + // EVERY WAIT EXIT DRAINS (§2.6). It is what the flow-control deadlock argument rests + // on: the server blocks only on a full event ring, the client blocks only on + // watermarks the server advances, and every client wait drains - so at most one side + // is parked at any instant. + DrainEventRing(m_events); + ReleaseFillPins(); +#else + (void)why; +#endif + } + + void ClientSession::Finish() { + // glFinish. Nothing goes on the wire - Flush and Finish have no record (ARCHITECTURE + // §11) - and under lockstep there was nothing to do, because the client had already + // waited out every command it issued. Under run-ahead "the commands issued so far have + // completed" is a promise the queue can break, so this is where it is kept. + WaitForApplyToCatchUp("glFinish"); + } + + Uint64 ClientSession::AcquirePresentCredit() { +#if MOBILEGL_BUILD_DISAGGREGATED + // MINTED CLIENT-SIDE AND 1-BASED (§1). Before P5e the client sent 0 and the server + // stamped its own frame count, which was honest while nothing paced on it; a credit + // needs an id space only the payer can advance, and that is this one. + const Uint64 serial = m_presentsSent + 1; + if (!m_runAheadArmed) { + // Lockstep: the present record's own barrier IS the pacing, one frame deep by + // construction. The serial is still minted so that both arms carry the same field + // and the server's ReturnPresentCredit is fed on both - an A/B whose two arms + // disagree about a wire field is not an A/B (Config.h's rule for the knobs). + m_presentsSent = serial; + return serial; + } + const Uint32 credit = MG_Config::Ipc.PresentCredit; + if (m_presentsSent >= credit) { + // Wait for swap number (serial - credit) to have come back. With credit 1 that is + // the previous frame: the client publishes frame N+1's records while the server + // applies and swaps frame N, and blocks here until swap N returned - one frame of + // overlap and at most one frame of added latency (ruling 4 / ID-92). + const Uint64 awaited = serial - credit; + ++m_presentCreditWaits; + const BarrierWaitScope waiting; + Transport::SessionWait wait = Transport::SessionWait::TimedOut; + for (;;) { + // §2.6 again: the credit park breaks on a full SEG_EVENT too, for the same + // reason the applied park does - a server that stopped producing stopped + // swapping, and this waiter holds the only drain. + wait = m_producer.WaitForPresentAckOrEventBacklog(awaited, kBarrierTimeoutMs); + if (wait != Transport::SessionWait::Reached) break; + Transport::RingControl* control = m_producer.Control(); + if (control == nullptr || + Transport::Watermark::Reached(control->presentAckSerial, awaited)) { + break; + } + DrainEventRing(m_events); + } + if (wait == Transport::SessionWait::TimedOut) { + MGLOG_F("MGPipe: Fatal{PresentCreditTimeout} - present %llu waited %u ms for " + "swap %llu to come back with a credit of %u. The credit, never the " + "ring's bytes, is what paces a run-ahead client (CONTRACT-P5E §2.4), " + "so a credit that never returns is a server that stopped swapping", + static_cast(serial), kBarrierTimeoutMs, + static_cast(awaited), credit); + std::abort(); + } + // ShutDown is teardown and returns: the present did not happen, the same answer + // the barrier gives on a dead doorbell. + if (wait == Transport::SessionWait::Reached) DrainEventRing(m_events); + } + m_presentsSent = serial; + return serial; +#else + return ++m_presentsSent; +#endif + } + // R-1's mutual-exclusion invariant, as two probes that answer honestly. // // THE CLIENT'S FLAG IS THREAD-LOCAL AND THE SERVER'S IS NOT, and the asymmetry is the @@ -1012,7 +1279,8 @@ namespace MobileGL::MG_Remote::Client { return g_applyThreadInsideApplier.load(std::memory_order_acquire); } - void ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface) { + void ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface, + Bool isBarrieredFill) { // THE CHAIN IS A CONJUNCTION, SO ITS ORDER IS FREE - AND IT IS ORDERED CHEAPEST-FIRST // (P5d round 3, package D). Every line below is a pure predicate whose only effect is // "no violation, go home"; the Fatal fires on the AND of all of them, so reordering @@ -1028,6 +1296,28 @@ namespace MobileGL::MG_Remote::Client { // both miss do we pay for a role probe, two singleton reads, a shared atomic and the // thread_local at the end. if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; + // ---- P5e (ra), CONTRACT-P5E §3.5: THE RUN-AHEAD RULE IS A DIFFERENT RULE ----------- + // + // It is first, before the BatchWaits return, because BatchWaits' argument does not + // survive run-ahead: "the pull-verbs' own wait keeps their pulled reads fenced" is a + // statement about a client that waits at every verb, and this one does not. Under + // run-ahead the block is SERVER-ROLE memory (§3) and the single legal GL-thread write + // is the residual fill of a BARRIERED record - the one the client is about to park + // behind. Every other touch races the applier by construction, so there is no + // in-applier flag to consult and no "count it" arm: the value would be torn. + // + // This is where a fill left standing for an unbarriered record lands, by name. + if (ClientSessionInstance().RunAheadArmed()) { + if (isBarrieredFill) return; + if (Server::ServerLoop::OnApplyThread()) return; // the applier owns the block + MGLOG_F("MGPipe: Fatal{RoleViolation, \"gPipeInputs\"} - the GL thread touched " + "gPipeInputs (%s) on a RUN-AHEAD session outside a barriered fill. With " + "the client running ahead the block is the server's to read for as long " + "as an unbarriered record is in flight (CONTRACT-P5E §3), and this thread " + "has no wait that would make the touch quiescent", + surface); + std::abort(); + } // MOBILEGL_IPC_BATCH_WAITS=1 makes "the fill runs while the apply thread applies an // earlier value record" the INTENDED shape: the fill writes only fields no record // supplies (the wire-live skips are the same answer the emitters use), the applier @@ -1090,6 +1380,14 @@ namespace MobileGL::MG_Remote::Client { } if (AdoptCapsSnapshot(envelope->msg_as_CapsSnapshot())) ++adopted; } + // P5e (ra, §1): the run-ahead latch rides R-12's invalidation edge, because "the first + // caps adoption after Start" and "the one place a CapsSnapshot becomes a mirror + // generation" are the same instant, and a latch taken anywhere else would be reading a + // mirror whose generation nobody had adopted yet. Called even when `adopted` is 0: the + // first call in Start() is what takes the latch, and a pump that adopted nothing still + // has to leave it taken (a handshake with no snapshot latches "not armed", which is + // the honest answer for a server that never said what it consumes). + if (adopted != 0 || !m_runAheadLatched) LatchRunAheadFromCaps(); return adopted; } @@ -1167,9 +1465,9 @@ namespace MobileGL::MG_Remote::Client { 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 cliwait=%llu clipark=%llu - R-10's proof obligation, " - "R-9's producer readings and P5d round 3's client wait pair, published from the " - "session that produced them", + "ringwaits=%llu emitseq=%llu cliwait=%llu clipark=%llu credit-waits=%llu - " + "R-10's proof obligation, R-9's producer readings, P5d round 3's client wait " + "pair and P5e's present credit, published from the session that produced them", static_cast(maxRecord), m_encoder.MaxRecordOpName(), static_cast(cap), static_cast(permille / 10), @@ -1185,7 +1483,13 @@ namespace MobileGL::MG_Remote::Client { // profile runs did not set MOBILEGL_PIPE_STATS either, and "did the barrier spin // or did it park" is the question their logs have to be able to answer. static_cast(m_producer.Waits()), - static_cast(m_producer.Parks())); + static_cast(m_producer.Parks()), + // P5e (ra, §2.4). The number the device exit reads to say whether the present + // credit is what paces this workload: 0 with credit 1 means the server is + // never the frame's critical path, and a count near the frame count means it + // always is. It is on this line rather than only on the stats line for + // LogWireLedger's own reason - a trace replay never reaches a stats summary. + static_cast(m_presentCreditWaits)); } #undef MGP5_C0_STUB diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index b4ff76df..a0a0f2c7 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -112,15 +112,79 @@ namespace MobileGL::MG_Remote::Client { // or its MGPDrawIndirect block behind the range array). The one-tail form above is this // one with a single WireTail; the barrier policy lives in exactly one body. A distinct // name rather than an overload, because `nullptr, 0` would match both. + // P5e (ra, CONTRACT-P5E §2.5): `wantReply` is the ONE caller-side relaxation of the + // catalogue's reply rule, and it exists for exactly one row - resource_subdata's + // BUFFER half, whose Bool the only caller discards (PipeFill.cpp's + // MGPipeEmitResourceSubData). The row keeps kReplySlot on the wire (the server still + // posts; an unread slot is harmless, ReplySlot.h:16, 105), so nothing about the + // protocol moves - what moves is that under run-ahead this client neither waits for + // the answer nor reads it, which is what makes the 64 KB persistent-map push + // fire-and-forget instead of a hidden round trip per block. It is NOT a licence to + // skip an answer a caller uses: R-5 still forbids re-deriving one locally, and every + // other kReplySlot row passes true (the default) and waits. Uint64 EmitAndWaitTails(MG_Pipe::MGPWireOp op, const void* payload, Uint64 payloadBytes, const Wire::WireTail* tails, Uint32 tailCount, void* replyOut, Uint64 replyBytes, Int32* statusOut, - Uint64* replySizeOut = nullptr); + Uint64* replySizeOut = nullptr, Bool wantReply = true); // MOBILEGL_IPC_VERB_BARRIER. False is the R-1 negative control and is EXPECTED to be // red; it must be run once and the way it goes red recorded. Bool BarrierArmed() const; + // ---- P5e (ra): the wait rule (CONTRACT-P5E §1, §2.3, §2.4, §2.5) ------------------- + + // A CONJUNCTION OF THREE, LATCHED ONCE: `m_barrierArmed && Ipc.RunAhead && + // Caps().HasCap(kCapRunAheadApply)`. The third conjunct is the server's own statement + // that it applies an unbarriered record without reading client memory, so run-ahead is + // never something the client can turn on by itself - on Magma, and on Espryt until the + // integration commit publishes bit 10, this answers false and EmitAndWaitTails runs + // today's lockstep path byte for byte. + // + // LATCHED AT THE FIRST CAPS ADOPTION AFTER Start, AND A LATER SNAPSHOT MAY ONLY TURN IT + // OFF. A caps re-run cannot promote a lockstep server to a run-ahead one mid-frame: + // records already published without a wait would have been published against the wrong + // rule, and there is no way to un-publish them. Demotion is safe in the other + // direction - it only adds waits - so it is allowed. + Bool RunAheadArmed() const; + + // §2.3: the four O-class SharedPtr rows of gPipeInputs (PipeInputs.h:801-809 - the + // bound VAO and the three programs), dropped on the GL thread once a barriered apply + // has returned. Under lockstep the next fill overwrites them within a verb; under + // run-ahead the next fill may be a frame away, and a pinned VAO whose last owner ends + // up being the apply thread is exactly the deferred-destroy hazard §2.7 keeps a queue + // for. Releasing them at the one instant the applier is provably idle costs four + // refcount drops per barriered record and removes the pin entirely. + void ReleaseFillPins(); + + // §2.5's forced wait, as one named entry point: publish nothing, wait for the apply + // thread to reach LastPublishedSeq, drain the reverse channel. `why` names the caller + // in the timeout Fatal, because "which forced wait wedged" is the whole diagnostic. + // A no-op when run-ahead is not armed (the client is already in step by construction). + void WaitForApplyToCatchUp(const char* why); + + // glFinish. Flush and Finish stay no-ops ON THE WIRE (ARCHITECTURE §11, :415) - there + // is no record to emit - but "the GL commands issued so far have completed" cannot be + // true while a run-ahead queue still holds them, so under run-ahead glFinish becomes + // exactly WaitForApplyToCatchUp + the drain that comes with it. + void Finish(); + + // The last seq this session published, whether or not it waited for it. The forced + // waits above are defined against it. + Uint64 LastPublishedSeq() const; + + // §2.4's counter, published on the stats line as `credit-waits`: how often the client + // actually blocked for a swap to come back. Zero with a credit of 1 means the server + // is never the frame's critical path; a number close to the frame count means the + // credit is what paces this workload, which is what the device exit measures. + Uint64 PresentCreditWaits() const; + + // §2.4, called by EmitPresent immediately before it encodes: pay the credit (parking + // if this client already has `MOBILEGL_IPC_PRESENT_CREDIT` presents in flight), then + // mint and return this frame's 1-based MGPPresent::FrameSerial. The credit wait is + // BEFORE the encode on purpose: a client that encoded first would hold a SEG_CMD + // reservation across the park. + Uint64 AcquirePresentCredit(); + // R-1's invariant made checkable rather than only written down: true while this // thread is inside a barrier wait. The apply thread sets its own flag on entry to the // applier; a debug/verify build asserts the two are never both true, and that the @@ -140,7 +204,17 @@ namespace MobileGL::MG_Remote::Client { // split session is live, when the caller IS the apply thread, and when the barrier is // disarmed - MOBILEGL_IPC_VERB_BARRIER=0 is R-1's negative control and its red belongs // to Fatal{BarrierViolation}, not to this check. - static void RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface); + // + // P5e (ra, CONTRACT-P5E §3.5) RE-DERIVES IT UNDER RUN-AHEAD, and `isBarrieredFill` is + // what the caller says about ITSELF: "this touch is the residual fill of a record this + // thread is about to park behind". With run-ahead armed the block is server-role + // memory for every other touch, so the rule stops being "not while the applier is + // inside" - which is unanswerable when the client never waits - and becomes "not at + // all, unless you are that fill". The MOBILEGL_IPC_BATCH_WAITS early return goes with + // it: batching is about how many round trips a lockstep client pays, and it says + // nothing about a client that pays none. + static void RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface, + Bool isBarrieredFill = false); // ---- c1's additions --------------------------------------------------------------- @@ -250,9 +324,20 @@ namespace MobileGL::MG_Remote::Client { void LogWireLedger() const; private: + // P5e (ra): the latch, taken from PumpControlPlane after every adoption. Private + // because "when may run-ahead start" is this class's own rule and not a caller's. + void LatchRunAheadFromCaps(); + Wire::PipeWireEncoder m_encoder; Wire::SegmentTable m_segments; Bool m_barrierArmed = true; + // P5e (ra): the latch of RunAheadArmed() and the fact that it has been taken once. + Bool m_runAheadArmed = false; + Bool m_runAheadLatched = false; + // §2.4: presents published (the 1-based FrameSerial space) and how often the credit + // wait really blocked. + Uint64 m_presentsSent = 0; + Uint64 m_presentCreditWaits = 0; std::unique_ptr m_clientTransport; std::unique_ptr m_serverTransport; diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index ac5dc9e4..3cc9451f 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -1034,10 +1034,20 @@ namespace MobileGL::MG_Remote::Client { } 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 - // id space with no consumer. - record.FrameSerial = 0; + // P5e (ra, CONTRACT-P5E §1, §2.4). FrameSerial USED TO BE 0 - "the server stamps + // its own" - and that was honest while nothing paced on it. It is now minted here, + // 1-based, by AcquirePresentCredit, which also PAYS the credit: if this client + // already has MOBILEGL_IPC_PRESENT_CREDIT presents in flight it parks until the + // server's OnPresent returns one, and only then does it mint. + // + // THE CREDIT WAIT IS BEFORE THE ENCODE, WHICH IS NOT A DETAIL: EmitAndWait + // reserves SEG_CMD bytes as its first act, so a client that encoded and then + // parked would hold a ring reservation across a whole frame of server time. It + // also drains SEG_EVENT on its way out, like every other wait (§2.6). + // + // With run-ahead disarmed this is a counter and nothing else, and the record below + // travels exactly as it did - the present row's own barrier is the pacing there. + record.FrameSerial = session.AcquirePresentCredit(); session.EmitAndWait(MG_Pipe::MGPWireOp::Present, &record, sizeof(record), nullptr, 0, nullptr, 0, nullptr); diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index 947fb771..ae11348b 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -340,15 +340,33 @@ namespace MobileGL::MG_Remote::Client { ClientSession& session = RequireSession("ResourceSubData"); MG_Pipe::MGPSubData record = *payload; record.Blob = StageOptional(session, blobBytes, blobByteCount); + // P5e (ra, CONTRACT-P5E §2.5 / ruling 15): the BUFFER half does not want its + // answer, and MGPipeSubDataWantsItsReply is the one place that decides - the same + // function the route reads, so the two halves of this call cannot disagree about + // whether a wait is owed. Under run-ahead the record is published and this thread + // returns; the persistent-map push (PersistentMapTracker's 64 KB blocks) stops + // being a hidden round trip per block, which is the single biggest wait left on + // the steady path that is not a draw. + // + // THE ANSWER IS ACCEPT-BY-CONSTRUCTION, AND THAT IS HONEST HERE AND NOWHERE ELSE: + // the only caller discards it (PipeFill.cpp's MGPipeEmitResourceSubData), so + // "accepted" is not a re-derivation of a server decision - it is the absence of a + // question. The server still posts the real answer into the slot; nothing reads + // it, which ReplySlot.h:16, 105 makes legal. A row whose acceptance a caller USES + // may never take this path - R-5 has not moved. + const Bool wantReply = MG_Pipe::MGPipeSubDataWantsItsReply(record); Int32 status = 0; - const Uint64 seq = session.EmitAndWait( - MGPWireOp::ResourceSubData, &record, sizeof(record), varTail, - static_cast(varTailCount) * sizeof(MG_Pipe::MGPSubRegion), nullptr, 0, - &status); + const Wire::WireTail tail{varTail, static_cast(varTailCount) * + sizeof(MG_Pipe::MGPSubRegion)}; + const Uint64 seq = session.EmitAndWaitTails( + MGPWireOp::ResourceSubData, &record, sizeof(record), + varTail != nullptr ? &tail : nullptr, varTail != nullptr ? 1u : 0u, nullptr, 0, + &status, nullptr, wantReply); reply->Id = seq; - MG_Pipe::MGPipePostReply(*reply, status, status == 0 ? 1u : 0u); + MG_Pipe::MGPipePostReply(*reply, wantReply ? status : 0, + (wantReply ? status == 0 : true) ? 1u : 0u); ++g_emitted; - if (status == 1) ++g_declined; + if (wantReply && status == 1) ++g_declined; } void Wire_BufferSubDataResident(const MG_Pipe::MGPSubData* payload, const void* blobBytes, diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index cfd3f735..645e9f22 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -10,6 +10,7 @@ #include "PipeApplier.h" +#include "ServerSession.h" #include "../Transport/ReplySlot.h" #include @@ -29,6 +30,20 @@ namespace MobileGL::MG_Remote::Server { + namespace { + // P5e (ra, CONTRACT-P5E §1 / §6): DOES THIS SERVER PUBLISH kCapRunAheadApply? The + // question is asked of the server's OWN CallMask and not of a build constant, because + // "the client may run ahead" is exactly what that bit says and Magma never sets it. + // A session with no CallMask yet (the bring-up window, a fixture that never called + // SetCapabilityBits) answers false: no client can have latched run-ahead against a + // snapshot that was never published. + Bool ServerPublishesRunAhead() { + const ServerSession* session = ServerSession::Active(); + if (session == nullptr || !session->CallMaskIsSet()) return false; + return (session->CallMask() & static_cast(MG_Pipe::kCapRunAheadApply)) != 0; + } + } // namespace + ReplyPool::ReplyPool(void* base, Uint64 sizeBytes, Uint32 slotCount, Uint32 slotBytes) : m_base(static_cast(base)), m_size(sizeBytes), m_slots(slotCount), m_slotBytes(slotBytes) {} @@ -288,7 +303,30 @@ namespace MobileGL::MG_Remote::Server { ++m_presents; // FrameSerial 0 means "the server stamps its own" (c1-v1 8.3): P5 has no client-side // present credit, so the client sends 0 and the frame count on this side IS the serial. + // + // P5e (ra, CONTRACT-P5E §1, §2.4): IT IS THE CLIENT'S NOW, 1-BASED AND MINTED BY THE + // PAYER. A credit can only be paced in an id space the waiter advances, and the waiter + // is the client (`WaitForPresentAck(m_presentsSent + 1 - credit)`), so a server-stamped + // serial would be the server acknowledging its own count. The 0 arm survives for a + // peer that has not been re-built - and on a server that PUBLISHES the run-ahead cap it + // is a protocol fault, because such a client is pacing on this answer and a 0 would + // acknowledge a frame nobody asked about. + if (present.FrameSerial == 0 && ServerPublishesRunAhead()) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"Present.FrameSerial\"} - a run-ahead " + "server was handed present serial 0. The client mints this 1-based and " + "waits on it for its credit (CONTRACT-P5E §2.4); returning a credit for " + "serial 0 would release a wait that is asking about frame N"); + std::abort(); + } m_lastPresentSerial = present.FrameSerial != 0 ? present.FrameSerial : m_presents; + // §2.4's other half, and the reason ServerSession::ReturnPresentCredit has had no + // production caller since v1 wrote it: ONE CREDIT PER SWAP, returned after Present() + // has returned rather than before it, because what the client is waiting for is the + // swap and not the record's apply. It advances presentAckSerial AND rings the client's + // bell - a client parked in WaitForPresentAck(kWaitForever) needs the pair. + if (ServerSession* session = ServerSession::Active()) { + session->ReturnPresentCredit(m_lastPresentSerial); + } return true; } diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 342b3e6e..30cc44ce 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -885,11 +885,19 @@ namespace MobileGL::MG_Remote::Wire { // The allocation still cannot progress after reclamation. Count this // blocked allocation once, not each watermark poll or each reclaimed mark. if (!waited) { ++m_stageReclaimWaits; waited = true; } - if (!m_stageRetirementBell->Wait(m_control->producerParked, ready, 0, 5000)) { + // P5e (ra, CONTRACT-P5E §2.6): drain BEFORE parking and again after waking. + // Before, because the ring may already be full and the server already stopped + // - in which case retiredSeq will never move and there is nothing to wake us; + // after, because the wake may have come from a server that is about to fill it + // again. The hook is null in every standalone codec, which has no session and + // therefore no reverse channel to drain. + if (m_stageWaitHook != nullptr) m_stageWaitHook(m_stageWaitSelf); + if (!ready() && !m_stageRetirementBell->Wait(m_control->producerParked, ready, 0, 5000)) { MGLOG_F("MGPipe: Fatal{RetirementWaitFailed, \"SEG_STAGE\"} producer wait " "ended before the pending allocation retired (shutdown or timeout)"); std::abort(); } + if (m_stageWaitHook != nullptr) m_stageWaitHook(m_stageWaitSelf); } ReclaimStagedBytes(); } diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index 05d42005..2bf944c3 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -340,6 +340,20 @@ namespace MobileGL::MG_Remote::Wire { // without a consumer cannot wait for retirement and retain the named refusal. void SetStageRetirementDoorbell(Transport::Doorbell* bell) { m_stageRetirementBell = bell; } + // P5e (ra, CONTRACT-P5E §2.6): THE STAGE BELL IS A CLIENT WAIT, so it has to drain the + // reverse channel like every other one. The encoder cannot drain it itself - SEG_EVENT + // and its consumers belong to the session - so the session installs a hook and this + // layer only says WHEN. Without it the flow-control deadlock is one buffer upload + // wide: the server stops applying on a full event ring, retiredSeq stops moving, and a + // client parked here for staged bytes never drains the ring that would release it. + // A raw function pointer rather than std::function: this header sits below MG_Impl and + // the call is on a path that is already about to park. + using StageWaitHook = void (*)(void* self); + void SetStageWaitHook(StageWaitHook hook, void* self) { + m_stageWaitHook = hook; + m_stageWaitSelf = self; + } + // 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 @@ -383,6 +397,9 @@ namespace MobileGL::MG_Remote::Wire { Uint64 m_cmdWrapPads = 0; Uint64 m_stageReclaimWaits = 0; Transport::Doorbell* m_stageRetirementBell = nullptr; + // P5e (ra): the session's event drain, called around the stage-bell park (§2.6). + StageWaitHook m_stageWaitHook = nullptr; + void* m_stageWaitSelf = nullptr; Vector m_stageMarks; SizeT m_stageMarkFront = 0; Uint8* m_stageBase = nullptr;