From 15f9b27a425c98a6927ed2bedd61334ba6173008 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 2 Oct 2026 10:35:44 -0400 Subject: [PATCH] [Fix] (P14): publish a cached sampler CSO into each share group that names it --- MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp | 21 +++- MobileGL/MG_Impl/EGLImpl/EGLImpl.h | 5 + MobileGL/MG_Impl/Pipe/SamplerEmit.h | 93 +++++++++++++++++- MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp | 113 ++++++++++++++++++++++ MobileGL/MG_Test/Pipe/TextureEmitTest.cpp | 87 +++++++++++++++++ 5 files changed, 316 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp b/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp index 95e06b6b..786f5985 100644 --- a/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp +++ b/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp @@ -82,6 +82,10 @@ namespace MobileGL::MG_Impl::EGLImpl { // may have moved it since this one last bound. The caller holds EGLOperationMutex. thread_local Uint64 t_threadContextToken = 0; Uint64 g_boundContextToken = 0; // guarded by EGLOperationMutex + // P14: the share groups of the two tokens above, kept beside them so the client can say + // which group's object records a record written now lands in (StreamBoundShareGroupToken). + thread_local Uint64 t_threadShareGroupToken = 0; + Uint64 g_boundShareGroupToken = 0; // guarded by EGLOperationMutex void EmitContextBinding(EGLStateContext* state, EGLContext switched, Uint64 previousToken) { (void)previousToken; @@ -91,6 +95,7 @@ namespace MobileGL::MG_Impl::EGLImpl { // not a slow path. if (MG_Remote::Client::RunsAsTheServerRole()) return; t_threadContextToken = switched == EGL_NO_CONTEXT ? 0 : state->GetContextClientToken(switched); + t_threadShareGroupToken = switched == EGL_NO_CONTEXT ? 0 : state->GetContextShareGroupToken(switched); StreamBindCallingThreadLocked(); } #endif @@ -113,7 +118,16 @@ namespace MobileGL::MG_Impl::EGLImpl { void StreamBindCallingThreadLocked() { const Uint64 token = t_threadContextToken; if (token == g_boundContextToken) return; - if (MG_Remote::Client::EmitBindContextRecord(token)) g_boundContextToken = token; + if (MG_Remote::Client::EmitBindContextRecord(token)) { + g_boundContextToken = token; + g_boundShareGroupToken = t_threadShareGroupToken; + } + } + + Bool StreamBoundShareGroupToken(Uint64* outToken) { + if (!StreamGateActive()) return false; + *outToken = g_boundShareGroupToken; + return true; } namespace { @@ -593,7 +607,10 @@ namespace MobileGL::MG_Impl::EGLImpl { // The server drops its binding to 0 when the current context is destroyed // (ServerSession::DestroyContext's compare-and-swap); the gate mirrors it so the next // write re-binds. - if (g_boundContextToken == contextToken) g_boundContextToken = 0; + if (g_boundContextToken == contextToken) { + g_boundContextToken = 0; + g_boundShareGroupToken = 0; + } const auto emitted = MG_Remote::Client::SendDestroyContextFrame(contextToken); if (emitted == MG_Remote::Client::ContextFrameEmit::Refused) { MGLOG_E_ONCE("eglDestroyContext: the server holds no context token %llu; the " diff --git a/MobileGL/MG_Impl/EGLImpl/EGLImpl.h b/MobileGL/MG_Impl/EGLImpl/EGLImpl.h index 47ffeb2d..0d7ac22a 100644 --- a/MobileGL/MG_Impl/EGLImpl/EGLImpl.h +++ b/MobileGL/MG_Impl/EGLImpl/EGLImpl.h @@ -91,6 +91,11 @@ namespace MobileGL::MG_Impl::EGLImpl { Bool StreamGateActive(); std::recursive_mutex& StreamMutex(); void StreamBindCallingThreadLocked(); + // P14: the share-group token of the context the stream is bound to - the group whose object + // records anything written right now lands in on the server. False while the gate is inactive + // (a monolith, the server's own apply thread), where no binding crosses. Read under + // StreamMutex(), like the binding itself. + Bool StreamBoundShareGroupToken(Uint64* outToken); class GLStreamScope { public: diff --git a/MobileGL/MG_Impl/Pipe/SamplerEmit.h b/MobileGL/MG_Impl/Pipe/SamplerEmit.h index 21318fb7..e8150111 100644 --- a/MobileGL/MG_Impl/Pipe/SamplerEmit.h +++ b/MobileGL/MG_Impl/Pipe/SamplerEmit.h @@ -38,6 +38,9 @@ // // HEADER-ONLY, for the ownership reason Tracker.h and ResourceTracker.h both state. #if MOBILEGL_PIPE_PUSH +#if MOBILEGL_BUILD_DISAGGREGATED +#include +#endif #include #include #include @@ -209,6 +212,30 @@ namespace MobileGL::MG_Pipe { "a cache that cannot hold one whole emission pass would mint over capacity on " "every pass; correctness against eviction is the reference count, not this bound"); + // P14 (docs/Disaggregated/design/11-state-ownership.md): THE SHARE GROUP A RECORD EMITTED NOW + // LANDS IN. The server files object records - create_sampler_state's among them - per + // {session, share group}, under the context the stream is bound to. That is the stream's own + // answer and not the calling thread's: a GL entry point rebinds to the caller first + // (GLStreamScope), but an EGL operation does not, and eglCreateContext's birth hooks mint. + // Without a stream (a monolith, a unit case) the caller's GLContext is the answer. Group + // tokens are minted once and never reused (EGLState::CreateContext), which is what makes them + // usable as an identity; 0 is "no EGL context", the server's "no context" applier. + inline Uint64 MGPipeCurrentShareGroupToken() { +#if MOBILEGL_BUILD_DISAGGREGATED + Uint64 bound = 0; + if (MG_Impl::EGLImpl::StreamBoundShareGroupToken(&bound)) return bound; +#endif + const auto& context = MG_State::pGLContext; + if (!context) return 0; + const auto& group = context->GetShareGroup(); + return group ? group->GetToken() : 0; + } + + // How many share groups one entry remembers having published into. Forgetting one is always + // safe - the next Acquire there re-issues the create on the same handle, which the applier + // takes as a serial bump - so this bounds memory, not correctness. + inline constexpr SizeT kMGPipeSamplerCsoPublishedGroups = 8; + class MGPipeSamplerCsoCache { public: struct Counters { @@ -244,6 +271,9 @@ namespace MobileGL::MG_Pipe { // choice structurally guarantees it stays 0; it is what would say so in a shipped // build if a second caller of Evict ever appeared. Uint64 ReferencedEvictions = 0; + // create_sampler_state RE-ISSUED on an existing handle because the bound share group + // had not been sent it - see AcquireCanonical. + Uint64 GroupPublishes = 0; }; // The handle for `params`' value, AND A REFERENCE ON IT. Mints and emits @@ -376,8 +406,49 @@ namespace MobileGL::MG_Pipe { // THE CANONICAL BYTES, not the caller's. This is the memcmp's other operand and it // has to have the same deterministic padding the probe's copy has. SamplerParameters Params{}; + // THE SHARE GROUPS whose object records hold this handle's create_sampler_state + // (MGPipeCurrentShareGroupToken). The entry is process-wide but the applier's record + // is per group, so a hit from a group not listed here re-issues the create before + // the caller names the handle there. Full -> the oldest is overwritten. + Uint64 PublishedGroups[kMGPipeSamplerCsoPublishedGroups] = {}; + Uint32 PublishedGroupCount = 0; + Uint32 NextPublishedGroup = 0; }; + static Bool IsPublishedIn(const Entry& entry, Uint64 group) { + for (Uint32 i = 0; i < entry.PublishedGroupCount; ++i) { + if (entry.PublishedGroups[i] == group) return true; + } + return false; + } + + static void NotePublishedIn(Entry& entry, Uint64 group) { + if (IsPublishedIn(entry, group)) return; + if (entry.PublishedGroupCount < kMGPipeSamplerCsoPublishedGroups) { + entry.PublishedGroups[entry.PublishedGroupCount++] = group; + return; + } + entry.PublishedGroups[entry.NextPublishedGroup] = group; + entry.NextPublishedGroup = (entry.NextPublishedGroup + 1) % kMGPipeSamplerCsoPublishedGroups; + } + + // The create Mint sends, re-issued on an entry that already exists: same handle, same + // canonical bytes. + void EmitCreate(Entry& entry, Uint64& payloadBytes) { + MGPSamplerDesc desc{}; + desc.Cso = entry.Cso; + desc.Parameters.Seg = kMGHostSpanSegNone; + desc.Parameters.Offset = 0; + desc.Parameters.Size = 0; + MGPipeRouteCreateSamplerState(desc, &entry.Params); + NotePublishedIn(entry, MGPipeCurrentShareGroupToken()); + payloadBytes += sizeof(MGPSamplerDesc) + sizeof(SamplerParameters); + if (MG_Util::PipeStats::Enabled()) { + MG_Util::PipeStats::AddBytes(MG_Util::PipeStats::ByteClass::CsoBlobBytes, + sizeof(SamplerParameters)); + } + } + MGPipeHandle AcquireCanonical(Uint64 hash, const SamplerParameters& canon, Uint64& payloadBytes) { ++m_counters.Acquisitions; for (SizeT i = 0; i < m_entries.size(); ++i) { @@ -397,6 +468,17 @@ namespace MobileGL::MG_Pipe { m_entries[i].LastUsed = ++m_clock; ++m_entries[i].RefCount; ++m_counters.Hits; + // A HIT IS ONLY A HIT IN THE GROUPS THAT WERE SENT THE CREATE. Two non-shared + // contexts asking for one value get one handle, but the server holds the record + // in the first one's share group only; naming the handle from the second (a + // set_texture_params, a bind_sampler_states) resolved to nothing there and the + // texture sampled with default parameters. The re-issue lands the record in this + // group under the same handle, which is safe because the slot space is + // process-wide: another group's table holding it is wider, never aliased. + if (!IsPublishedIn(m_entries[i], MGPipeCurrentShareGroupToken())) { + EmitCreate(m_entries[i], payloadBytes); + ++m_counters.GroupPublishes; + } return m_entries[i].Cso; } return Mint(hash, canon, payloadBytes); @@ -457,6 +539,7 @@ namespace MobileGL::MG_Pipe { // call and the bytes it stores are provably the bytes the memcmp will confirm // against later. MGPipeRouteCreateSamplerState(desc, &m_entries.back().Params); + NotePublishedIn(m_entries.back(), MGPipeCurrentShareGroupToken()); // THE CREATE ACTUALLY WENT OUT, so the publication latch is taken here and nowhere // else (contract-v2 §3.1). It is what the six death helpers read, and without it a // delete_sampler_state can never go out for this kind. @@ -491,7 +574,15 @@ namespace MobileGL::MG_Pipe { // only then does the slot go back. There is no NotifyStateObjectDestroyed step // here - a content-addressed CSO has no frontend object whose death is being // announced, which is precisely why this eviction is the only death path it has. - MGPipeRouteDeleteSamplerState(handle); + // + // THE DELETE REACHES THE BOUND SHARE GROUP ONLY, so it is sent only when that group + // holds the record; a group that was never sent the create would count + // it as a refused object call. Other groups keep an unreferenced record at this slot + // (nothing may name it: the count is 0) until a create for the slot's next + // generation is published there and starts it over, or the group is torn down. + if (IsPublishedIn(m_entries[index], MGPipeCurrentShareGroupToken())) { + MGPipeRouteDeleteSamplerState(handle); + } // AND THE LATCH GOES WITH THE DELETE. This is the "an emitter that drops a record // for its own reasons calls MGPipeNoteHandleUnpublished" half of the publication // protocol (contract-v2 §3.1): the record is gone, so a death helper reaching this diff --git a/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp b/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp index 873b2092..3495a8ae 100644 --- a/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp @@ -795,6 +795,119 @@ namespace { EXPECT_EQ(Cache().GetCounters().Collisions, 0u); } + // ============================ P14: one cache, many share groups ============================ + + // The calling thread's {session, context} for the applier's key resolver - the shape the + // server's own resolver (SessionRuntime.cpp) fills in from the session's bound context. + thread_local Uint64 t_groupSession = 0; + thread_local Uint64 t_groupContext = 0; + + Bool TwoGroupKeyResolver(MGPipeApplierKey* outKey) { + if (t_groupSession == 0) return false; + outKey->SessionKey = t_groupSession; + outKey->ContextToken = t_groupContext; + return true; + } + + // Two contexts of one session in two share groups (or one, with `shared`), made current in + // turn on BOTH halves a served client keeps in step: the frontend's pGLContext and the + // server's bound context (GLStreamScope rebinds the session before a GL entry point writes). + struct ShareGroupPair { + static constexpr Uint64 kSession = 0x5A3E5C50C0000000ull; + explicit ShareGroupPair(Bool shared) { + m_previous = MG_State::pGLContext; + for (Uint64 i = 0; i < 2; ++i) { + const Uint64 group = shared ? 0x51 : 0x51 + i; + m_contexts[i] = MakeShared(MakeShared(group)); + MGPipeApplierRegisterContext(kSession, i + 1, group); + } + MGPipeSetApplierKeyResolver(&TwoGroupKeyResolver); + } + ~ShareGroupPair() { + t_groupSession = 0; + t_groupContext = 0; + MGPipeSetApplierKeyResolver(nullptr); + MGPipeApplierReleaseSession(kSession); + MG_State::pGLContext = m_previous; + } + ShareGroupPair(const ShareGroupPair&) = delete; + ShareGroupPair& operator=(const ShareGroupPair&) = delete; + + void MakeCurrent(Uint32 index) { + MG_State::pGLContext = m_contexts[index]; + t_groupSession = kSession; + t_groupContext = index + 1; + } + + SharedPtr m_previous; + SharedPtr m_contexts[2]; + }; + + // The record the applier the calling thread is bound to holds for `cso`, or null. + const MGPipeSamplerCsoRecord* BoundGroupSamplerRecord(MGPipeHandle cso) { + const auto& table = MGPipeApplier().SamplerCsos; + if (MGPipeHandleIsNull(cso) || cso.Slot >= table.size()) return nullptr; + const MGPipeSamplerCsoRecord& record = table[cso.Slot]; + return record.Live && record.Gen == cso.Gen ? &record : nullptr; + } + + // THE CACHE IS PROCESS-WIDE AND THE RECORD IS PER SHARE GROUP. A value minted while one + // non-shared context was current is a cache hit for the next one, and before the fix that hit + // handed back a handle the second group's applier had never been sent: the texture naming it + // sampled with default parameters on Magma and with none at all on Espryt. Red without the + // re-issue in AcquireCanonical: the second group has no record and GroupPublishes stays 0. + TEST(SamplerEmit, AValueMintedInOneShareGroupIsPublishedIntoTheNextGroupThatAcquiresIt) { + EmitterScope scope; + ShareGroupPair groups(/*shared=*/false); + Cache().ResetCounters(); + const SamplerParameters params = DistinctParameters(); + Uint64 payload = 0; + + groups.MakeCurrent(0); + const MGPipeHandle first = Cache().Acquire(params, payload); + ASSERT_NE(BoundGroupSamplerRecord(first), nullptr) << "the minting group was not sent the create"; + + groups.MakeCurrent(1); + const MGPipeHandle second = Cache().Acquire(params, payload); + EXPECT_EQ(second, first) << "one value is still one CSO across groups"; + const MGPipeSamplerCsoRecord* record = BoundGroupSamplerRecord(second); + ASSERT_NE(record, nullptr) << "the second share group names a sampler CSO its applier never received"; + EXPECT_EQ(record->Params.magFilter, params.magFilter); + EXPECT_EQ(record->Params.borderColorForm, params.borderColorForm); + EXPECT_EQ(Cache().GetCounters().Mints, 1u); + EXPECT_EQ(Cache().GetCounters().GroupPublishes, 1u); + + // Once per group, not once per acquire: a second hit in either group sends nothing. + const MGPipeHandle again = Cache().Acquire(params, payload); + groups.MakeCurrent(0); + const MGPipeHandle back = Cache().Acquire(params, payload); + EXPECT_EQ(Cache().GetCounters().GroupPublishes, 1u); + EXPECT_NE(BoundGroupSamplerRecord(back), nullptr); + + for (const MGPipeHandle held : {first, second, again, back}) Cache().Release(held); + EXPECT_EQ(Cache().GetCounters().UnknownReleases + Cache().GetCounters().UnderflowedReleases, 0u); + } + + // The control: contexts that SHARE a group share the record too, so a hit from the second + // re-issues nothing. + TEST(SamplerEmit, ASharingContextReusesTheRecordItsGroupAlreadyHolds) { + EmitterScope scope; + ShareGroupPair groups(/*shared=*/true); + Cache().ResetCounters(); + const SamplerParameters params = DistinctParameters(); + Uint64 payload = 0; + + groups.MakeCurrent(0); + const MGPipeHandle first = Cache().Acquire(params, payload); + groups.MakeCurrent(1); + const MGPipeHandle second = Cache().Acquire(params, payload); + EXPECT_EQ(second, first); + EXPECT_NE(BoundGroupSamplerRecord(second), nullptr); + EXPECT_EQ(Cache().GetCounters().GroupPublishes, 0u); + Cache().Release(first); + Cache().Release(second); + } + // ============================ D-F2: the sampler view ============================ TEST(SamplerEmit, AnUnchangedTextureReIssuesNothing) { diff --git a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp index b80796a5..b861b44c 100644 --- a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp @@ -1888,6 +1888,93 @@ TEST(TextureEmit, ATextureBornBeforeTheConsumerRegisteredGetsItsRecordFromItsFir EXPECT_EQ(Textures().RefusedParamCount(), 0u); } +// ============================ P14: textures of two share groups ============================ +// +// THE SHAPE BEHIND A SERVED APPLICATION's BLACK TEXTURES. Two non-shared contexts each give a +// texture the same sampler parameters. The server files texture and sampler-CSO records per +// share group, while the client's sampler CSO cache is one per process - so the second texture's +// set_texture_params named a built-in sampler handle that only the FIRST group's applier had a +// record for. Magma sampled that texture with default parameters and Espryt declined the push, +// leaving the driver's incomplete-texture default. Red without the per-group re-issue in +// MGPipeSamplerCsoCache::AcquireCanonical. +namespace { + thread_local Uint64 t_pairSession = 0; + thread_local Uint64 t_pairContext = 0; + + Bool PairKeyResolver(MGPipeApplierKey* outKey) { + if (t_pairSession == 0) return false; + outKey->SessionKey = t_pairSession; + outKey->ContextToken = t_pairContext; + return true; + } + + // Two contexts of one served session in two share groups, made current in turn on both halves + // a served client keeps in step: the frontend's GL state and the server's bound context + // (GLStreamScope rebinds the session before a GL entry point writes). + struct ShareGroupPair { + static constexpr Uint64 kSession = 0x5A3E7E50C0000000ull; + ShareGroupPair() : m_previous(MG_State::pGLContext) { + for (Uint64 i = 0; i < 2; ++i) { + m_contexts[i] = MakeShared(MakeShared(0x61 + i)); + MGPipeApplierRegisterContext(kSession, i + 1, 0x61 + i); + } + MGPipeSetApplierKeyResolver(&PairKeyResolver); + } + ~ShareGroupPair() { + t_pairSession = 0; + t_pairContext = 0; + MGPipeSetApplierKeyResolver(nullptr); + MGPipeApplierReleaseSession(kSession); + MG_State::pGLContext = m_previous; + } + ShareGroupPair(const ShareGroupPair&) = delete; + ShareGroupPair& operator=(const ShareGroupPair&) = delete; + + void MakeCurrent(Uint32 index) { + MG_State::pGLContext = m_contexts[index]; + t_pairSession = kSession; + t_pairContext = index + 1; + } + + SharedPtr m_previous; + SharedPtr m_contexts[2]; + }; +} // namespace + +TEST(TextureEmit, ATextureInASecondShareGroupNamesABuiltinSamplerItsOwnGroupHolds) { + TextureScope scope; + ShareGroupPair groups; + const auto makeCurrent = [&](Uint32 index) { groups.MakeCurrent(index); }; + + makeCurrent(0); + auto first = MakeTexture2D(301, 8); + first->GetSamplerObject()->SetLodBias(1.25f); + MG_Pipe::MGPipeEmitTextureParams(*first); + + makeCurrent(1); + auto second = MakeTexture2D(302, 8); + second->GetSamplerObject()->SetLodBias(1.25f); + MG_Pipe::MGPipeEmitTextureParams(*second); + + const MGPipeResourceRecord* record = AppliedTexture(Textures().FindTexture(*second)); + ASSERT_NE(record, nullptr) << "the second group holds no record for its own texture"; + const MGPipeHandle sampler = record->Params.BuiltinSampler; + const auto& samplers = MGPipeApplier().SamplerCsos; + ASSERT_FALSE(MGPipeHandleIsNull(sampler)); + ASSERT_LT(sampler.Slot, samplers.size()) + << "the texture names built-in sampler {" << sampler.Slot << ", " << sampler.Gen + << "}, which its share group's applier was never sent"; + EXPECT_TRUE(samplers[sampler.Slot].Live && samplers[sampler.Slot].Gen == sampler.Gen) + << "the texture names built-in sampler {" << sampler.Slot << ", " << sampler.Gen + << "}, which its share group's applier was never sent"; + EXPECT_EQ(samplers[sampler.Slot].Params.lodBias, 1.25f); + + // Each texture dies in its own group, where its record is. + second.reset(); + makeCurrent(0); + first.reset(); +} + #endif // MOBILEGL_PIPE_PUSH // =========================================================================================