diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 4447dbcf..ed19e4c3 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -8844,14 +8844,21 @@ namespace MobileGL::MG_Backend::DirectGLES { // The one direct-iteration site over a twin table. The legacy walk reads the map // KEY, i.e. the raw frontend address, and has to test the entry's weak_ptr by hand - // before it dares dereference it. ForEachLive hands over a strong reference instead, - // so that hazard cannot arise; the body is otherwise identical, which is why it is - // lifted into a lambda both arms call. + // before it dares dereference it; the body is otherwise identical, which is why it + // is lifted into a lambda both arms call. #if MOBILEGL_PIPE_PUSH - // The push arm walks the twin table through ForEachLive, which hands over a - // STRONG reference to the state object; the legacy map walk below it reads the - // map key - the raw frontend address - and has to test the entry weak_ptr by - // hand first. The body is shared between the two arms through the lambda. + // P5e (id, CONTRACT-P5E §4.1): the push arm walks the twin table through + // ForEachLive, which now hands over the entry's HANDLE and its twin - never a + // frontend SharedPtr, because a walk that produced one on every step was the last + // place the server held a frontend object across records. This site asks for the + // object BY HANDLE instead, through StateForHandle, inside the frontend-keyed + // registry scope opened above: one named, greppable read that the fb package + // replaces with its reverse index (texture -> FBO slots), rather than a property of + // the iteration that nothing could name. + // + // The set of framebuffers walked is UNCHANGED: an entry whose state object the old + // walk could not lock is one StateForHandle answers null for, and detachFrom + // returns immediately on a null FBO. const auto detachFrom = [&](MG_State::GLState::FramebufferObject* stateFBO, const SharedPtr& backendFBO) { if (stateFBO == nullptr || !backendFBO || stateFBO->IsDefaultFramebuffer()) { @@ -8902,8 +8909,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // below is the RUNTIME one. if (EsprytSlotTablesEnabled()) { FramebufferImpl::g_backendFramebufferObjects.ForEachLive( - [&](const SharedPtr& stateFBO, + [&](MG_Pipe::MGPipeHandle fbo, const SharedPtr& backendFBO) { + const auto stateFBO = + FramebufferImpl::g_backendFramebufferObjects.StateForHandle(fbo); detachFrom(stateFBO.get(), backendFBO); }); return; diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 6d170b25..9b08eeac 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -223,37 +223,45 @@ namespace MobileGL::MG_Backend::DirectGLES { // nor the client's allocator is touched from there any more. if (MG_Config::Transport != MG_Config::TransportMode::Monolith && !MG_Remote::Server::ServerLoop::OnApplyThread()) { - if (MG_Remote::Client::EmitObjectDeathRecord(kind, lifetimeId) != - MG_Remote::Client::ObjectDeathEmit::NoSession) { - return; - } - // NoSession is the ONE shape the record cannot carry: a transport configured - // with no client session at all - a ServerLoop fixture driving a server role - // with no client, never a real split (there a session exists whenever the - // server does). The server loop IS there, so the death takes the delivery - // that predates the record, kept VERBATIM for exactly this arm: a stack - // struct hopped to the apply thread, which runs the switch below. NoHandle - // does NOT land here - a handle that never existed has no twin to kill (§5.2) - // - and a loop that is not RUNNING (teardown, pre-Start) has no thread to hop - // to; the twin dies with the server either way. - if (!MG_Remote::Server::ServerLoopInstance().Running()) { - return; - } - struct Death { MG_Pipe::MGPipeKind kind; Uint64 lifetimeId; } death{kind, lifetimeId}; - MG_Remote::Server::ServerLoopInstance().RunOnApplyThread( - +[](void* user) -> MobileGLResult { - const auto& death = *static_cast(user); - OnFrontendStateObjectDestroyed(death.kind, death.lifetimeId); - return MOBILEGL_OK; - }, &death); + // P5e (id), CONTRACT-P5E §4.4: THE RECORD IS THE ONLY DELIVERY NOW. What used + // to sit here was a mailbox hop for the ONE shape the record cannot carry - a + // transport configured with no client session at all, i.e. a ServerLoop + // fixture driving a server role with no client, never a real split (there a + // session exists whenever the server does). It posted a stack struct to the + // apply thread, which re-entered this function and ran the switch below; that + // switch's DestroyByLifetimeId probes and FREES in the CLIENT'S allocator from + // the apply thread, which is precisely the read P5e is retiring - and it is + // not a read a barrier could ever make safe, because under run-ahead there is + // no wait behind which the allocator stands still. + // + // WHAT REPLACES IT: nothing, and that is the answer rather than an omission. A + // fixture with no client session has no client allocator worth the name and no + // handle ever crossed, so there is no twin the server built from a record; its + // twins die with the backend at Stop (InProcessTeardown / the registry's own + // destruction), which is what already happened for every death raised past + // `Running() == false`. ID-96 enumerates the fixtures this changed and what + // each one drives instead; every one of them now drives `object_death` by + // handle through the applier, which is the delivery a real split uses. + // ID-96, ANSWERED BY MEASUREMENT: the set of fixtures that lose death delivery + // here is EMPTY. A probe that aborted in exactly this branch (NoSession with a + // RUNNING server loop, i.e. the only shape the deleted hop ever fired for) was + // run over the whole unit lane (2204 entries) and the whole integration-split + // lane (111) and never fired once. The one fixture family that could reach it - + // ServerLoopTest.cpp's ServerFixture / EglServerFixture, the only server role in + // the tree with no client session - has exactly one case that lets a re-keyed + // frontend object die off the apply thread + // (ServerLoopEglTest.FrontendFramebufferDeathDeletesOnTheContextOwner), and that + // case is already refused at its FIRST step by P5c's allocator guard, so its + // `framebuffer.reset()` was unreachable at 2fde7034 too. + (void)MG_Remote::Client::EmitObjectDeathRecord(kind, lifetimeId); return; } - // P5c merge coordination: the record arm above carries every death a real split - // sees; what still lands HERE through the mailbox is the NoSession server-only - // fixture arm, whose DestroyByLifetimeId probes the client allocator from the - // apply thread. Those probes ride inside the G6 scope (CONTRACT-P5C §5.2, the - // amended ruling) - named debt that dies with the mailbox hop. - const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; + // Falling through HERE under a transport means the death was raised ON the apply + // thread - the server backend destroying a frontend object it created itself + // (Magma's hidden blit/depth-mipmap resources). Those sites carry their own named + // scope (MagmaP7AllocatorDebtScope), so the allocator probe below is admitted + // there and refused everywhere else; this function no longer opens a scope of its + // own, because the one arm that needed it is the mailbox hop that just went. #endif switch (kind) { case MG_Pipe::MGPipeKind::Texture: @@ -325,6 +333,29 @@ namespace MobileGL::MG_Backend::DirectGLES { } // namespace #if MOBILEGL_BUILD_DISAGGREGATED + // P5e (id), CONTRACT-P5E §4.2 / §4.3: THE TWIN DEATH MOMENT, AND THE ABA ANSWER. + // + // The moment: the apply of `object_death` / the kind's own delete opcode / `resource_destroy` + // - in RING ORDER, i.e. after every verb that named the dying generation and before any + // record that names the slot's next owner - or the server backend's own destruction at Stop + // (InProcessTeardown). `applier_reset` touches neither twins nor object records + // (PipeApply.h's MGPipeApplierReset), so a make-current kills nothing. + // + // The ABA answer, which is what makes "no wait is added at any delete" (§2.7) safe: Gen + // moves ONLY on reuse (SlotAllocator.cpp), the client frees a slot only AFTER its death + // record went out (PipeFill.cpp's NotifyAndFree), and the ring is in order. So at the + // server every record naming {s, g} is applied before object_death{s, g}, which is applied + // before create{s, g+1}. THE ORDERING IS THE ARGUMENT AND THE GEN COMPARE IS THE PROOF: + // * GetOrCreate({s, g+1}) on a live {s, g} entry RE-MINTS - it resets the twin, so the + // successor never inherits the dead object's driver ids (SlotTables.h); + // * FindByHandle({s, g}) after the recycle answers NULL, so a stale handle resolves to + // nothing rather than to its successor's twin; + // * a backward {s, g-1} is REFUSED rather than adopted, so a late record cannot destroy + // the incumbent. + // Which means even a SKIPPED death (an object that never crossed emits none, + // WireTables.cpp) is answered by the forward-Gen reset alone. Pinned by + // DirectGLESSlotTable.AGenerationBehindTheLiveTwinIsRefusedRatherThanAdopted (all three + // clauses, on the table itself) and end to end by the CtWireScenario recycle cases. Bool ReleaseTwinsForWireObjectDeath(MG_Pipe::MGPipeHandle handle, MG_Pipe::MGPipeKind kind) { // The handle-keyed twin of the notice switch above, and deliberately NOT that switch: // the record carried the handle, so no lifetime id is probed and the client's @@ -2932,10 +2963,19 @@ namespace MobileGL::MG_Backend::DirectGLES { #if MOBILEGL_BUILD_DISAGGREGATED // P5c merge coordination (CONTRACT-P5C §3.1's named exemption): every caller of // this function is a site whose handle-carrying records (set_shader_buffers / - // set_stream_output_targets) the client does not emit until P4b, so the probe + // set_stream_output_targets) the client does not emit until sb, so the probe // below runs inside the scoped exemption. The scope, not a silent guard removal, // is what keeps the debt named and greppable. - const MG_Pipe::MGPipeReverseAnnouncementScope reverseAnnouncement; + // + // P5e (id), ruling 12: THE SCOPE CHANGED UNDER THIS SITE AND THE SITE HAD TO MOVE. + // The exemption it used to name is now MagmaP7AllocatorDebtScope and is keyed on a + // DirectVulkan server (Magma's four P7 debts); this is Espryt's, and it belongs to + // the frontend-keyed registry family with the rest of the Espryt debt. Which scope + // it names is not cosmetic: the Magma one would not exempt it here at all. It + // retires when vi/sb carry the handle into EnsureBufferResourceForHandle / + // IsBufferDrawCleanByHandle / MarkBufferGpuWrittenByHandle, which is the last + // caller set this function has. + const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; #endif return g_backendBufferResources.HandleOf(bufferObject); } @@ -4876,6 +4916,27 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!record.Live || record.Gen != cso.Gen) return nullptr; return &record; } + + // P5e (id), CONTRACT-P5E §4.1. See Managers.h for the contract; the body is the one + // SamplerImpl::ResolveSamplerCsoTwin already runs, with the sync left to vi. + BackendVertexArrayObject* ResolveVaoTwin(MG_Pipe::MGPipeHandle elements) { + if (MG_Pipe::MGPipeHandleIsNull(elements)) return nullptr; + // THE RECORD FIRST, before the table is touched: a handle with no record means the + // client named a vertex-elements CSO it never described (or one it deleted while a + // binding still names it), and adopting a slot for it would leave a twin nothing + // can ever sync. The applier's own Gen check is what turns a stale handle into this + // null rather than into its successor's record. + if (PipeRecordAt(MG_Pipe::MGPipeApplier().VertexElementsCsos, elements) == nullptr) { + MGLOG_E_ONCE("MGPipe: vertex-elements CSO {%u, %u} has no applier record on the handle " + "arm, so no driver VAO can be built for it and the draw keeps what is bound", + elements.Slot, elements.Gen); + return nullptr; + } + auto* slot = AdoptTwinByHandle(g_backendVertexArrayObjects, elements, "vertex-elements CSO"); + if (slot == nullptr) return nullptr; + if (!*slot) *slot = MakeShared(); + return slot->get(); + } #endif // MOBILEGL_PIPE_PUSH void BackendVertexArrayObject::SyncToBackend( @@ -9096,6 +9157,28 @@ namespace MobileGL::MG_Backend::DirectGLES { MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS> g_boundTexturesCache; TwinRegistry g_backendTextureObjects; + +#if MOBILEGL_PIPE_PUSH + // P5e (id), CONTRACT-P5E §4.1. See Managers.h for the contract. + BackendTextureObject* ResolveTextureTwin(MG_Pipe::MGPipeHandle res) { + if (MG_Pipe::MGPipeHandleIsNull(res)) return nullptr; + // THE RECORD FIRST: a texture handle with no resource record is a seam defect - the + // client named a texture it never created, or one whose resource_destroy has + // already applied while a binding still names it - and a twin adopted for it would + // own a driver texture nothing ever uploads to. + if (PipeTextureRecordForHandle(res) == nullptr) { + MGLOG_E_ONCE("MGPipe: texture {%u, %u} has no applier resource record on the handle " + "arm, so no driver texture can be built for it and the unit keeps what " + "it holds", + res.Slot, res.Gen); + return nullptr; + } + auto* slot = AdoptTwinByHandle(g_backendTextureObjects, res, "texture"); + if (slot == nullptr) return nullptr; + if (!*slot) *slot = MakeShared(); + return slot->get(); + } +#endif } // namespace TextureImpl namespace FramebufferImpl { @@ -10756,6 +10839,27 @@ namespace MobileGL::MG_Backend::DirectGLES { constexpr Int kMaxDrainedProgramErrors = 32; TwinRegistry g_backendProgramObjects; +#if MOBILEGL_PIPE_PUSH + // P5e (id), CONTRACT-P5E §4.1. See Managers.h for the contract - in particular why the + // composite band had to land in the same package as this function. + BackendProgramObjectImpl* ResolveProgramTwin(MG_Pipe::MGPipeHandle cso) { + if (MG_Pipe::MGPipeHandleIsNull(cso)) return nullptr; + // THE RECORD FIRST, and through the BAND-AWARE reader: a composite pipeline's CSO + // lives in the applier's CompositeShaderCsos table, and asking the ordinary one for + // it would answer null for a program that is perfectly live. + if (PipeShaderCsoRecordForHandle(cso) == nullptr) { + MGLOG_E_ONCE("MGPipe: shader CSO {%u, %u} has no applier record on the handle arm, so " + "no driver program can be built for it and the draw keeps what is bound", + cso.Slot, cso.Gen); + return nullptr; + } + auto* slot = AdoptTwinByHandle(g_backendProgramObjects, cso, "shader CSO"); + if (slot == nullptr) return nullptr; + if (!*slot) *slot = MakeShared(); + return slot->get(); + } +#endif + BackendProgramObjectImpl::BackendProgramObjectImpl() { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 4af571a4..f0f426d8 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -467,10 +467,15 @@ namespace MobileGL::MG_Backend::DirectGLES { return m_slotTable.LiveGenAt(slot); } -#if MOBILEGL_BUILD_DISAGGREGATED // P5c (hd): the two halves of SlotTables.h's state note, forwarded. A caller holding // both the record's handle and the frontend object (the record-driven sync) notes the // object so a later handle-only resolution can reach it without the client allocator. + // + // P5e (id): under MOBILEGL_PIPE_PUSH rather than MOBILEGL_BUILD_DISAGGREGATED, because + // the re-typed ForEachLive's caller resolves its object through StateForHandle in the + // push-monolith build too. Each half is a named Fatal from an unbarriered apply + // (SlotTables.h); on the legacy arm there is no note and StateForHandle answers null, + // which is the answer the legacy walk's own weak_ptr test already gives. void NoteStateForHandle(MG_Pipe::MGPipeHandle handle, const StatePtr& stateObj) { if (EsprytSlotTablesEnabled()) { m_slotTable.NoteStateForHandle(handle, stateObj); @@ -482,7 +487,6 @@ namespace MobileGL::MG_Backend::DirectGLES { } return nullptr; } -#endif // NO ReleaseByHandle HERE THROUGH P5, AND THAT WAS A DECISION (review M-4). The death // half of GetOrCreateByHandle existed only for a kind whose announcement is its own @@ -527,23 +531,21 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } - // fn(const StatePtr& state, const BackendPtr& twin) over every live entry. The legacy - // begin()/end() handed out the map key, i.e. the raw frontend address - exactly the - // identity the backend must stop reading - and handed it out for entries whose state - // object had already died, so the one caller had to test stateRef.expired() itself - // before dereferencing it. Here the state object arrives as a strong reference. + // P5e (id), CONTRACT-P5E §4.1: fn(MGPipeHandle, const BackendPtr& twin) over every live + // entry of the HANDLE ARM, and of that arm only. + // + // IT NO LONGER SERVES THE LEGACY ARM, and that is a narrowing rather than a loss. The + // legacy map is keyed by the raw frontend address and has no handle to hand over - a + // synthesised null one would be a lie the callee could not tell from a real answer - + // and its one caller (ScopedDetachedTextureFramebufferAttachments) has always had its + // own `#if MOBILEGL_PIPE_LEGACY_MEMOS` begin()/end() walk beside the call, because the + // legacy entry needs its stateRef.expired() test done by hand. So the arm check here + // was answering a question no caller asked, and leaving it would have meant inventing a + // second signature for a walk the P5e rule exists to delete. template void ForEachLive(Fn&& fn) const { if (EsprytSlotTablesEnabled()) { m_slotTable.ForEachLive(fn); - return; - } - for (const auto& [stateKey, entry] : m_entries) { - (void)stateKey; - if (!entry.backend) continue; - const StatePtr state = entry.stateRef.lock(); - if (!state) continue; - fn(state, entry.backend); } } #endif @@ -1423,6 +1425,24 @@ namespace MobileGL::MG_Backend::DirectGLES { extern TwinRegistry g_backendVertexArrayObjects; +#if MOBILEGL_PIPE_PUSH + // P5e (id), CONTRACT-P5E §4.1 / §4.2: THE VAO TWIN, RESOLVED BY THE HANDLE THE RECORD + // CARRIED - `MGPipeApplierState::BoundVertexElements` at a draw, never + // `Find(vao.get())`. One of the three resolvers the identity package lands so the + // per-family packages have a by-handle door from day one; SamplerImpl's + // ResolveSamplerCsoTwin is the shape all four share. + // + // WHAT IT DOES AND, AS IMPORTANTLY, WHAT IT DOES NOT. Record first (a handle with no + // applier record is a seam defect, and adopting a slot for it would leave a twin that + // syncs nothing), then AdoptTwinByHandle, then a twin if the slot is empty. It never + // touches a frontend object and never probes the client's slot allocator - those two + // absences ARE the deliverable - and it does not SYNC: which serials gate a VAO sync, + // and what the sync reads, is the vi package's, and a resolver that synced would have + // to know. Null, loudly, for a handle with no record or a generation behind the live + // twin's; null silently for the null handle. + BackendVertexArrayObject* ResolveVaoTwin(MG_Pipe::MGPipeHandle elements); +#endif + // Shadowed glBindVertexArray: every backend VAO bind goes through here so a // draw's second bind of the same VAO (SyncToBackend, then PrepareForDraw's // re-bind) reaches the driver once. Invalidate whenever the ES context is @@ -1841,6 +1861,22 @@ namespace MobileGL::MG_Backend::DirectGLES { void UnbindTexture(Uint unit, GLenum target); extern TwinRegistry g_backendTextureObjects; + +#if MOBILEGL_PIPE_PUSH + // P5e (id), CONTRACT-P5E §4.1 / §4.2: THE TEXTURE TWIN BY HANDLE - the handle a record + // carried (`BoundSamplerViews[u].Texture`, `BoundShaderImages[u].Res`, a framebuffer + // record's `MGPSurface::Res`, `VerbMipRes`, `VerbCopyTexDst`, `MGPCopyImage`'s two + // endpoints), never `Find(textureObject.get())`. Same shape and the same three + // absences as VertexArrayImpl::ResolveVaoTwin: record first, no frontend touch, no + // allocator probe, and NO SYNC - the three texture syncs and the clean condition that + // gates them are the tx2 package's. + // + // The by-value twin copy plus second Find that SyncTextureObjectToBackend pays today + // (the registry's Find could relocate its own return) is not reproduced here: the slot + // table's answer is an array element and only a GetOrCreate that GROWS the table moves + // it, which this function has already done by the time it returns. + BackendTextureObject* ResolveTextureTwin(MG_Pipe::MGPipeHandle res); +#endif SharedPtr& SyncTextureObjectToBackend( const SharedPtr& textureObject, Bool imageBindableStorageRequired = false); @@ -2598,6 +2634,23 @@ namespace MobileGL::MG_Backend::DirectGLES { extern TwinRegistry g_backendProgramObjects; +#if MOBILEGL_PIPE_PUSH + // P5e (id), CONTRACT-P5E §4.1 / §4.2: THE PROGRAM TWIN BY HANDLE - `st.DrawProgram` at + // a draw, `st.DispatchProgram` at a dispatch, `st.BoundShaderCso` at a bind - never + // `Find(currentProgram.get())` and never the raw-pointer stash. Same shape and the same + // three absences as the VAO and texture resolvers beside it: record first, no frontend + // touch, no allocator probe, no sync (the nine-clause clean condition and what feeds it + // are the pg package's). + // + // THE BAND IS WHY THIS ONE MATTERS MOST. A program-pipeline composite's ShaderCso slot + // is >= kMGPipeShaderCsoCompositeSlotBase, and before P5e the by-handle path would have + // grown g_backendProgramObjects to ~983k entries to reach it. SlotTables.h's m_band + // lands in this package for exactly that reason - the rekey makes the composite the + // ordinary path, so the band is a prerequisite and not a follow-up. The record reader + // (PipeShaderCsoRecordForHandle) has been band-aware since P4a. + BackendProgramObjectImpl* ResolveProgramTwin(MG_Pipe::MGPipeHandle cso); +#endif + // Points one shader storage block of an ALREADY-LINKED backend program at // `binding`. `blockName` is the frontend interface-query spelling; the real // driver's own index for it is looked up here, because the transpiled ESSL's diff --git a/MobileGL/MG_Backend/DirectGLES/SlotTables.h b/MobileGL/MG_Backend/DirectGLES/SlotTables.h index 8b44fa0b..3ae3d464 100644 --- a/MobileGL/MG_Backend/DirectGLES/SlotTables.h +++ b/MobileGL/MG_Backend/DirectGLES/SlotTables.h @@ -58,14 +58,20 @@ // of the six kinds has exactly one table type in this backend. Magma's subsystem-4 table mints // out of its own per-renderer allocator, not MGPipeSlots(), so it is not a holder here.) // -// The weak_ptr per entry survives for exactly one reason: ForEachLive() hands the callee a -// STRONG reference to the frontend object, which the one direct-iteration site -// (ScopedDetachedTextureFramebufferAttachments) needs. It is never an identity test - that is -// what Gen is for - and it is never read to decide whether an entry is dead: a destructor that -// runs after exit() has begun has its notice dropped by InProcessTeardown(), and that twin is -// then a DELIBERATE leak (the process is exiting, the driver reclaims the object, and a twin -// destructor must not call into a driver that may already be unloaded), not something to be -// collected later. +// The weak_ptr per entry survives as the MONOLITH-GLUE half of the table's identity: the +// minting GetOrCreate stores it, NoteStateForHandle/StateForHandle read it, and the legacy +// death notice walks it. It is never an identity test - that is what Gen is for - and it is +// never read to decide whether an entry is dead: a destructor that runs after exit() has begun +// has its notice dropped by InProcessTeardown(), and that twin is then a DELIBERATE leak (the +// process is exiting, the driver reclaims the object, and a twin destructor must not call into +// a driver that may already be unloaded), not something to be collected later. +// +// P5e (id, CONTRACT-P5E §4.1): ForEachLive() NO LONGER HANDS IT OUT. It answers +// fn(MGPipeHandle, const BackendPtr&), because a walk that produced a frontend SharedPtr out +// of server memory was the last place the server could hold one across records - and a caller +// that still needs the object (the detach walk) asks StateForHandle for it BY HANDLE, inside +// the scope that names the debt, so the read is one greppable site rather than a property of +// the iteration. // // P3+ DEBT, recorded rather than hidden: this header is under MG_Backend/ and it MINTS // handles (MGPipeSlots().Acquire below) off a frontend SharedPtr's GetLifetimeId(). @@ -81,6 +87,20 @@ // Fatal{RoleViolation, "MGPipeSlots"} when reached from the apply thread (the same check the // allocator's own three entries carry, repeated here so the refusal names this surface), and // the split paths resolve through GetOrCreate(handle) / ReleaseByHandle instead. +// +// P5e (id, CONTRACT-P5E §4.1 / §5.8) SETTLES WHAT HAPPENS TO THE FRONTEND-KEYED HALF: it is +// NOT deleted. The push build under Transport=monolith keeps its frontend arms token for token +// (ruling 1), which is what the MOBILEGL_PIPE_VERIFY comparator needs and what makes +// MOBILEGL_IPC_RUN_AHEAD=0 a pure wait-rule A/B on identical server code. So +// GetOrCreate(StatePtr), Find(StateObject*), HandleOf, NoteStateForHandle/StateForHandle, +// Entry::stateRef and the resolution memo all survive - as MONOLITH GLUE, each of them a NAMED +// FATAL the moment it is reached from an apply thread that is applying an UNBARRIERED record +// (§4.4: the client's wait is the only thing that makes such a read stable, and an unbarriered +// record has none). The refusal is raised either by the allocator guard, for the three members +// that call the allocator, or by MGPipeRefuseFrontendKeyedRegistryFromUnbarrieredApply for the +// three that do not. There is no silent path: the phase's claim is that the twin is resolved +// from the handle the record carried, and a surface that quietly answered from a frontend +// pointer instead would make that claim untestable. namespace MobileGL::MG_Backend::DirectGLES { #if MOBILEGL_PIPE_PUSH @@ -153,12 +173,34 @@ namespace MobileGL::MG_Backend::DirectGLES { // is letting a corrupt 32-bit slot decide a vector resize. See GetOrCreate(MGPipeHandle). static constexpr Uint32 kMaxHandleSlot = 1u << 20; + // P5e (id), CONTRACT-P5E §4.3: THE COMPOSITE BAND, and it is a P5e PREREQUISITE rather + // than a follow-up. ShaderCso's top 1/16 of slot space (from + // kMGPipeShaderCsoCompositeSlotBase = 983040) is the program-pipeline composites' + // (MGPipeHandles.h); EntryAt below indexes m_slots BY SLOT and resizes to it, so a + // single composite used to grow g_backendProgramObjects to ~983k entries of ~40 B - + // ~40 MB for one program pipeline. Today that is reachable only through + // GetOrCreate(StatePtr) for a composite program; after the rekey the by-handle + // resolution IS the ordinary path, so every composite bind would pay it. + // + // A SECOND VECTOR RATHER THAN MORE OF THE FIRST, exactly as the allocator + // (SlotAllocator.h's KindState::BandSlots) and the applier (PipeApply.h's + // CompositeShaderCsos) already do, and for the same reason: both spaces stay dense + // against their own high-water mark, which is the property that lets the server index + // rather than hash. The band is EMPTY for every kind but ShaderCso and costs those + // kinds one empty Vector per table. + static constexpr Bool kHasCompositeBand = (kKind == MG_Pipe::MGPipeKind::ShaderCso); + static constexpr SizeT kMaxBandSlots = + MG_Pipe::kMGPipeShaderCsoSlotLimit - MG_Pipe::kMGPipeShaderCsoCompositeSlotBase; + struct Entry { BackendPtr backend; - // LIVENESS ONLY, and only for ForEachLive(), which locks it so the callee holds a - // strong ref. Never compared against another object to decide identity - that is - // what Gen is for - never dereferenced for its address, and never read to decide - // whether the slot is dead: death is announced, not discovered. + // THE MONOLITH-GLUE HALF of the entry (P5e, CONTRACT-P5E §4.1): written by the + // minting GetOrCreate and by NoteStateForHandle, read only by StateForHandle and + // the legacy death notice. Never compared against another object to decide identity + // - that is what Gen is for - never dereferenced for its address, and never read to + // decide whether the slot is dead: death is announced, not discovered. ForEachLive + // stopped reading it at P5e: a walk that handed a frontend SharedPtr out of server + // memory on every step was the last place the server could hold one across records. StateWeakPtr stateRef; // The generation this entry's twin was built for. An entry whose Gen no longer // matches the allocator's is a twin of the slot's PREVIOUS owner. @@ -173,6 +215,7 @@ namespace MobileGL::MG_Backend::DirectGLES { BackendSlotTable() { LinkHolder(); } BackendSlotTable(const BackendSlotTable& other): m_slots(other.m_slots), + m_band(other.m_band), m_nullTwin(other.m_nullTwin), m_memoLifetimeId(other.m_memoLifetimeId), m_memoHandle(other.m_memoHandle) { @@ -180,16 +223,19 @@ namespace MobileGL::MG_Backend::DirectGLES { } BackendSlotTable(BackendSlotTable&& other) noexcept: m_slots(std::move(other.m_slots)), + m_band(std::move(other.m_band)), m_nullTwin(std::move(other.m_nullTwin)), m_memoLifetimeId(other.m_memoLifetimeId), m_memoHandle(other.m_memoHandle) { other.m_slots.clear(); + other.m_band.clear(); other.ForgetHandle(); LinkHolder(); } BackendSlotTable& operator=(const BackendSlotTable& other) { if (this != &other) { m_slots = other.m_slots; + m_band = other.m_band; m_nullTwin = other.m_nullTwin; m_memoLifetimeId = other.m_memoLifetimeId; m_memoHandle = other.m_memoHandle; @@ -199,10 +245,12 @@ namespace MobileGL::MG_Backend::DirectGLES { BackendSlotTable& operator=(BackendSlotTable&& other) noexcept { if (this != &other) { m_slots = std::move(other.m_slots); + m_band = std::move(other.m_band); m_nullTwin = std::move(other.m_nullTwin); m_memoLifetimeId = other.m_memoLifetimeId; m_memoHandle = other.m_memoHandle; other.m_slots.clear(); + other.m_band.clear(); other.ForgetHandle(); } return *this; @@ -241,6 +289,12 @@ namespace MobileGL::MG_Backend::DirectGLES { // overload below. (The check also lives at the allocator's own three entries; it // is repeated at this entry so the refusal names this surface even if the entry // set changes.) + // + // P5e (id, §4.4): that one call is also this member's unbarriered-apply refusal. + // The guard exempts a probe only inside a named scope AND while the record being + // applied is barriered, so reaching a MINT from an unbarriered apply aborts here + // whatever scope is open - which is what "kept only as monolith glue" has to mean + // if it is to be checkable. MG_Pipe::MGPipeRefuseAllocatorFromApplyThread("GetOrCreate(StatePtr)"); #endif @@ -279,10 +333,13 @@ namespace MobileGL::MG_Backend::DirectGLES { // so the twin at it describes driver ids the new resource never made) and hand back // the twin pointer. FindByHandle beside it is the same shape and already existed. // - // No StatePtr, therefore no Entry::stateRef: the weak pointer is liveness for - // ForEachLive() and a handle-keyed entry has no frontend object to weakly hold. Such - // an entry is therefore invisible to ForEachLive, which is correct - the one direct - // iteration site walks texture twins, and it is not one of these tables. + // No StatePtr, therefore no Entry::stateRef unless a caller that holds the object notes + // it (NoteStateForHandle): a handle-keyed entry has no frontend object to weakly hold + // by itself. P5e (§4.1) makes that stop mattering for the iteration - ForEachLive keys + // on Live && backend, so such an entry is VISIBLE to the walk and it is StateForHandle, + // asked by the one caller that needs the object, that answers null for it. Which is the + // same set the old stateRef-locking walk produced, decided at one site instead of in + // the loop. // // Death stays ANNOUNCED, as it is on the other overload: for a handle-keyed kind the // announcement is the family's own destroy call, not the shared death notice, and the @@ -345,38 +402,48 @@ namespace MobileGL::MG_Backend::DirectGLES { // holds no live entry. It exists so a caller can DIAGNOSE - in a release build, where // MOBILEGL_ASSERT is inert - the refusal GetOrCreate(handle) above performs silently. Uint32 LiveGenAt(Uint32 slot) const { - if (slot >= m_slots.size()) return 0; - const Entry& entry = m_slots[slot]; - return entry.Live ? entry.Gen : 0; + const Entry* const entry = EntryOrNull(slot); + if (entry == nullptr) return 0; + return entry->Live ? entry->Gen : 0; } -#if MOBILEGL_BUILD_DISAGGREGATED // P5c (hd): remember the frontend object a HANDLE-keyed twin was synced from. The // minting overload sets stateRef itself; the handle overload cannot (no object // crosses), so a caller that legitimately holds the object - the record-driven sync, // which arrived holding it through the object-class barrier-pulled rows - notes it - // here. It is what lets a later handle-only resolution (P5c's named blit) reach the - // frontend object the twin's sync body still walks, without probing the client's slot - // allocator (T2). Same liveness rules as the minted stateRef: never an identity test, - // never read to decide the slot is dead. + // here. It is what lets a later handle-only resolution (P5c's named blit, and P5e's + // re-typed ForEachLive) reach the frontend object the twin's sync body still walks, + // without probing the client's slot allocator (T2). Same liveness rules as the minted + // stateRef: never an identity test, never read to decide the slot is dead. + // + // P5e (id): the pair is now compiled under MOBILEGL_PIPE_PUSH rather than + // MOBILEGL_BUILD_DISAGGREGATED, because ForEachLive's caller needs it in the + // push-monolith build too, where the note is simply always present. And each half is a + // NAMED FATAL from an unbarriered apply (§4.4): they answer from a frontend SharedPtr + // that only the client's wait pins, so without that wait the object they hand back may + // already be the client's next one. Neither touches the allocator, so the allocator + // guard never sees them - this is their own refusal. void NoteStateForHandle(MG_Pipe::MGPipeHandle handle, const StatePtr& stateObj) { - if (MG_Pipe::MGPipeHandleIsNull(handle) || handle.Slot >= m_slots.size()) return; - Entry& entry = m_slots[handle.Slot]; - if (!entry.Live || entry.Gen != handle.Gen) return; - entry.stateRef = stateObj; +#if MOBILEGL_BUILD_DISAGGREGATED + MG_Pipe::MGPipeRefuseFrontendKeyedRegistryFromUnbarrieredApply("NoteStateForHandle"); +#endif + if (MG_Pipe::MGPipeHandleIsNull(handle)) return; + Entry* const entry = EntryOrNull(handle.Slot); + if (entry == nullptr || !entry->Live || entry->Gen != handle.Gen) return; + entry->stateRef = stateObj; } // The frontend object noted for this handle, or null. The handle answers identity; // this answers only "which object was this twin last synced from". StatePtr StateForHandle(MG_Pipe::MGPipeHandle handle) const { - if (MG_Pipe::MGPipeHandleIsNull(handle) || handle.Slot >= m_slots.size()) { - return nullptr; - } - const Entry& entry = m_slots[handle.Slot]; - if (!entry.Live || entry.Gen != handle.Gen) return nullptr; - return entry.stateRef.lock(); - } +#if MOBILEGL_BUILD_DISAGGREGATED + MG_Pipe::MGPipeRefuseFrontendKeyedRegistryFromUnbarrieredApply("StateForHandle"); #endif + if (MG_Pipe::MGPipeHandleIsNull(handle)) return nullptr; + const Entry* const entry = EntryOrNull(handle.Slot); + if (entry == nullptr || !entry->Live || entry->Gen != handle.Gen) return nullptr; + return entry->stateRef.lock(); + } // P3a: the death half of the overload above, for a kind whose announcement is its own // destroy CALL rather than the shared death notice (D-L). Hands the twin OUT rather @@ -394,13 +461,12 @@ namespace MobileGL::MG_Backend::DirectGLES { BackendPtr ReleaseByHandle(MG_Pipe::MGPipeHandle handle) { if (MG_Pipe::MGPipeHandleIsNull(handle)) return BackendPtr{}; if (m_memoHandle.Slot == handle.Slot) ForgetHandle(); - if (handle.Slot >= m_slots.size()) return BackendPtr{}; - Entry& entry = m_slots[handle.Slot]; - if (!entry.Live || entry.Gen != handle.Gen) return BackendPtr{}; - BackendPtr dead = std::move(entry.backend); - entry.backend.reset(); - entry.stateRef.reset(); - entry.Live = false; + Entry* const entry = EntryOrNull(handle.Slot); + if (entry == nullptr || !entry->Live || entry->Gen != handle.Gen) return BackendPtr{}; + BackendPtr dead = std::move(entry->backend); + entry->backend.reset(); + entry->stateRef.reset(); + entry->Live = false; return dead; } @@ -419,10 +485,9 @@ namespace MobileGL::MG_Backend::DirectGLES { BackendPtr* FindByHandle(MG_Pipe::MGPipeHandle handle) { if (MG_Pipe::MGPipeHandleIsNull(handle)) return nullptr; - if (handle.Slot >= m_slots.size()) return nullptr; - Entry& entry = m_slots[handle.Slot]; - if (!entry.Live || entry.Gen != handle.Gen) return nullptr; - return &entry.backend; + Entry* const entry = EntryOrNull(handle.Slot); + if (entry == nullptr || !entry->Live || entry->Gen != handle.Gen) return nullptr; + return &entry->backend; } // The handle this object's twin is keyed on, or the null handle. This is what a backend @@ -438,6 +503,9 @@ namespace MobileGL::MG_Backend::DirectGLES { #if MOBILEGL_BUILD_DISAGGREGATED // P5c (hd): same guard as the minting overload - a lifetime-id probe from the // apply thread is Fatal{RoleViolation, "MGPipeSlots"} with an active transport. + // P5e (id, §4.4): and now also whenever the record being applied is UNBARRIERED, + // scope or no scope. This member and Find(StateObject*) below it are the two the + // per-family packages retire by passing the handle the record carried instead. MG_Pipe::MGPipeRefuseAllocatorFromApplyThread("HandleOf"); #endif const Uint64 lifetimeId = stateObj->GetLifetimeId(); @@ -517,11 +585,23 @@ namespace MobileGL::MG_Backend::DirectGLES { return count; } - // fn(const StatePtr& state, const BackendPtr& twin) over every live, still-owned entry. - // Replaces the registry's begin()/end(), whose iterator exposed the raw frontend - // address as the map key - the one place the backend read an identity it must not have. - // The state object is handed over as a STRONG reference, so the callee cannot be handed - // a dangling key the way the old iteration could. + // P5e (id), CONTRACT-P5E §4.1: fn(MGPipeHandle, const BackendPtr& twin) over every + // live, twinned entry. Replaces the registry's begin()/end(), whose iterator exposed + // the raw frontend address as the map key - the one place the backend read an identity + // it must not have - and replaces P3a's own fn(StatePtr, BackendPtr), which handed a + // frontend SharedPtr OUT OF SERVER MEMORY on every step. + // + // WHAT THE CALLER LOST AND HOW IT GETS IT BACK, because the two are not the same thing. + // The walk no longer locks Entry::stateRef, so a caller that needs the frontend object + // asks StateForHandle(handle) for it - which is the same weak reference, read at one + // named site inside the scope that owns the debt, instead of at every step of an + // iteration. Coverage is UNCHANGED by construction: an entry ForEachLive used to skip + // because its stateRef did not lock is one whose StateForHandle answers null, and the + // caller skips it there instead. + // + // The handle is the entry's own {slot, gen}, band-aware: a composite ShaderCso's slot + // is reported as kMGPipeShaderCsoCompositeSlotBase + its band index, i.e. the slot the + // client minted, never the index into m_band. template void ForEachLive(Fn&& fn) const { // Index loop and a COPIED twin, not a range-for over references: fn is arbitrary @@ -529,13 +609,20 @@ namespace MobileGL::MG_Backend::DirectGLES { // invalidate both the iterator and any reference into the vector that outlives the // call. The one caller today happens not to insert; that is not a property the // walk should depend on. - for (SizeT slot = 0; slot < m_slots.size(); ++slot) { - const Entry& entry = m_slots[slot]; + for (SizeT index = 0; index < m_slots.size(); ++index) { + const Entry& entry = m_slots[index]; if (!entry.Live || !entry.backend) continue; - const StatePtr state = entry.stateRef.lock(); - if (!state) continue; const BackendPtr twin = entry.backend; - fn(state, twin); + fn(MG_Pipe::MGPipeHandle{static_cast(index), entry.Gen}, twin); + } + for (SizeT index = 0; index < m_band.size(); ++index) { + const Entry& entry = m_band[index]; + if (!entry.Live || !entry.backend) continue; + const BackendPtr twin = entry.backend; + fn(MG_Pipe::MGPipeHandle{ + static_cast(index) + MG_Pipe::kMGPipeShaderCsoCompositeSlotBase, + entry.Gen}, + twin); } } @@ -544,9 +631,29 @@ namespace MobileGL::MG_Backend::DirectGLES { for (const Entry& entry : m_slots) { if (entry.Live) ++count; } + for (const Entry& entry : m_band) { + if (entry.Live) ++count; + } return count; } + // The band's own live count, so a case can tell "the composite is twinned" from "the + // ordinary table grew to reach it" - which is the whole assertion the band exists for + // and is untestable from LiveCount alone. + Uint32 CompositeLiveCount() const { + Uint32 count = 0; + for (const Entry& entry : m_band) { + if (entry.Live) ++count; + } + return count; + } + + // How many entries each space has ALLOCATED, live or not. A leak case asserts on these + // rather than on LiveCount: growth is what the band prevents, and a dense table that + // never shrinks is invisible to a liveness count. + SizeT OrdinaryCapacityForTest() const { return m_slots.size(); } + SizeT CompositeCapacityForTest() const { return m_band.size(); } + private: // Drop the twin at `handle` if THIS table holds it. Frees nothing: the slot belongs to // the kind, not to the table, and OnFrontendObjectDestroyed returns it once, after @@ -556,31 +663,61 @@ namespace MobileGL::MG_Backend::DirectGLES { // there: a memo can be a handle learned from the allocator for an object another // holder twinned, and it must not survive the slot's next handout. if (m_memoHandle.Slot == handle.Slot) ForgetHandle(); - if (handle.Slot >= m_slots.size()) return false; // The twin's destructor is a driver call and could, in principle, re-enter // GetOrCreate on this table and resize m_slots. So NOTHING that outlives the // destructor may be a reference into m_slots: the twin is moved out into a local, // the entry is finished with, and only then is the local released. BackendPtr dead; { - Entry& entry = m_slots[handle.Slot]; - if (!entry.Live || entry.Gen != handle.Gen) return false; - dead = std::move(entry.backend); - entry.backend.reset(); - entry.stateRef.reset(); - entry.Live = false; + Entry* const entry = EntryOrNull(handle.Slot); + if (entry == nullptr || !entry->Live || entry->Gen != handle.Gen) return false; + dead = std::move(entry->backend); + entry->backend.reset(); + entry->stateRef.reset(); + entry->Live = false; } dead.reset(); return true; } - // Grows the table to hold `slot`. Every caller bounds `slot` first - the minting + // ---- P5e (id): the two slot spaces, resolved in ONE place --------------------------- + // + // Every reader and writer below goes through these three, so a caller that forgot the + // band cannot exist - the shape MGPipeSlotAllocator::EntryOf already uses one level + // out. kHasCompositeBand folds to `false` at compile time for the five non-ShaderCso + // instantiations, so their band is dead code and an empty Vector. + static constexpr Bool SlotIsBanded(Uint32 slot) { + return kHasCompositeBand && MG_Pipe::MGPipeIsCompositeShaderSlot(slot); + } + static constexpr SizeT IndexOfSlot(Uint32 slot) { + return SlotIsBanded(slot) + ? static_cast(slot - MG_Pipe::kMGPipeShaderCsoCompositeSlotBase) + : static_cast(slot); + } + + // The entry a slot names, or null when its space has never grown that far. NEVER grows: + // a lookup that resized would turn every miss into an allocation, which is the hazard + // Find documents. + Entry* EntryOrNull(Uint32 slot) { + Vector& table = SlotIsBanded(slot) ? m_band : m_slots; + const SizeT index = IndexOfSlot(slot); + if (index >= table.size()) return nullptr; + return &table[index]; + } + const Entry* EntryOrNull(Uint32 slot) const { + return const_cast(this)->EntryOrNull(slot); + } + + // Grows the right space to hold `slot`. Every caller bounds `slot` first - the minting // overload because the allocator produced it, the handle overload against // kMaxHandleSlot - because this is the one place a client-supplied number decides an - // allocation size. + // allocation size. A composite slot grows m_band by (slot - base) + 1, so the ordinary + // table never learns the band exists. Entry& EntryAt(Uint32 slot) { - if (slot >= m_slots.size()) m_slots.resize(static_cast(slot) + 1); - return m_slots[slot]; + Vector& table = SlotIsBanded(slot) ? m_band : m_slots; + const SizeT index = IndexOfSlot(slot); + if (index >= table.size()) table.resize(index + 1); + return table[index]; } void RememberHandle(Uint64 lifetimeId, MG_Pipe::MGPipeHandle handle) const { @@ -621,6 +758,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // Indexed by MGPipeHandle::Slot; [0] is the reserved slot and is never live. Vector m_slots; + // P5e (id): the ShaderCso COMPOSITE band, indexed by (slot - + // kMGPipeShaderCsoCompositeSlotBase) and EMPTY for every other kind. See + // kHasCompositeBand above for why it is a second vector and not more of the first. + Vector m_band; // Handed back by GetOrCreate for a null state object. Never live, never handed a handle. BackendPtr m_nullTwin; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index e40317d0..b019c180 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -4551,7 +4551,12 @@ void main() { // delete, so the scope admits the probe as named debt rather than letting the guard // Fatal at server teardown. P7 gives these resources storage that is not a frontend // object. - const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; + // + // P5e (id), ruling 12: one of Magma's FOUR apply-thread allocator debts, and the scope + // is now named after that debt rather than after the frontend-keyed registry - Espryt's + // half of which P5e is retiring, while this one waits for P7. The exemption is keyed on + // a DirectVulkan server, which is what this file always is. + const MG_Pipe::MagmaP7AllocatorDebtScope magmaP7AllocatorDebt; #endif m_blitResources = {}; } @@ -4630,8 +4635,9 @@ void main() { #if MOBILEGL_BUILD_DISAGGREGATED // Same shape as ShutdownBlitResources above: the hidden depth-mipmap program is a // frontend object created and destroyed by the server backend on the apply thread, and - // its destructor's lifetime-id probe is admitted here as named debt. - const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; + // its destructor's lifetime-id probe is admitted here as named debt - Magma's, P7's to + // retire, which is what the scope's P5e name says (ruling 12). + const MG_Pipe::MagmaP7AllocatorDebtScope magmaP7AllocatorDebt; #endif m_depthMipmapResources = {}; } @@ -8974,8 +8980,11 @@ void main() { // the resolution here is frontend-keyed - the handle was minted over the frontend // object's lifetime id, and the two probes below (the client allocator's slot entry // and the frontend context's framebuffer pool) are named debt inside the scope, the - // same shape as Espryt's StateForHandle arm: P3b/P4b retire it by carrying the object - // identity in the record. + // same shape as Espryt's StateForHandle arm: P7 retires it by carrying the object + // identity in the record. P5e (id), ruling 12: the third of Magma's four debts, so the + // scope it rides in is MagmaP7AllocatorDebtScope. Magma stays lockstep for the whole of + // P5e (CONTRACT-P5E §6.1), so the record being applied here is always barriered and the + // exemption still holds. if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { auto& applierState = MG_Pipe::MGPipeApplier(); const MG_Pipe::MGPipeHandle readHandle = applierState.VerbBlitReadFbo; @@ -8983,7 +8992,7 @@ void main() { if (!MG_Pipe::MGPipeHandleIsNull(readHandle) || !MG_Pipe::MGPipeHandleIsNull(drawHandle)) { const auto resolveEndpoint = [](MG_Pipe::MGPipeHandle handle) -> SharedPtr { - const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; + const MG_Pipe::MagmaP7AllocatorDebtScope magmaP7AllocatorDebt; if (handle == MG_Pipe::kMGPipeDefaultFramebuffer) { return MG_State::pGLContext ? MG_State::pGLContext->GetFramebufferObject(0) : nullptr; } diff --git a/MobileGL/MG_Impl/Pipe/ResourceTracker.h b/MobileGL/MG_Impl/Pipe/ResourceTracker.h index 48f3b4e6..d6870ea1 100644 --- a/MobileGL/MG_Impl/Pipe/ResourceTracker.h +++ b/MobileGL/MG_Impl/Pipe/ResourceTracker.h @@ -717,10 +717,15 @@ namespace MobileGL::MG_Pipe { const MGPipeHandle res = [&]() { #if MOBILEGL_BUILD_DISAGGREGATED // CONTRACT-P5C §3.1's named exemption, Magma's half: the binding records that - // would carry this buffer's handle are P4b's to emit and Magma's server-side + // would carry this buffer's handle are sb's to emit and Magma's server-side // binding table is P7's, so until then the probe runs inside the scope - the // debt's named, greppable form rather than a silent guard removal. - const MGPipeReverseAnnouncementScope reverseAnnouncement; + // + // P5e (id), ruling 12: the FOURTH of Magma's apply-thread allocator debts and the + // last user of the scope P5e renamed. Espryt never reaches this arm under a + // transport - MarkBufferGpuWrittenByHandle carries the handle from the record side + // (sb/fb) - so the DirectVulkan key on the exemption costs it nothing. + const MagmaP7AllocatorDebtScope magmaP7AllocatorDebt; #endif return MGPipeSlots().FindByLifetimeId(MGPipeKind::Buffer, bufferObject->GetLifetimeId()); }(); diff --git a/MobileGL/MG_Impl/Pipe/SlotAllocator.cpp b/MobileGL/MG_Impl/Pipe/SlotAllocator.cpp index 5661263a..f2b74952 100644 --- a/MobileGL/MG_Impl/Pipe/SlotAllocator.cpp +++ b/MobileGL/MG_Impl/Pipe/SlotAllocator.cpp @@ -11,6 +11,10 @@ #if MOBILEGL_BUILD_DISAGGREGATED #include +// P5e (id): the guard's exemption is keyed on whether the record being applied is BARRIERED +// (CONTRACT-P5E §4.4), and MGPipeApplierCurrentRecordIsBarriered() is where ApplyOne stamps +// that. Declarations only - this file names no applier state. +#include #include #include @@ -19,20 +23,64 @@ namespace MobileGL::MG_Pipe { #if MOBILEGL_BUILD_DISAGGREGATED + Bool MGPipeApplierIsUnbarrieredApply() { + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return false; + if (!MG_Remote::Server::ServerLoop::OnApplyThread()) return false; + return !MGPipeApplierCurrentRecordIsBarriered(); + } + void MGPipeRefuseAllocatorFromApplyThread(const char* entry) { if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; if (!MG_Remote::Server::ServerLoop::OnApplyThread()) return; - // The named exemption of CONTRACT-P5C §3.1: the sites whose handle-carrying records - // are not emitted yet (P4b/P7) probe read-only inside the scope. The G6 - // frontend-keyed registry family (P3b/P4b) does the same inside its own scope. - if (MGPipeReverseAnnouncementScope::ActiveOnApplyThread()) return; - if (MGPipeFrontendKeyedRegistryScope::ActiveOnApplyThread()) return; + // CONTRACT-P5E §4.4 AMENDS CONTRACT-P5C §3.1 HERE, AND IT IS THE WHOLE OF WHAT id + // CHANGES ABOUT THIS FUNCTION: a named exemption is a debt the CLIENT'S WAIT pays for. + // Behind a barriered record the client is parked in WaitForApplied and its allocator + // is not moving, so a read-only probe is stale-free; behind an unbarriered one it is + // running ahead, and the same probe reads a free list and a lifetimeId -> slot map the + // client is concurrently mutating. So the two scopes below exempt nothing at all once + // the record is unbarriered - regardless of MOBILEGL_IPC_STRICT_ERRORS, because the + // value would be wrong by construction and there is no "count it" arm for that + // (CONTRACT-P5E, rule F). + // + // Every record is barriered until ra lands the wait rule, so this reads exactly as it + // did at 2fde7034 for the whole of this phase. + if (MGPipeApplierCurrentRecordIsBarriered()) { + // Ruling 12: Magma's four P7 debts, and ONLY on a DirectVulkan server. The key is + // on the backend kind rather than on the site because the scope is a class anyone + // can construct, and an Espryt probe borrowing Magma's exemption is exactly the + // hole P5e is closing. + if (MagmaP7AllocatorDebtScope::ActiveOnApplyThread() && + MG_Config::ActiveBackendType == BackendType::DirectVulkan) { + return; + } + // The G6 frontend-keyed registry family: the barriered-row sites the per-family + // packages have not carried a handle to yet (CONTRACT-P5E §4.4). + if (MGPipeFrontendKeyedRegistryScope::ActiveOnApplyThread()) return; + } MGLOG_F("MGPipe: Fatal{RoleViolation, \"MGPipeSlots\"} - the apply thread called " "MGPipeSlots().%s. With an active transport the client slot allocator is " - "client-only memory (CONTRACT-P5C §3.1, rule E): a handle arrives already " - "minted in a record, and a server that resolves or mints one off a frontend " - "object's lifetime id is reading memory that will not exist on its side of a " - "real split", + "client-only memory (CONTRACT-P5C §3.1, rule E; CONTRACT-P5E §4.4): a handle " + "arrives already minted in a record, and a server that resolves or mints one " + "off a frontend object's lifetime id is reading memory that will not exist on " + "its side of a real split. A named exemption scope admits it only while the " + "record being applied is BARRIERED (barriered=%d) and, for Magma's P7 debt, " + "only on a DirectVulkan server", + entry, MGPipeApplierCurrentRecordIsBarriered() ? 1 : 0); + std::abort(); + } + + // P5e (id): the non-allocator half of the same rule. SlotTables.h's state note, its reader + // and ForEachLive's weak-reference walk read server memory KEYED BY FRONTEND IDENTITY and + // hand a frontend SharedPtr back, which is the same violation one step removed - no + // allocator call, so the guard above never sees it. + void MGPipeRefuseFrontendKeyedRegistryFromUnbarrieredApply(const char* entry) { + if (!MGPipeApplierIsUnbarrieredApply()) return; + MGLOG_F("MGPipe: Fatal{RoleViolation, \"MGPipeSlots\"} - the apply thread reached " + "BackendSlotTable::%s while applying an UNBARRIERED record. The frontend-keyed " + "half of the twin table is monolith glue (CONTRACT-P5E §4.1, §5.8): it answers " + "from a frontend object the server has no wait pinning, so the SharedPtr it " + "would hand back may already be the client's next object. Resolve the twin " + "from the handle the record carried instead", entry); std::abort(); } @@ -54,8 +102,10 @@ namespace MobileGL::MG_Pipe { // WHY IT WAS WORTH DOING. A shared-library thread_local costs an __emutls_get_address // call per access. On the MONOLITH's GL thread that symbol is the TOP entry of the // profile at 7.7%, and these two scopes' ctor/dtor are 32.7% of its samples - // (MGPipeFrontendKeyedRegistryScope 18.75 ctor + 13.95 dtor, MGPipeReverseAnnouncement - // 8.4); on the split's apply thread emutls is 7.4%. The scope is constructed at ~20 + // (MGPipeFrontendKeyedRegistryScope 18.75 ctor + 13.95 dtor, the scope P5e renamed + // MagmaP7AllocatorDebtScope 8.4); on the split's apply thread emutls is 7.4%. The + // measurement predates the rename and the numbers are quoted as measured. The scope is + // constructed at ~20 // sites in DirectGLES.cpp, several inside StateBackendObjectRegistry::HandleOf, which // is per-draw. The cost on the GL thread is now one inlined predicate per end. // @@ -65,23 +115,27 @@ namespace MobileGL::MG_Pipe { // meaning cannot ask for it by accident: whoever wants "is a scope open on THIS // thread" has to add it, and move the counting back, rather than read a false that // looks like an answer. - Uint32 g_reverseAnnouncementScopeDepth = 0; + Uint32 g_magmaP7AllocatorDebtScopeDepth = 0; } - MGPipeReverseAnnouncementScope::MGPipeReverseAnnouncementScope() + MagmaP7AllocatorDebtScope::MagmaP7AllocatorDebtScope() // Decided ONCE and remembered: the destructor must undo exactly what the constructor // did, and re-asking the predicate would leak a count across a scope that straddled // the apply thread's exit block. : m_counted(MG_Remote::Server::ServerLoop::OnApplyThread()) { - if (m_counted) ++g_reverseAnnouncementScopeDepth; + if (m_counted) ++g_magmaP7AllocatorDebtScopeDepth; } - MGPipeReverseAnnouncementScope::~MGPipeReverseAnnouncementScope() { - if (m_counted) --g_reverseAnnouncementScopeDepth; + MagmaP7AllocatorDebtScope::~MagmaP7AllocatorDebtScope() { + if (m_counted) --g_magmaP7AllocatorDebtScopeDepth; } - Bool MGPipeReverseAnnouncementScope::ActiveOnApplyThread() { - return g_reverseAnnouncementScopeDepth != 0; + // The DEPTH is not backend-keyed and deliberately so: the backend kind is read by the + // GUARD, once, at the moment it decides. Counting only on DirectVulkan would make the + // depth's meaning depend on a global that a test can move between the constructor and the + // destructor, which is the m_counted bug one level out. + Bool MagmaP7AllocatorDebtScope::ActiveOnApplyThread() { + return g_magmaP7AllocatorDebtScopeDepth != 0; } namespace { diff --git a/MobileGL/MG_Impl/Pipe/SlotAllocator.h b/MobileGL/MG_Impl/Pipe/SlotAllocator.h index 26918fab..8c028ae1 100644 --- a/MobileGL/MG_Impl/Pipe/SlotAllocator.h +++ b/MobileGL/MG_Impl/Pipe/SlotAllocator.h @@ -173,14 +173,37 @@ namespace MobileGL::MG_Pipe { // pull build's bytes do not move (G1). void MGPipeRefuseAllocatorFromApplyThread(const char* entry); - // The NAMED EXEMPTION to the rule above (CONTRACT-P5C §3.1): the family of sites whose - // handle-carrying records the client does not EMIT yet. set_shader_buffers and - // set_stream_output_targets exist in the catalogue but are P4b's to emit - // (SetHashSuppressor.h says so), so the buffer binding-point ensures and the GPU-written - // announcement they feed have no record handle to resolve from today. Inside this scope - // ONE read-only lifetime-id probe stays legal; the scope is the debt's measurable, - // greppable form, and it retires with P4b's emission (Espryt) and P7's server-side - // binding table (Magma). Every other apply-thread allocator access stays Fatal. + // ---- P5e (id, CONTRACT-P5E §4.4): THE CONDITION UNDER WHICH A NAMED SCOPE EXEMPTS ----- + // + // "This thread is applying a record the client is NOT parked behind." Both scopes below + // exempt a probe only while it is FALSE, because the client's wait is the whole of what + // makes a frontend read from the apply thread safe: behind a barriered record the client + // is blocked in WaitForApplied and its memory is stable; behind an unbarriered one it is + // running ahead and the same read is torn or stale BY CONSTRUCTION, which is why rule F + // has no "count it" arm and this answer does not consult MOBILEGL_IPC_STRICT_ERRORS. + // + // False in a monolith process and on any thread but the apply thread, so a GL-thread + // caller is never refused by it. + Bool MGPipeApplierIsUnbarrieredApply(); + + // The refusal for the frontend-keyed surfaces that do NOT touch the allocator and so are + // not covered by MGPipeRefuseAllocatorFromApplyThread: the state note, its reader, and + // ForEachLive's weak-reference walk (SlotTables.h). They survive P5e as MONOLITH GLUE + // (CONTRACT-P5E §5.8) and this is what keeps that claim honest - reaching one of them from + // an unbarriered apply is Fatal{RoleViolation, "MGPipeSlots"} naming the member, not a + // silently stale SharedPtr. Same name in the refusal as the allocator guard's on purpose: + // it is one surface, "server memory keyed by frontend identity", and a reader chasing the + // abort should land on the same rule either way. + void MGPipeRefuseFrontendKeyedRegistryFromUnbarrieredApply(const char* entry); + + // The NAMED EXEMPTION to the rule above (CONTRACT-P5C §3.1, as amended by CONTRACT-P5E + // §4.4): the family of sites whose handle-carrying records the client does not EMIT yet. + // set_shader_buffers and set_stream_output_targets exist in the catalogue but are P4b/sb's + // to emit (SetHashSuppressor.h says so), so the buffer binding-point ensures and the + // GPU-written announcement they feed have no record handle to resolve from today. Inside + // this scope ONE read-only lifetime-id probe stays legal WHILE THE CURRENT RECORD IS + // BARRIERED; the scope is the debt's measurable, greppable form, and it retires with P7's + // server-side binding table. Every other apply-thread allocator access stays Fatal. // // THE DEPTH IS COUNTED ONLY ON THE APPLY THREAD (P5d round 3, package D). The counter's // only reader is MGPipeRefuseAllocatorFromApplyThread, which returns before it looks unless @@ -196,12 +219,28 @@ namespace MobileGL::MG_Pipe { // monolith process) both ends are now one inlined predicate and a branch instead of an // emutls call, which is where 32.7% of the monolith GL thread's __emutls_get_address - its // top symbol at 7.7% - was going. - class MGPipeReverseAnnouncementScope { + // + // P5e (id), ruling 12: RENAMED FROM MGPipeReverseAnnouncementScope AND KEYED ON THE + // BACKEND KIND. The rename is the finding: what is left inside it after P5e is not "the + // reverse announcement" - Espryt's half of that moved to the frontend-keyed scope with the + // rest of the registry debt - it is MAGMA'S FOUR APPLY-THREAD ALLOCATOR TOUCHES, which P7 + // retires (VulkanRenderer.cpp's two hidden-resource shutdowns and its named-blit endpoint + // resolve, plus MGPipeAnnounceBufferGpuWritten in ResourceTracker.h). Naming the scope + // after the debt rather than after one of its sites is what makes "has P7 landed yet" a + // grep; naming it after Magma is what stops a new Espryt site being wrapped in it. + // + // THE EXEMPTION IS REFUSED WHEN THE SERVER BACKEND IS NOT DirectVulkan, and that is the + // point of the key rather than a safety belt. Magma stays lockstep for the whole of P5e + // (§6.1: the DirectVulkan arm never publishes kCapRunAheadApply, so every record there is + // barriered and every probe inside this scope keeps P5C's semantics). Espryt is what P5e + // is retiring the lockstep for, so an Espryt probe must not be able to borrow Magma's + // exemption - even by wrapping itself in Magma's scope. + class MagmaP7AllocatorDebtScope { public: - MGPipeReverseAnnouncementScope(); - ~MGPipeReverseAnnouncementScope(); - MGPipeReverseAnnouncementScope(const MGPipeReverseAnnouncementScope&) = delete; - MGPipeReverseAnnouncementScope& operator=(const MGPipeReverseAnnouncementScope&) = delete; + MagmaP7AllocatorDebtScope(); + ~MagmaP7AllocatorDebtScope(); + MagmaP7AllocatorDebtScope(const MagmaP7AllocatorDebtScope&) = delete; + MagmaP7AllocatorDebtScope& operator=(const MagmaP7AllocatorDebtScope&) = delete; static Bool ActiveOnApplyThread(); private: @@ -212,11 +251,21 @@ namespace MobileGL::MG_Pipe { // registry (audit row G6). The texture / sampler-view / FBO-legacy HandleOf probes are // how the server answers "which twin is this frontend object" while the registry is // keyed by frontend identity - server-PRIVATE state whose rekey onto handles is - // P3b/P4b's, not P5c's. A probe inside this scope stays a read-only, barrier-held debt; - // wrapping a NEW site in it is the greppable act of naming that debt, and an unwrapped + // P5e's per-family packages', not P5c's. A probe inside this scope stays a read-only, + // BARRIER-HELD debt - and P5e (id) makes the "barrier-held" half literal rather than + // documentary: the exemption now holds only while MGPipeApplierCurrentRecordIsBarriered() + // (§4.4), i.e. only while the client is actually parked behind the record being applied. + // Wrapping a NEW site in it is the greppable act of naming that debt, and an unwrapped // probe from the apply thread is still Fatal{RoleViolation, "MGPipeSlots"}. // - // Apply-thread-only depth, and m_counted, for MGPipeReverseAnnouncementScope's reasons + // WHAT THE CLASS IS FOR NOW, since P5e deletes most of its sites: vi/sb/pg/tx2/fb delete + // their ~20 draw-path constructions with the probes they wrap, and what survives is the + // set of BARRIERED-ROW sites (CONTRACT-P5E §4.4: DirectGLES.cpp's CopyTex / GetTexImage / + // mipmap-shape / set_storage_block_binding / detach-walk sites, the two verify arms, and - + // until vi/sb carry the handle - BufferImpl::HandleOfBuffer). Those keep P5C's semantics + // because their records are barriered and their client IS parked. + // + // Apply-thread-only depth, and m_counted, for MagmaP7AllocatorDebtScope's reasons // above. This is the scope that pays: it is constructed at ~20 sites in DirectGLES.cpp, // several of them inside StateBackendObjectRegistry::HandleOf on the per-draw path, and // its ctor/dtor alone were 18.75% + 13.95% of the monolith GL thread's emutls samples. diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index 52f91983..777c7eee 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -1212,6 +1212,30 @@ namespace MobileGL::MG_Pipe { Uint64 MGPipeApplierContextSerial() { return g_applier.ContextSerial; } void MGPipeApplierNoteTextureStateMoved() { ++g_applier.TextureShutterSerial; } + // P5e (id), CONTRACT-P5E §2.1. THE BODY ra REPLACES, and the reason it is a constant here + // rather than absent: every caller of the predicate lands with this package, so a missing + // body would be a link error in six worktrees at once, and a body that guessed at the wait + // rule would change behaviour in the package whose whole claim is that it changes none. + // True for every record IS today's rule - the client blocks after publishing every verb + // (ClientSession.cpp's :915-959 lockstep path) - so this is a restatement, not a stub. + Bool MGPipeBarriered(MGPWireOp op, const void* payload, const MGPipeApplierState& st) { + (void)op; + (void)payload; + (void)st; + return true; + } + + // §4.4's input. Outside a disaggregated build there is no ApplyOne to stamp it and no + // client that could run ahead, so the answer is the constant the monolith has always + // behaved as. + Bool MGPipeApplierCurrentRecordIsBarriered() { +#if MOBILEGL_BUILD_DISAGGREGATED + return g_applier.CurrentRecordBarriered; +#else + return true; +#endif + } + void MGPipeSetResourceOps(const MGPipeResourceOps* ops) { g_resourceOps = ops; } const MGPipeResourceOps* MGPipeGetResourceOps() { return g_resourceOps; } diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 872d582d..a23e8fd2 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -819,6 +819,23 @@ namespace MobileGL::MG_Pipe { // dispatch_indirect's command buffer (MGPGridInfo::IndirectBuffer). MGPipeHandle VerbDispatchIndirectBuffer = kMGPipeNullHandle; + // ---- P5e (id, CONTRACT-P5E §2.1 / §4.4): IS THE RECORD BEING APPLIED BARRIERED? ---- + // + // SERVER-PRIVATE and per-record: PipeApplier::ApplyOne writes it from MGPipeBarriered() + // before the decode and nothing else may. It is what §4.4's exemption is keyed on - a + // frontend probe inside a named scope is legal only while the CLIENT IS PARKED behind + // this record, because the wait is the whole of what makes the client's memory stable + // to read. An unbarriered record's apply has no such wait, so the same probe would + // read memory the client is concurrently moving, and the scope stops exempting it. + // + // TRUE IS THE ONLY VALUE THIS PHASE PRODUCES, and that is deliberate: MGPipeBarriered + // answers true for every record until ra lands the wait rule, so id changes no + // behaviour at all (lockstep is unchanged) while every site that must one day ask the + // question already asks it. It is also the value a MONOLITH applier keeps - nothing + // writes it there and there is no client to run ahead - which is what keeps Magma and + // the push-monolith build on P5C's semantics (§6.1). + Bool CurrentRecordBarriered = true; + // Every verb's writer calls this FIRST and then sets its own fields, so no field ever // outlives the verb that wrote it and a reader can treat non-null as "this verb's". void ClearVerbHandles() { @@ -915,6 +932,38 @@ namespace MobileGL::MG_Pipe { // this, so the bump rule lives in exactly one place. void MGPipeApplierNoteTextureStateMoved(); + // ---- P5e: the two halves of the barriered predicate (CONTRACT-P5E §2.1, §4.4) --------- + // + // OWNERSHIP NOTE, so the duplicate is not mistaken for a fork: package c0e owns the final + // declaration of both names in this header. id declares them here because the identity + // guard (§4.4) cannot compile without them and the two packages are implemented in + // parallel; the integrator resolves the textual overlap at landing. + // + // MGPWireOp is declared opaquely rather than by including MGPipe.h: the enum is generated + // into generated/PipeWire.inc, which MGPipe.h includes INSIDE this namespace, and this + // header sits below it (PipeRoute.h includes both). An opaque enum declaration with the + // same fixed underlying type is the whole coupling the predicate needs. + enum class MGPWireOp : Uint16; + + // "Does the client block after publishing this record?" - a pure function of wire-visible + // data, computed identically by the emit table and by the sink (§2.1): + // + // MGPipeWaitClassFor(op) != kWaitNone // static column + // || (MGPipeCallClassFor(op) == kCtxVerb && st.IsTransformFeedbackActive) // (i) XFB + // || (op == DrawVbo && (payload.Flags & kDrawClientArrays)) // (ii) + // + // UNTIL ra LANDS THE WAIT RULE THE ANSWER IS TRUE FOR EVERY RECORD, and that is what keeps + // this package behaviour-free: the client waits exactly where it waits today, so every + // apply-thread frontend read that is legal today stays legal today. `op` and `payload` are + // therefore unread by the body id ships; they are in the signature because ra rebases onto + // it rather than onto a narrower one. + Bool MGPipeBarriered(MGPWireOp op, const void* payload, const MGPipeApplierState& st); + + // The record currently being applied, as ApplyOne stamped it. Server-private; `true` on a + // server that does not publish kCapRunAheadApply and in every monolith process, which is + // what leaves Magma (§6.1) and the push-monolith build on P5C's semantics. + Bool MGPipeApplierCurrentRecordIsBarriered(); + // A MAKE-CURRENT, NOT A TEARDOWN - and the distinction is the whole of this function's // contract. It runs on every change of the current GLContext (MGPipeTracker::Update resets // the tracker whenever the context pointer moves, and the emitter calls this from the diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 3eb09c5d..55a78e5d 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -1055,6 +1055,15 @@ namespace MobileGL::MG_Remote::Server { // dropped before this function returns, and s1's SessionConsumer::ApplyOne publishes // appliedSeq only after that - so by the time the client is runnable the flag is down. const Client::ClientSession::ScopedApplierEntry insideApplier; + // P5e (id), CONTRACT-P5E §2.1 / §4.4: STAMP WHETHER THE CLIENT IS PARKED BEHIND THIS + // RECORD, before anything can ask. It is the input to the allocator guard's exemption + // (SlotAllocator.cpp) and to every frontend-keyed twin member that survives as monolith + // glue (SlotTables.h), and it has to be up before DecodeAndApply because the sinks those + // reach are exactly the askers. MGPipeBarriered answers true for every record until ra + // lands the wait rule, so this line changes nothing this phase and is the line ra + // rebases onto rather than adds. + MG_Pipe::MGPipeApplier().CurrentRecordBarriered = MG_Pipe::MGPipeBarriered( + static_cast(record.kind), record.payload, MG_Pipe::MGPipeApplier()); // ORDER IS THE CONTRACT'S: stamp, then apply. The stamp is what makes any server-side // read of gPipeInputs legal at all (PipeApplier.h's block 1), so a record applied // before it aborts on the FIRST field inside SyncRenderState. diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 98da37b5..80beaa1a 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -3410,6 +3410,12 @@ namespace { BackendSlotTable; using FakeSharedKindSlotTable = MobileGL::MG_Backend::DirectGLES:: BackendSlotTable; + // P5e (id): ShaderCso is the ONE kind with a composite band, so the band case needs a table + // of that kind. It never touches MGPipeSlots() - the whole case runs through the by-handle + // overloads - and the holder list is per TABLE TYPE, so this instantiation shares nothing + // with the real g_backendProgramObjects. + using FakeShaderCsoSlotTable = MobileGL::MG_Backend::DirectGLES:: + BackendSlotTable; // A log file path no other process and no other case can be writing to: the pid keeps two // SanityTest processes on one host apart, the counter keeps two cases in one process apart. @@ -4932,6 +4938,72 @@ TEST(DirectGLESSlotTable, AGenerationBehindTheLiveTwinIsRefusedRatherThanAdopted EXPECT_EQ(table.FindByHandle(MG_Pipe::MGPipeHandle{FakeSlotTable::kMaxHandleSlot, 1u}), nullptr); } +// P5e (id), CONTRACT-P5E §4.3's last paragraph: THE COMPOSITE BAND, and why it is a P5e +// prerequisite rather than a follow-up. +// +// A program-pipeline composite's ShaderCso slot comes out of the reserved top 1/16 of the slot +// space (kMGPipeShaderCsoCompositeSlotBase = 983040). EntryAt indexes BY SLOT and resizes to +// it, so before the band a single composite grew g_backendProgramObjects to ~983k entries of +// ~40 B - ~40 MB for one glBindProgramPipeline. That was reachable only through +// GetOrCreate(StatePtr) for a composite program; after id's rekey the by-handle resolution IS +// the ordinary path (ResolveProgramTwin), so every composite bind would pay it. +// +// THE RED: make EntryAt index m_slots unconditionally again (drop the SlotIsBanded branch) and +// the first assertion below goes from 0 to 983044. LiveCount alone could not see that - the +// table would hold exactly one live entry either way - which is why the case asserts on the +// two spaces' CAPACITIES and not on liveness. +TEST(DirectGLESSlotTable, ACompositeHandleDoesNotGrowTheOrdinaryTable) { + using namespace MobileGL; + using namespace MobileGL::MG_Backend::DirectGLES; + + FakeShaderCsoSlotTable table; + const MG_Pipe::MGPipeHandle composite{MG_Pipe::kMGPipeShaderCsoCompositeSlotBase + 3u, 1u}; + ASSERT_TRUE(MG_Pipe::MGPipeIsCompositeShaderSlot(composite.Slot)); + + auto& twin = table.GetOrCreate(composite); + twin = MakeShared(); + twin->marker = 0x5A; + + EXPECT_EQ(table.OrdinaryCapacityForTest(), 0u) + << "a composite slot grew the ORDINARY table - one program pipeline is ~40 MB of twin " + "entries for a single live program"; + EXPECT_EQ(table.CompositeCapacityForTest(), 4u) + << "the band is indexed by (slot - base), so slot base+3 needs exactly four entries"; + EXPECT_EQ(table.LiveCount(), 1u); + EXPECT_EQ(table.CompositeLiveCount(), 1u) << "the live composite was counted in the wrong space"; + + auto* const found = table.FindByHandle(composite); + ASSERT_NE(found, nullptr) << "the band entry is invisible to the lookup that has to find it"; + EXPECT_EQ((*found)->marker, 0x5A); + + // An ordinary ShaderCso lands in the ordinary space and disturbs neither the band's + // contents nor its size: the two are dense against their OWN high-water marks. + auto& ordinary = table.GetOrCreate(MG_Pipe::MGPipeHandle{5u, 1u}); + ordinary = MakeShared(); + EXPECT_EQ(table.OrdinaryCapacityForTest(), 6u); + EXPECT_EQ(table.CompositeCapacityForTest(), 4u); + EXPECT_EQ(table.CompositeLiveCount(), 1u); + + // ForEachLive reports the composite at THE SLOT THE CLIENT MINTED, never the band index - + // the handle it hands over is the one a caller turns straight back into a record lookup. + Vector walked; + table.ForEachLive([&](MG_Pipe::MGPipeHandle handle, const SharedPtr&) { + walked.push_back(handle.Slot); + }); + ASSERT_EQ(walked.size(), 2u); + EXPECT_EQ(walked[0], 5u) << "the ordinary space is walked first, at its own slot"; + EXPECT_EQ(walked[1], composite.Slot) + << "the band was reported at its INDEX rather than at the client's slot, so a caller " + "resolving the record for it would read another program's"; + + // §4.3's recycle answer holds inside the band exactly as it does outside it. + auto& recycled = table.GetOrCreate(MG_Pipe::MGPipeHandle{composite.Slot, 2u}); + EXPECT_EQ(recycled, nullptr) << "a successor at a recycled composite slot inherited its " + "predecessor's driver program"; + EXPECT_EQ(table.FindByHandle(composite), nullptr) << "the predecessor's handle still resolves"; + EXPECT_EQ(table.CompositeCapacityForTest(), 4u) << "the recycle re-grew the band"; +} + #else // G2 wants the pull and the push build to list the SAME ctest entries. The twin table only // exists under MOBILEGL_PIPE_PUSH, so in the pull build each case above keeps its name and @@ -5012,6 +5084,10 @@ TEST(DirectGLESSlotTable, AGenerationBehindTheLiveTwinIsRefusedRatherThanAdopted GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; } +TEST(DirectGLESSlotTable, ACompositeHandleDoesNotGrowTheOrdinaryTable) { + GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; +} + TEST(DirectGLESSlotTable, ADeathNoticeForEveryP4aKindIsIdempotent) { GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; } diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index cba77bd5..301bd552 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -1858,9 +1858,18 @@ MGL_BUFFER_GUARD_TEST(BufferHasDefinedContentFromTheApplyThreadIsFatalByName, 70 // exempting the apply thread, which is exactly the leak the thread_local used to prevent; // (b) drop the increment altogether and (a) goes red - the exemption stops exempting and // the debt's own sites (DirectGLES' HandleOf probes) abort the server; -// (c) drop the guard's `MGPipeReverseAnnouncementScope::ActiveOnApplyThread()` / +// (c) drop the guard's `MagmaP7AllocatorDebtScope::ActiveOnApplyThread()` / // `FrontendKeyedRegistryScope::ActiveOnApplyThread()` rows and (b) - the control - stops // being the only aborting arm. +// +// P5e (id), CONTRACT-P5E §4.4 REPLACED (b). The case that used to sit there - +// `AnAllocatorProbeInsideAnExemptionScopeOnTheApplyThreadIsAllowed` - asserted that a scope +// exempts UNCONDITIONALLY, which is exactly the rule §4.4 retires: an exemption is a debt the +// CLIENT'S WAIT pays for, so it holds only while the record being applied is barriered. Its +// replacement below drives the same scope with the barriered flag forced false and expects the +// abort. The green half of the old case did not need a test of its own: every Espryt +// `integration-split` entry runs the frontend-keyed scope's surviving sites on the apply thread +// behind barriered records, so a guard that stopped exempting would take the whole lane down. TEST(RemoteGuards, AnAllocatorProbeFromTheApplyThreadOutsideEveryScopeIsFatalByName) { const auto child = RunInChild([] { StartControlSession(); @@ -1875,21 +1884,47 @@ TEST(RemoteGuards, AnAllocatorProbeFromTheApplyThreadOutsideEveryScopeIsFatalByN ExpectNamedAbort(child, "Fatal{RoleViolation, \"MGPipeSlots\"}"); } -TEST(RemoteGuards, AnAllocatorProbeInsideAnExemptionScopeOnTheApplyThreadIsAllowed) { +// P5e (id) RED-ONCE, CONTRACT-P5E §4.4. The exemption scope is OPEN and the probe is the one +// the debt is named for - `HandleOf`, the frontend-keyed registry's own entry - and it still +// aborts, because the record being applied is unbarriered: the client is running ahead and the +// allocator's free list and lifetimeId -> slot map are moving under the read. +// +// THE HOOK IS THE APPLIER FIELD ITSELF and that is deliberate: `CurrentRecordBarriered` is what +// PipeApplier::ApplyOne stamps from MGPipeBarriered() on every record, so forcing it here drives +// the production path rather than a test-only branch beside it. Until ra lands the wait rule +// MGPipeBarriered answers true for every record, so this is the ONLY way to reach the arm - and +// the arm has to exist before ra, because ra is what makes it reachable in production. +// +// The red: put §4.4's `if (MGPipeApplierCurrentRecordIsBarriered())` back to an unconditional +// exemption in MGPipeRefuseAllocatorFromApplyThread and this case stops aborting - red by +// expectation, which is the shape a guard case fails in. +// EVERYTHING BUT THE PROBE RUNS ON THE CLIENT THREAD, which is the buffer/texture guard cases' +// shape one block down and is load-bearing here rather than tidy: a TextureObject2D's +// constructor MINTS a resource handle, and building it on the apply thread would abort at the +// mint with the same diagnostic - a case that passes for a reason other than the one it names. +TEST(RemoteGuards, AnAllocatorProbeInsideAnExemptionScopeFromTheApplyThreadIsFatalEvenAtTheOldDebtSites) { + struct Probe { + TestTextureTable* table; + MG_State::GLState::TextureObject2D* texture; + }; const auto child = RunInChild([] { StartControlSession(); + MG_State::GLState::TextureObject2D texture(46); + TestTextureTable table; + Probe probe{&table, &texture}; Srv::ServerLoopInstance().RunOnApplyThread( - +[](void*) -> MobileGLResult { + +[](void* self) -> MobileGLResult { + auto& probe = *static_cast(self); + // The record being applied is one the client did NOT park behind. + MG_Pipe::MGPipeApplier().CurrentRecordBarriered = false; const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; - MG_Pipe::MGPipeRefuseAllocatorFromApplyThread("FindByLifetimeId"); - const MG_Pipe::MGPipeReverseAnnouncementScope reverseAnnouncement; - MG_Pipe::MGPipeRefuseAllocatorFromApplyThread("Acquire"); + probe.table->HandleOf(probe.texture); return MOBILEGL_OK; }, - nullptr); + &probe); ClientSessionInstance().Stop(); }); - ExpectChildSuccess(child); + ExpectNamedAbort(child, "Fatal{RoleViolation, \"MGPipeSlots\"}"); } // THE ONE THAT PINS THE SINGLE-WRITER ARGUMENT. The scope is open on the GL thread for the