swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-09 12:48:32 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (Pipe): key the composite resolver's memo on the context as well as the pipeline's GL name - the resolver is a process singleton while GL names are per context, so a make-current released the other context's live composite

Browse Source
This commit is contained in:
swung0x48
2026-09-08 15:33:42 -04:00
parent 820d1a60b9
commit ac09e5b37a
3 changed files with 231 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/Pipe/CompositeResolver.h
+105 -49
View File
@@ -39,10 +39,39 @@
// no-op: MGPipeSlotAllocator::Free refuses a slot that is not live at that generation and
// bumps no generation of its own, so a double release cannot skip a generation either.
//
// THE KEY IS ComputeDrawProgramSignature(), the per-graphics-stage {lifetimeId, GetLinkVersion()}
// array - and DELIBERATELY NOT GetBackendStateVersion(), which is what made the SSO
// conformance loop rebuild the composite (glslang + SPIR-V + spirv-opt) on every draw, because
// a glUniform1i to a sampler moves it.
// THE MEMO's KEY IS (CONTEXT ID, PIPELINE GL NAME) AND THE CONTEXT HALF IS NOT OPTIONAL.
// This resolver is a PROCESS singleton while a pipeline's GL name is per context: GLContext
// owns m_programPipelines AND its own name generator m_programPipelineNames (Core.h), so name
// N names two different ProgramPipelineObjects in two contexts, each with its own composite
// and its own handle. Keyed on the name alone, the first emission after a make-current found
// the OTHER context's entry, matched nothing - two composites are two ProgramObjects with two
// lifetime ids, so the handles differ even when the stage set and the signature are identical
// - and released it: a delete_shader_state and a cleared publication latch for a composite
// whose frontend ProgramObject is alive, its band slot handed back and re-issued at gen + 1,
// and the server rebuilding that program (glslang + SPIR-V + spirv-opt, the very cost the
// signature below exists to avoid) once per context switch.
//
// THE CONTEXT ID IS GLContext::GetTextureContextId() AND NOTHING ELSE - the tree's existing
// never-reused per-context id (TextureState::AllocateContextId; PipeInputs carries it as
// m_textureContextId at seven fill points and the backends' own per-context memos key on it).
// Deliberately NOT the GLContext ADDRESS that MGB_CTX_IDENTITY and MGPipeTracker::m_context
// compare, because Core.h states the reason that id exists at all: a context freed and remade
// lands on the old heap address, which would put this same defect back one context recreation
// later.
//
// WHAT RELEASES A DESTROYED CONTEXT's ENTRIES: nothing in this file, and that is the correct
// answer rather than an omission. Destroying a context drops m_programPipelines, which drops
// each ProgramPipelineObject, which drops the composite it cached; ~ProgramObject then runs
// MGPipeEmitShaderCsoDestroyAndFree - the composite's OWN release path, the second of the two
// above - and the slot goes back exactly once. The entries those composites leave behind can
// never be found again (no future Observe can carry a dead context id) and could not release
// anything if they were (the allocator erases the lifetime-id mapping on Free), so Reset()
// DROPS them instead of releasing them. That is also what bounds the vector; see Reset().
//
// THE SIGNATURE IS ComputeDrawProgramSignature(), the per-graphics-stage {lifetimeId,
// GetLinkVersion()} array - and DELIBERATELY NOT GetBackendStateVersion(), which is what made
// the SSO conformance loop rebuild the composite (glslang + SPIR-V + spirv-opt) on every draw,
// because a glUniform1i to a sampler moves it.
//
// HEADER-ONLY, for the ownership reason Tracker.h states: a new .cpp would need the root
// CMakeLists.txt, which is the contract package's.
@@ -84,6 +113,11 @@ namespace MobileGL::MG_Pipe {
Uint64 Mints = 0; // signatures this resolver has seen minted
Uint64 Reuses = 0; // a signature that had not moved
Uint64 Releases = 0; // signature-move releases, i.e. the pipeline-cache path
// Entries dropped by Reset() because the composite's slot was already gone - the
// shape every entry of a DESTROYED CONTEXT ends in. A dropped entry is not a
// release: nothing is emitted and nothing is freed, the obligation having been
// discharged by the composite's own ~ProgramObject.
Uint64 Sweeps = 0;
};
// Told, at every emission, which composite the frontend handed out for which pipeline.
@@ -95,63 +129,81 @@ namespace MobileGL::MG_Pipe {
// one death helper and in its fixed order. That is the pipeline-cache release path; the
// composite's own destructor is the other one and the second of the two is the proven
// no-op.
MGPipeHandle Observe(const ProgramPipelineObject& pipeline, const ProgramObject& composite,
MGPipeHandle handle) {
MGPipeHandle Observe(Uint64 contextId, const ProgramPipelineObject& pipeline,
const ProgramObject& composite, MGPipeHandle handle) {
const DrawProgramSignature signature = pipeline.ComputeDrawProgramSignature();
const Uint key = pipeline.GetExternalIndex();
Entry* entry = Find(key);
const Uint pipelineName = pipeline.GetExternalIndex();
Entry* entry = Find(contextId, pipelineName);
if (entry != nullptr) {
if (entry->Signature == signature && entry->Handle == handle) {
// THE SAME COMPOSITE. Not merely "the same signature": the handle is minted
// off the composite ProgramObject's own lifetime id, so an identical handle
// IS an identical object, and there is nothing to release whatever the
// memo's freshness says. Re-arming Fresh here is what a Reset() costs - one
// re-observation - and Live is deliberately NOT touched, because it is the
// release obligation and it is still owed for exactly this handle.
entry->Fresh = true;
// IS an identical object and there is nothing to release. Live is
// deliberately NOT touched - it is the release obligation and it is still
// owed for exactly this handle.
++m_counters.Reuses;
return handle;
}
// A MOVED SIGNATURE ON THIS CONTEXT's OWN ENTRY, which is the only thing that
// can reach here now: another context's pipeline of the same name is not found
// above and therefore not released, its obligation staying owed to the context
// that took it.
ReleaseEntry(*entry);
} else {
m_entries.push_back(Entry{});
entry = &m_entries.back();
entry->PipelineName = key;
entry->ContextId = contextId;
entry->PipelineName = pipelineName;
}
entry->Signature = signature;
entry->Handle = handle;
entry->CompositeLifetimeId = composite.GetLifetimeId();
entry->Live = true;
entry->Fresh = true;
++m_counters.Mints;
return handle;
}
// A make-current. The entries name composites that belong to the frontend objects of
// the context being left, and those objects outlive the switch, so the RECORDS are not
// released here - releasing them would emit a delete for a live program.
// A make-current, and it RELEASES NOTHING. The entries name composites that belong to
// the frontend objects of the context being left, those objects outlive the switch, and
// releasing them would emit a delete for a live program.
//
// ONLY THE MEMO's FRESHNESS IS DROPPED, AND `Fresh` IS A SEPARATE FLAG FROM `Live` FOR
// EXACTLY THAT REASON. The two were one flag and the conflation was a real defect: the
// reuse branch above returns before anything could restore it, so after the first
// make-current every entry stayed at false for the life of the process and ReleaseEntry
// early-returned for ever - no delete_shader_state, no Free, no counter, and the old
// composite's handle simply overwritten out of the resolver. `Live` is a RELEASE
// OBLIGATION and nothing but ReleaseEntry may clear it; `Fresh` is the memo's own "does
// this entry describe the current context's pipeline of this name", which costs at most
// one re-observation when it is wrong.
// NOR IS ANY MEMO INVALIDATED, and that is what the context key bought. This used to
// clear a per-entry `Fresh` flag beside `Live`, because with a name-only key an entry
// could not say whether it described "my own pipeline before the switch" or "another
// context's pipeline of the same name" - and exactly one of those two properties could
// hold at a time. The key answers the question directly now, so the freshness flag and
// its one reader (a HandleFor() accessor that had no caller anywhere in the tree) are
// both gone rather than left as scaffolding: `Live`, the release obligation, is the
// entry's only state and nothing but ReleaseEntry may clear it.
//
// WHAT IS LEFT TO DO HERE IS RECLAMATION, and this is the one moment the client is told
// that a context boundary was crossed. An entry whose composite slot is no longer live
// has had its obligation discharged elsewhere - by that composite's own ~ProgramObject,
// which is precisely what happened to EVERY entry of a context that has just been
// destroyed - so it is DROPPED rather than released: a release would resolve nothing
// anyway (the allocator erases the lifetime-id mapping on Free) and no reader is left.
// Without this the vector would grow by one per (context, pipeline name) pair the
// process ever used, where the name-only key bounded it by the highest pipeline name;
// with it, it is bounded by the pairs whose composite slot is actually live.
void Reset() {
for (Entry& entry : m_entries) entry.Fresh = false;
SizeT kept = 0;
for (SizeT i = 0; i < m_entries.size(); ++i) {
if (!m_entries[i].Live || !MGPipeSlots().IsLive(MGPipeKind::ShaderCso, m_entries[i].Handle)) {
++m_counters.Sweeps;
continue;
}
if (kept != i) m_entries[kept] = m_entries[i];
++kept;
}
m_entries.resize(kept);
}
void ResetCounters() { m_counters = Counters{}; }
MGPipeHandle HandleFor(Uint pipelineName) const {
for (const Entry& entry : m_entries) {
if (entry.PipelineName == pipelineName && entry.Live && entry.Fresh) return entry.Handle;
}
return kMGPipeNullHandle;
}
// Diagnostics and unit cases only; nothing on the emission path asks. There is no
// HandleFor(name) accessor and there must not be one: the emitter takes the handle from
// the composite ProgramObject it already holds, so a lookup by name would be a second
// authority on an identity the allocator already owns.
SizeT Size() const { return m_entries.size(); }
const Counters& GetCounters() const { return m_counters; }
@@ -161,17 +213,21 @@ namespace MobileGL::MG_Pipe {
// choice: a static that held one would put a frontend destructor on an exit
// handler's path into a torn-down pipe. A GL name, a signature of plain integers,
// a handle and a lifetime id are all this needs.
// KEYED ON THE GL NAME, because a ProgramPipelineObject has no lifetime id -
// ComputeDrawProgramSignature reads the STAGE programs' ids and the pipeline itself
// carries none. glGenProgramPipelines recycles names, so a deleted-and-recreated
// pipeline can inherit its predecessor's entry; that is bounded and self-correcting
// rather than a hazard. The first Observe on the new object finds a signature and a
// handle that do not match and releases the old entry, and that release resolves
// NOTHING - the allocator erases the lifetime-id mapping on Free, so a stale
// CompositeLifetimeId emits no delete and frees no slot; all it costs is one
// redundant, idempotent death notice, which is the same shape the composite's own
// second release path already has. The vector is keyed by name, so it cannot grow
// past the highest pipeline name the process ever used.
// KEYED ON (CONTEXT ID, GL NAME), and the name half is the GL name because a
// ProgramPipelineObject has no lifetime id - ComputeDrawProgramSignature reads the
// STAGE programs' ids and the pipeline itself carries none. The context half is
// GLContext::GetTextureContextId(); see the file header for why the name alone was
// wrong and why the context ADDRESS would be too.
//
// WITHIN ONE CONTEXT glGenProgramPipelines recycles names, so a deleted-and-
// recreated pipeline can still inherit its predecessor's entry; that is bounded and
// self-correcting rather than a hazard. The first Observe on the new object finds a
// signature and a handle that do not match and releases the old entry, and that
// release resolves NOTHING - the allocator erases the lifetime-id mapping on Free,
// so a stale CompositeLifetimeId emits no delete and frees no slot; all it costs is
// one redundant, idempotent death notice, which is the same shape the composite's
// own second release path already has.
Uint64 ContextId = 0;
Uint PipelineName = 0;
DrawProgramSignature Signature{};
MGPipeHandle Handle = kMGPipeNullHandle;
@@ -179,13 +235,13 @@ namespace MobileGL::MG_Pipe {
// THE RELEASE OBLIGATION. Set when this entry takes responsibility for a composite's
// slot, cleared ONLY by ReleaseEntry when that responsibility is discharged.
Bool Live = false;
// THE MEMO's FRESHNESS, and deliberately not the same flag as Live - see Reset().
Bool Fresh = false;
};
Entry* Find(Uint pipelineName) {
// BOTH HALVES OF THE KEY, always. An entry of another context is not this pipeline's
// entry: not found, not matched, not released.
Entry* Find(Uint64 contextId, Uint pipelineName) {
for (Entry& entry : m_entries) {
if (entry.PipelineName == pipelineName) return &entry;
if (entry.ContextId == contextId && entry.PipelineName == pipelineName) return &entry;
}
return nullptr;
}
MobileGL/MG_Impl/Pipe/ProgramEmit.h
+8 -1
View File
@@ -115,7 +115,14 @@ namespace MobileGL::MG_Pipe {
// allocator refuses a slot that is not live at that generation.
if (drawProgram && MGPipeProgramIsPipelineComposite(*drawProgram)) {
if (const auto& pipeline = ctx.GetBoundProgramPipeline()) {
MGPipeCompositeResolverInstance().Observe(*pipeline, *drawProgram, drawCso);
// THE CONTEXT IS PART OF THE RESOLVER's KEY and this is the only place that
// supplies it: the resolver is a process singleton and a pipeline's GL name
// is per context, so without it a make-current between two contexts holding
// one pipeline name released the other context's LIVE composite.
// GetTextureContextId() is the tree's never-reused per-context id, the same
// one PipeInputs carries and the backends' per-context memos key on.
MGPipeCompositeResolverInstance().Observe(ctx.GetTextureContextId(), *pipeline,
*drawProgram, drawCso);
}
}
const MGPipeHandle dispatchCso =
MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp
+118 -1
View File
@@ -361,7 +361,9 @@ TEST(CompositeResolver, ASlotAtTheShaderCsoLimitIsRefusedWhileTheLastBandSlotIsN
X(CompositeResolver, TwoPipelinesWithTheSameSignatureKeepTheirOwnComposite) \
X(CompositeResolver, EvictionThenDestructionFreesTheSlotExactlyOnce) \
X(CompositeResolver, DestructionThenEvictionFreesTheSlotExactlyOnce) \
X(CompositeResolver, ASignatureMoveAfterAMakeCurrentStillReleasesThroughTheResolver)
X(CompositeResolver, ASignatureMoveAfterAMakeCurrentStillReleasesThroughTheResolver) \
X(CompositeResolver, TwoContextsHoldingOnePipelineNameKeepTheirOwnComposites) \
X(CompositeResolver, ADestroyedContextsEntryIsDroppedRatherThanReleasedASecondTime)
#define MGL_DECLARE_PULL_SKIP(Suite, Name) \
TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; }
@@ -373,6 +375,7 @@ namespace {
namespace GL = MobileGL::MG_Impl::GLImpl;
using GLContext = MG_State::GLState::GLContext;
using MG_State::GLState::ProgramObject;
using MG_State::GLState::ProgramPipelineObject;
struct ResolverScope {
ResolverScope() { Clear(); }
@@ -620,6 +623,120 @@ void main() { o_color = vec4(0.5); }
<< "and the first composite's slot really went back, exactly once";
EXPECT_TRUE(MGPipeSlots().IsLive(MGPipeKind::ShaderCso, second));
}
// THE RESOLVER IS A PROCESS SINGLETON AND A PIPELINE's GL NAME IS PER CONTEXT (C2-M1).
// GLContext owns m_programPipelines AND its own name generator m_programPipelineNames, so
// name N names two different ProgramPipelineObjects in two contexts, each with its own
// composite and its own handle. Keyed on the name alone, the first Observe after a
// make-current found the OTHER context's entry: the signature matched - two composites of
// the same stage set have the same signature, and here two default pipelines have the same
// all-zero one - while the handle could not, because two composites are two ProgramObjects
// with two lifetime ids. So it fell into ReleaseEntry and emitted delete_shader_state for a
// LIVE composite, cleared its publication latch and handed its band slot back while the
// frontend ProgramObject was still alive.
//
// DRIVEN AT THE RESOLVER RATHER THAN THROUGH GL, because one test process has one
// GLContext. Everything the case supplies is what the single production call site supplies:
// the context id is GLContext::GetTextureContextId()'s value, the two same-named pipeline
// objects are what two contexts hold, and the composites are ordinary ProgramObjects at
// external index 0 out of the reserved band, exactly as Core.cpp builds them.
TEST(CompositeResolver, TwoContextsHoldingOnePipelineNameKeepTheirOwnComposites) {
ResolverScope scope;
auto& resolver = MGPipeCompositeResolverInstance();
const LinkArtifacts link;
const SpirvArtifacts spirv;
constexpr Uint kSharedPipelineName = 9u;
constexpr Uint64 kContextA = 0x51A00001ull;
constexpr Uint64 kContextB = 0x51A00002ull;
const ProgramPipelineObject pipelineA{kSharedPipelineName};
const ProgramPipelineObject pipelineB{kSharedPipelineName};
const SharedPtr<ProgramObject> compositeA = MakeShared<ProgramObject>(0u);
const SharedPtr<ProgramObject> compositeB = MakeShared<ProgramObject>(0u);
ASSERT_TRUE(MGPipeProgramIsPipelineComposite(*compositeA));
ASSERT_EQ(pipelineA.GetExternalIndex(), pipelineB.GetExternalIndex());
const MGPipeHandle handleA = MGPipeSlots().AllocateComposite(compositeA->GetLifetimeId());
const MGPipeHandle handleB = MGPipeSlots().AllocateComposite(compositeB->GetLifetimeId());
ASSERT_NE(handleA, handleB);
for (const MGPipeHandle handle : {handleA, handleB}) {
MGPipeApplyCreateShaderState(CompositeDesc(handle, 0x3u), &link, &spirv);
MGPipeNoteHandlePublished(MGPipeKind::ShaderCso, handle);
}
const Uint64 releasesBefore = resolver.GetCounters().Releases;
EXPECT_EQ(resolver.Observe(kContextA, pipelineA, *compositeA, handleA), handleA);
// THE MAKE-CURRENT, which is what PipeFill's FreshlyPrimed arm reaches through the
// program emitter's own Reset()...
MGPipeProgramEmitterInstance().Reset();
// ... and then context B draws with ITS pipeline of the same name.
EXPECT_EQ(resolver.Observe(kContextB, pipelineB, *compositeB, handleB), handleB);
EXPECT_EQ(resolver.GetCounters().Releases, releasesBefore)
<< "the other context's entry was released - it is not this pipeline's entry";
EXPECT_TRUE(MGPipeSlots().IsLive(MGPipeKind::ShaderCso, handleA))
<< "A's band slot went back while A's composite ProgramObject was still alive";
EXPECT_TRUE(MGPipeHandleIsPublished(MGPipeKind::ShaderCso, handleA))
<< "delete_shader_state went out for a live composite and cleared its latch";
EXPECT_TRUE(MGPipeSlots().IsLive(MGPipeKind::ShaderCso, handleB));
// AND BACK TO A. Its obligation stayed owed to its own context, so the unmoved
// signature is a reuse of the same handle and still nothing is released.
const Uint64 reusesBefore = resolver.GetCounters().Reuses;
MGPipeProgramEmitterInstance().Reset();
EXPECT_EQ(resolver.Observe(kContextA, pipelineA, *compositeA, handleA), handleA);
EXPECT_EQ(resolver.GetCounters().Reuses, reusesBefore + 1u);
EXPECT_EQ(resolver.GetCounters().Releases, releasesBefore);
}
// WHERE A DESTROYED CONTEXT's ENTRIES ARE RELEASED, and it is not in the resolver. The
// context's death drops m_programPipelines, which drops the ProgramPipelineObject, which
// drops the composite it cached; ~ProgramObject then runs the one client-side death helper
// and the band slot goes back EXACTLY ONCE. The resolver speaks no second delete - the
// entry can never be found again, and the allocator has erased the lifetime-id mapping
// anyway - and the next make-current DROPS the stranded entry, which is what keeps the
// vector bounded now that its key carries the context.
TEST(CompositeResolver, ADestroyedContextsEntryIsDroppedRatherThanReleasedASecondTime) {
ResolverScope scope;
auto& resolver = MGPipeCompositeResolverInstance();
const LinkArtifacts link;
const SpirvArtifacts spirv;
constexpr Uint kPipelineName = 11u;
constexpr Uint64 kDoomedContext = 0x51A00003ull;
// ResolverScope's Clear() has already run one Reset(), so every entry standing here has
// a live composite slot and nothing but this case's own entry can be swept below.
const SizeT sizeBefore = resolver.Size();
const Uint32 liveBefore = MGPipeSlots().LiveCount(MGPipeKind::ShaderCso);
const ProgramPipelineObject pipeline{kPipelineName};
SharedPtr<ProgramObject> composite = MakeShared<ProgramObject>(0u);
const MGPipeHandle handle = MGPipeSlots().AllocateComposite(composite->GetLifetimeId());
MGPipeApplyCreateShaderState(CompositeDesc(handle, 0x3u), &link, &spirv);
MGPipeNoteHandlePublished(MGPipeKind::ShaderCso, handle);
ASSERT_EQ(resolver.Observe(kDoomedContext, pipeline, *composite, handle), handle);
ASSERT_EQ(resolver.Size(), sizeBefore + 1u);
const Uint64 releasesBefore = resolver.GetCounters().Releases;
const Uint64 sweepsBefore = resolver.GetCounters().Sweeps;
// THE CONTEXT DIES: the last SharedPtr to its composite goes with its pipeline.
composite.reset();
EXPECT_FALSE(MGPipeSlots().IsLive(MGPipeKind::ShaderCso, handle))
<< "~ProgramObject is the release path for this entry and it freed the slot";
EXPECT_FALSE(MGPipeHandleIsPublished(MGPipeKind::ShaderCso, handle))
<< "and it took the publication latch with the delete";
EXPECT_EQ(MGPipeSlots().LiveCount(MGPipeKind::ShaderCso), liveBefore) << "exactly once";
EXPECT_EQ(resolver.GetCounters().Releases, releasesBefore)
<< "the resolver spoke no release of its own for it";
// THE NEXT CONTEXT's FIRST VERB. The stranded entry is dropped, not released.
MGPipeProgramEmitterInstance().Reset();
EXPECT_EQ(resolver.GetCounters().Sweeps, sweepsBefore + 1u);
EXPECT_EQ(resolver.GetCounters().Releases, releasesBefore)
<< "a dropped entry emits nothing and frees nothing - the obligation was discharged";
EXPECT_EQ(resolver.Size(), sizeBefore)
<< "the vector is bounded by the pairs whose composite slot is actually live";
}
} // namespace
#endif // MOBILEGL_PIPE_PUSH