diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 587a2782..840ef309 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -451,6 +451,12 @@ namespace MobileGL::MG_Backend::DirectGLES { } #endif + // The seven DirectGLES.cpp call sites drive the LEGACY arm and nothing else. On the + // handle arm death is announced by the frontend object's destructor + // (MG_State/GLState/StateObjectDeathNotice.h), so there is no garbage to collect on a + // tick and this is the predicted branch plus a return - which is how ROADMAP.md:18's + // "delete the GC" is delivered without deleting the legacy arm's own collector while + // that arm is still compiled beside it. void CollectGarbageIfNeeded() { #if MOBILEGL_PIPE_PUSH if (EsprytSlotTablesEnabled()) { @@ -458,12 +464,14 @@ namespace MobileGL::MG_Backend::DirectGLES { return; } #endif +#if MOBILEGL_PIPE_LEGACY_MEMOS ++m_gcTick; if (m_gcTick < kGCInterval) { return; } CollectGarbage(); m_gcTick = 0; +#endif } void CollectGarbageNow() { diff --git a/MobileGL/MG_Backend/DirectGLES/SlotTables.h b/MobileGL/MG_Backend/DirectGLES/SlotTables.h index 61400651..b2c9751e 100644 --- a/MobileGL/MG_Backend/DirectGLES/SlotTables.h +++ b/MobileGL/MG_Backend/DirectGLES/SlotTables.h @@ -33,28 +33,26 @@ // * Slots are dense per kind, which is what lets the server side (ARCHITECTURE.md 10.1, // MG_Remote/Server/PipeObjectTables) be an array rather than an object graph. // -// Death: announced where the package may announce it, discovered everywhere else. -// DestroyByLifetimeId() below is step e2's backend half and it is complete - it drops the twin -// and returns the slot the moment the frontend object's last SharedPtr goes - and the notice -// that drives it (MG_State/GLState/StateObjectDeathNotice.h, BufferBackendOps' shape) is -// registered for all six kinds. What is only PARTLY wired is the firing side: a destructor has -// to raise the notice, and of the six object classes the P2 file-ownership table gives -// {Texture,Framebuffer,Sampler,VertexArray}State/* to other packages, so only ProgramObject -// and RenderbufferObject fire it here. The four that do not still rely on the sweep, which is -// why the table keeps ONE weak_ptr per entry and uses it for exactly one thing: -// ReclaimDeadSlots() frees the slot - and the twin, and the driver storage it owns - once the -// frontend object is gone. That is a liveness sweep, not an identity test, and it is what -// bumps Gen, which is precisely the ABA defence: a slot is only ever handed out again after it -// was freed. The four remaining one-line destructor calls retire the sweep entirely. +// Death is ANNOUNCED, and that is what lets this table have no garbage collector - the +// deliverable ROADMAP.md:18 spells "GC" in and the one D13 makes a precondition of the switch- +// over. All six re-keyed object classes raise MG_State::GLState::NotifyStateObjectDestroyed() +// from their destructor (BufferBackendOps' shape, one entry point for six kinds), the backend +// consumes it in Managers.cpp, and DestroyByLifetimeId() below drops the twin and returns the +// slot at the moment the frontend object's last SharedPtr goes. So: +// * there is NO draw-path tick and NO creation tick on this arm. CollectGarbageIfNeeded() is +// an empty call, and the seven call sites in DirectGLES.cpp drive the LEGACY registry only; +// * a twin, and the driver storage it owns, is freed when the application lets go of the +// object rather than up to 64 creations or 1024 draw ticks later. That is what +// Managers.h's "dead gigabytes" note asked for. // -// Because the sweep is still the only death signal, this table carries BOTH of the drivers -// the registry it replaces carries, and for the same reasons: -// * the draw-path tick (kGCInterval = 1024 CollectGarbageIfNeeded calls), and -// * the CREATION tick (kCreationGCInterval = 64 first-time insertions), because object CHURN -// rather than draw count is what makes the sweep urgent - a CTS-shaped case runs ~10 -// per-draw ticks, so 1024 of them span ~100 cases' worth of dead, gigabyte-sized objects. -// Dropping the second one would have made this table's memory behaviour strictly WORSE than -// the map it replaces, which is the opposite of what the slice is for. +// The weak_ptr per entry survives, and only for what it is honest about: +// * ForEachLive() hands the callee a STRONG reference to the frontend object, which the one +// direct-iteration site (ScopedDetachedTextureFramebufferAttachments) needs; and +// * ReclaimDeadSlots() is kept as the body of the EXPLICIT CollectGarbageNow(), i.e. a +// collection someone asks for, never a periodic one. It is the backstop for the one case +// the notice cannot cover: a destructor that runs after exit() has begun, where +// InProcessTeardown() drops the notice because a twin destructor must not call the driver. +// It is never an identity test - that is what Gen is for. // // 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(). @@ -148,22 +146,12 @@ namespace MobileGL::MG_Backend::DirectGLES { return m_nullTwin; } - // Sweep BEFORE the entry reference below exists, for the same reason the map arm - // does it here: EntryAt may grow m_slots and move every element, so a reference - // taken first would not survive it. The sweep is owed from an earlier creation - // rather than triggered by this one. - if (m_creationTick >= kCreationGCInterval) { - m_creationTick = 0; - ReclaimDeadSlots(); - } - const MG_Pipe::MGPipeHandle handle = MG_Pipe::MGPipeSlots().Acquire(kKind, stateObj->GetLifetimeId()); MOBILEGL_ASSERT(!MG_Pipe::MGPipeHandleIsNull(handle), "MGPipe slot space of kind %u is exhausted", static_cast(kKind)); Entry& entry = EntryAt(handle.Slot); - const Bool firstInsertion = !entry.Live || entry.Gen != handle.Gen; if (entry.Live && entry.Gen != handle.Gen) { // The slot was reclaimed and handed to a new object: the twin at it describes // driver ids the new state object never made. @@ -172,19 +160,11 @@ namespace MobileGL::MG_Backend::DirectGLES { entry.Gen = handle.Gen; entry.Live = true; entry.stateRef = stateObj; - if (firstInsertion) { - // A slot this table has never held (or held for a previous owner). Nothing - // tells the backend that a texture or renderbuffer was DELETED - the twin, and - // the driver storage it owns, lives until a collection - and - // CollectGarbageIfNeeded is ticked only from the per-draw sync paths, which a - // CTS-shaped workload runs about ten times per case. 1024 of those ticks then - // span ~100 cases, so ~100 cases' worth of dead (and, for this suite, - // gigabyte-sized) objects would stay allocated at once. Object CHURN rather - // than draw count is what makes the sweep urgent, so a twin the table has - // never seen ticks it too - and it does so on the path that is about to - // allocate, which is exactly when the memory is needed. - ++m_creationTick; - } + // No creation tick and no sweep here. The registry this replaces needed both, + // because nothing told it a texture or a renderbuffer had been DELETED and object + // CHURN rather than draw count is what made that urgent. Every one of the six kinds + // now announces its own death from its destructor, so a dead twin's slot is already + // back before the next creation asks for one. RememberHandle(stateObj->GetLifetimeId(), handle); return entry.backend; } @@ -285,24 +265,18 @@ namespace MobileGL::MG_Backend::DirectGLES { return true; } - void CollectGarbageIfNeeded() { - ++m_gcTick; - if (m_gcTick < kGCInterval) return; - m_gcTick = 0; - m_creationTick = 0; - ReclaimDeadSlots(); - } + // Deliberately EMPTY, and this is the P2 deliverable rather than an omission: on this + // arm death is announced, so there is nothing for a periodic sweep to discover. The + // seven DirectGLES.cpp call sites keep their spelling because they are the legacy + // registry's driver and that arm is still compiled beside this one; on this arm they + // cost the predicted branch in StateBackendObjectRegistry and return. + void CollectGarbageIfNeeded() {} - void CollectGarbageNow() { - m_creationTick = 0; - ReclaimDeadSlots(); - } - - // Test-only introspection: how many first-time insertions are owed before the - // creation-driven sweep fires. Reading it is what lets a test pin the CADENCE rather - // than only the effect of an explicit CollectGarbageNow(). - Uint32 CreationTickForTest() const { return m_creationTick; } - static constexpr Uint32 CreationGCIntervalForTest() { return kCreationGCInterval; } + // An EXPLICIT collection - someone asked, so it runs. Not a driver: nothing calls this + // on a tick. It is the backstop for a notice that could not be delivered (see the + // InProcessTeardown() note in the file header) and the tests' way of forcing the + // liveness sweep without waiting for one. + void CollectGarbageNow() { ReclaimDeadSlots(); } // 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 @@ -342,16 +316,8 @@ namespace MobileGL::MG_Backend::DirectGLES { m_memoHandle = MG_Pipe::kMGPipeNullHandle; } - static constexpr Uint32 kGCInterval = 1024; - // Creations are far rarer than draws, so this counts in a much smaller unit than - // kGCInterval does. Same value the map arm uses, so the two arms sweep at the same - // cadence under the same workload. - static constexpr Uint32 kCreationGCInterval = 64; - // Indexed by MGPipeHandle::Slot; [0] is the reserved slot and is never live. Vector m_slots; - Uint32 m_gcTick = 0; - Uint32 m_creationTick = 0; Bool m_isCollecting = false; // Handed back by GetOrCreate for a null state object. Never live, never swept. BackendPtr m_nullTwin; diff --git a/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp b/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp index b8e54d19..92402f06 100644 --- a/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp +++ b/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp @@ -7,6 +7,7 @@ // End of Source File Header #include "FramebufferObject.h" +#include "MG_State/GLState/StateObjectDeathNotice.h" #include "MG_Util/Types.h" #include @@ -21,6 +22,20 @@ namespace MobileGL::MG_State::GLState { return s_nextFramebufferLifetimeId.fetch_add(1, std::memory_order_relaxed); } +#if MOBILEGL_PIPE_PUSH + FramebufferObject::~FramebufferObject() { + // P2 step e2: ANNOUNCE the death instead of leaving the backend to discover it in a + // garbage sweep. This is the last SharedPtr to this object dropping - not the + // glDelete* that only marks the name and leaves a still-bound object very much + // alive - so it is the exact moment the backend's twin, and the driver storage + // that twin owns, stop being reachable. The notice carries the lifetime id + // because the object no longer exists to be passed, and because the lifetime id + // is what the client slot allocator resolves the handle from. No-op unless a + // backend registered the ops (a pull build declares none at all). + NotifyStateObjectDestroyed(MG_Pipe::MGPipeKind::Framebuffer, m_lifetimeId); + } +#endif + // FramebufferAttachmentObject FramebufferAttachmentObject::FramebufferAttachmentObject( const SharedPtr& texture, TextureUploadTarget textureUploadTarget, Int level, diff --git a/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h b/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h index 8e3bf156..d9153cf9 100644 --- a/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h +++ b/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h @@ -115,6 +115,12 @@ namespace MobileGL { Array(FramebufferAttachmentType::FramebufferAttachmentTypeCount)>; FramebufferObject(Uint externalIndex); +#if MOBILEGL_PIPE_PUSH + // P2 step e2. Out of line, and declared only where there is a notice to raise: + // in a pull build this class keeps its implicit destructor, which is what keeps + // the pull build's symbol set byte-for-byte the pre-P2 one (G1). + ~FramebufferObject(); +#endif void AttachTexture(FramebufferAttachmentType type, const SharedPtr& texture, TextureUploadTarget textureUploadTarget = TextureUploadTarget::Unknown, int level = 0, diff --git a/MobileGL/MG_State/GLState/SamplerState/SamplerObject.cpp b/MobileGL/MG_State/GLState/SamplerState/SamplerObject.cpp index c1010888..3f91b4ad 100644 --- a/MobileGL/MG_State/GLState/SamplerState/SamplerObject.cpp +++ b/MobileGL/MG_State/GLState/SamplerState/SamplerObject.cpp @@ -9,6 +9,7 @@ #include "SamplerObject.h" #include +#include #include @@ -24,6 +25,20 @@ namespace MobileGL { SamplerObject::SamplerObject(Uint externalIndex) : m_externalIndex(externalIndex), m_lifetimeId(AllocateLifetimeId()) {} +#if MOBILEGL_PIPE_PUSH + SamplerObject::~SamplerObject() { + // P2 step e2: ANNOUNCE the death instead of leaving the backend to discover it in a + // garbage sweep. This is the last SharedPtr to this object dropping - not the + // glDelete* that only marks the name and leaves a still-bound object very much + // alive - so it is the exact moment the backend's twin, and the driver storage + // that twin owns, stop being reachable. The notice carries the lifetime id + // because the object no longer exists to be passed, and because the lifetime id + // is what the client slot allocator resolves the handle from. No-op unless a + // backend registered the ops (a pull build declares none at all). + NotifyStateObjectDestroyed(MG_Pipe::MGPipeKind::SamplerCso, m_lifetimeId); + } +#endif + void SamplerObject::BumpVersion() { ++m_version; // Every setter early-outs on an unchanged value, so this only runs on a real diff --git a/MobileGL/MG_State/GLState/SamplerState/SamplerObject.h b/MobileGL/MG_State/GLState/SamplerState/SamplerObject.h index 90b4619c..b5dafc23 100644 --- a/MobileGL/MG_State/GLState/SamplerState/SamplerObject.h +++ b/MobileGL/MG_State/GLState/SamplerState/SamplerObject.h @@ -16,6 +16,12 @@ namespace MobileGL { class SamplerObject { public: SamplerObject(Uint externalIndex); +#if MOBILEGL_PIPE_PUSH + // P2 step e2. Out of line, and declared only where there is a notice to raise: + // in a pull build this class keeps its implicit destructor, which is what keeps + // the pull build's symbol set byte-for-byte the pre-P2 one (G1). + ~SamplerObject(); +#endif void SetWrapS(SamplerWrapMode mode); void SetWrapT(SamplerWrapMode mode); diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp b/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp index 039e301e..000f4d93 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp @@ -8,6 +8,7 @@ #include "TextureObject.h" #include "MG_State/GLState/Core.h" +#include "MG_State/GLState/StateObjectDeathNotice.h" #include "MG_Util/Types.h" #include @@ -25,6 +26,20 @@ namespace MobileGL { return s_nextTextureLifetimeId.fetch_add(1, std::memory_order_relaxed); } +#if MOBILEGL_PIPE_PUSH + TextureObjectBase::~TextureObjectBase() { + // P2 step e2: ANNOUNCE the death instead of leaving the backend to discover it in a + // garbage sweep. This is the last SharedPtr to this object dropping - not the + // glDelete* that only marks the name and leaves a still-bound object very much + // alive - so it is the exact moment the backend's twin, and the driver storage + // that twin owns, stop being reachable. The notice carries the lifetime id + // because the object no longer exists to be passed, and because the lifetime id + // is what the client slot allocator resolves the handle from. No-op unless a + // backend registered the ops (a pull build declares none at all). + NotifyStateObjectDestroyed(MG_Pipe::MGPipeKind::Texture, m_lifetimeId); + } +#endif + void TextureObjectBase::BumpShapeVersion() { ++m_shapeVersion; // Shape is what mipmap-completeness is computed from, and completeness decides diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObject.h b/MobileGL/MG_State/GLState/TextureState/TextureObject.h index 48ef2777..1cb33cca 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObject.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureObject.h @@ -116,7 +116,15 @@ namespace MobileGL::MG_State::GLState { class TextureObjectBase : public ITextureObject { public: TextureObjectBase(TextureTarget target, Uint externalIndex); +#if MOBILEGL_PIPE_PUSH + // P2 step e2. Out of line, and declared only where there is a notice to raise: in a + // pull build this stays the implicit `= default` the pre-P2 tree had, which is what + // keeps the pull build's symbol set byte-for-byte the pre-P2 one (G1). Declared on the + // BASE, so every concrete texture class - 2D, 3D, cube, buffer, view - announces once. + virtual ~TextureObjectBase(); +#else virtual ~TextureObjectBase() = default; +#endif TextureInternalFormat GetFormat() const override; TextureTarget GetTarget() const override; diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp index db784caf..02a0e5f8 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp @@ -8,6 +8,8 @@ #include "VertexArrayObject.h" +#include + #include namespace MobileGL::MG_State::GLState { @@ -37,6 +39,20 @@ namespace MobileGL::MG_State::GLState { } } +#if MOBILEGL_PIPE_PUSH + VertexArrayObject::~VertexArrayObject() { + // P2 step e2: ANNOUNCE the death instead of leaving the backend to discover it in a + // garbage sweep. This is the last SharedPtr to this object dropping - not the + // glDelete* that only marks the name and leaves a still-bound object very much + // alive - so it is the exact moment the backend's twin, and the driver storage + // that twin owns, stop being reachable. The notice carries the lifetime id + // because the object no longer exists to be passed, and because the lifetime id + // is what the client slot allocator resolves the handle from. No-op unless a + // backend registered the ops (a pull build declares none at all). + NotifyStateObjectDestroyed(MG_Pipe::MGPipeKind::VertexElementsCso, m_lifetimeId); + } +#endif + void VertexArrayObject::EnableAttribute(Uint index) { if (index >= MAX_VERTEX_ATTRIBS) return; diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h index 8a6d22f6..41c5065b 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h @@ -25,6 +25,12 @@ namespace MobileGL { static constexpr int MAX_VERTEX_ATTRIB_BINDINGS = 32; VertexArrayObject(Uint externIndex); +#if MOBILEGL_PIPE_PUSH + // P2 step e2. Out of line, and declared only where there is a notice to raise: + // in a pull build this class keeps its implicit destructor, which is what keeps + // the pull build's symbol set byte-for-byte the pre-P2 one (G1). + ~VertexArrayObject(); +#endif void EnableAttribute(Uint index); void DisableAttribute(Uint index); diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 933b5b72..3ae3531a 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -41,7 +41,10 @@ #include #include #include +#include #include +#include +#include #include #include #include @@ -3375,24 +3378,20 @@ TEST(DirectGLESSlotTable, TwoTablesOfTheSameKindShareOneSlotAndKeepTheirOwnTwin) << "the shared slot outlived both holders and the object"; } -// The sweep has TWO drivers and the table must carry both. Nothing below calls -// CollectGarbageIfNeeded() or CollectGarbageNow(): this is the CREATION-driven half, and it is -// the one that matters for a workload that churns objects without drawing much. The draw-path -// tick is 1024 CollectGarbageIfNeeded calls, i.e. ~100 CTS-shaped cases at ~10 per-draw ticks -// each, which is how the registry this replaces came to hold ~100 cases' worth of dead, -// gigabyte-sized twins at once before the creation tick was added to fix it. -TEST(DirectGLESSlotTable, ObjectChurnAloneDrivesTheSweep) { +// The sweep and both of its drivers are RETIRED on this arm (ROADMAP.md:18's "delete the GC"), +// and this is the property that replaces them. The registry this table replaces learned of a +// death only by finding an expired weak_ptr, so it needed a 1024-call draw tick AND a +// 64-creation tick and still held up to 64 dead, gigabyte-sized twins at once. An announced +// death returns the slot before the next creation asks for one, so NOTHING accumulates - +// nothing below calls CollectGarbageIfNeeded() or CollectGarbageNow(), and on this arm the +// former does nothing at all. +TEST(DirectGLESSlotTable, AnnouncedDeathKeepsObjectChurnFromAccumulatingWithoutASweep) { using namespace MobileGL; auto& slots = MG_Pipe::MGPipeSlots(); - const Uint32 interval = FakeSlotTable::CreationGCIntervalForTest(); - // The churn count is a FIXED constant, not a multiple of the interval: a negative control - // that pushes the interval out of reach must make this case go red, not make it run for - // 2^32 iterations. constexpr Uint32 kChurn = 256u; - ASSERT_GT(interval, 0u); - ASSERT_LT(interval, kChurn) << "the creation sweep can no longer fire inside this case"; const Uint32 highWaterBefore = slots.HighWater(MG_Pipe::MGPipeKind::Query); + const Uint32 liveBefore = slots.LiveCount(MG_Pipe::MGPipeKind::Query); FakeSlotTable table; Uint32 peakLive = 0; @@ -3400,15 +3399,21 @@ TEST(DirectGLESSlotTable, ObjectChurnAloneDrivesTheSweep) { auto object = MakeShared(0xF0000000ull + i); table.GetOrCreate(object) = MakeShared(); peakLive = std::max(peakLive, table.LiveCount()); - // The object dies here. NOTHING announces that to the table (step e2 is not landed); - // the only thing that can notice is a sweep. + // What a real object's destructor raises. FakeStateObject is not one of the six + // re-keyed frontend classes, so the firing half is driven by hand here; that those six + // classes really do fire it is EveryReKeyedObjectClassAnnouncesItsOwnDeath below, and + // that the registries answer it per kind is + // EverySwitchedOverKindResolvesItsTwinThroughTheHandleArm. + EXPECT_TRUE(table.DestroyByLifetimeId(object->GetLifetimeId())); } - EXPECT_LE(peakLive, interval + 2u) - << peakLive << " dead twins accumulated at once with " << kChurn - << " objects churned - object churn stopped driving the sweep"; - EXPECT_LE(table.LiveCount(), interval + 2u); - EXPECT_LE(slots.HighWater(MG_Pipe::MGPipeKind::Query) - highWaterBefore, interval + 2u) + EXPECT_EQ(peakLive, 1u) + << peakLive << " twins were live at once with " << kChurn + << " objects churned and every death announced - the notice stopped freeing the twin"; + EXPECT_EQ(table.LiveCount(), 0u); + EXPECT_EQ(slots.LiveCount(MG_Pipe::MGPipeKind::Query), liveBefore) + << "the churn leaked slots the announced deaths should have returned"; + EXPECT_LE(slots.HighWater(MG_Pipe::MGPipeKind::Query) - highWaterBefore, 1u) << "the slot space grew with the churn instead of being recycled"; } @@ -3459,45 +3464,57 @@ TEST(DirectGLESSlotTable, AnAnnouncedDeathReturnsTheSlotWithoutASweep) { EXPECT_FALSE(other.DestroyByLifetimeId(object->GetLifetimeId())); } -// The firing side of e2, on the two of the six object classes whose files this package owns. -// The notice has to arrive when the LAST SharedPtr drops - not when glDeleteProgram marks the -// name, because a still-bound object goes on living - so the object is simply dropped here. -TEST(DirectGLESSlotTable, AProgramAndARenderbufferAnnounceTheirOwnDeath) { +// The firing side of e2, on ALL SIX re-keyed object classes - the round-3 review's MAJOR 3. +// Until this round only ProgramObject and RenderbufferObject raised the notice and the other +// four discovered their death in a sweep; the sweep is now retired, so a class that stopped +// announcing would leak its twin and the driver storage that twin owns for the life of the +// process. The notice has to arrive when the LAST SharedPtr drops - not when glDelete* marks +// the name, because a still-bound object goes on living - so the objects are simply dropped. +TEST(DirectGLESSlotTable, EveryReKeyedObjectClassAnnouncesItsOwnDeath) { using namespace MobileGL; - struct Notice { - MG_Pipe::MGPipeKind kind = MG_Pipe::MGPipeKind::None; - Uint64 lifetimeId = 0; - }; - static Vector notices; + static Vector> notices; notices.clear(); const MG_State::GLState::StateObjectDeathOps recording = { .OnDestroyed = [](MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) { - notices.push_back(Notice{kind, lifetimeId}); + notices.emplace_back(kind, lifetimeId); }, }; const MG_State::GLState::StateObjectDeathOps* previous = MG_State::GLState::GetStateObjectDeathOps(); MG_State::GLState::SetStateObjectDeathOps(&recording); - Uint64 programId = 0; - Uint64 renderbufferId = 0; + Vector> expected; { auto program = MakeShared(0u); - programId = program->GetLifetimeId(); auto renderbuffer = MakeShared(0u); - renderbufferId = renderbuffer->GetLifetimeId(); + auto texture = MakeShared(0u); + auto framebuffer = MakeShared(1u); + auto sampler = MakeShared(0u); + auto vertexArray = MakeShared(0u); + + expected.emplace_back(MG_Pipe::MGPipeKind::ShaderCso, program->GetLifetimeId()); + expected.emplace_back(MG_Pipe::MGPipeKind::Renderbuffer, renderbuffer->GetLifetimeId()); + expected.emplace_back(MG_Pipe::MGPipeKind::Texture, texture->GetLifetimeId()); + expected.emplace_back(MG_Pipe::MGPipeKind::Framebuffer, framebuffer->GetLifetimeId()); + expected.emplace_back(MG_Pipe::MGPipeKind::SamplerCso, sampler->GetLifetimeId()); + expected.emplace_back(MG_Pipe::MGPipeKind::VertexElementsCso, vertexArray->GetLifetimeId()); + EXPECT_TRUE(notices.empty()) << "a live object announced its own death"; } MG_State::GLState::SetStateObjectDeathOps(previous); - ASSERT_EQ(notices.size(), 2u); - // Destruction is reverse of construction, so the renderbuffer speaks first. - EXPECT_EQ(notices[0].kind, MG_Pipe::MGPipeKind::Renderbuffer); - EXPECT_EQ(notices[0].lifetimeId, renderbufferId); - EXPECT_EQ(notices[1].kind, MG_Pipe::MGPipeKind::ShaderCso); - EXPECT_EQ(notices[1].lifetimeId, programId); + // Membership rather than a count or an order: every TextureObjectBase owns a private + // SamplerObject (TextureObject.cpp), so tearing a texture down legitimately raises a + // SamplerCso notice as well. What must hold is that each of the six classes announced its + // OWN id under its OWN kind. + for (const auto& want : expected) { + EXPECT_NE(std::find(notices.begin(), notices.end(), want), notices.end()) + << "kind " << static_cast(want.first) << " lifetime id " << want.second + << " was destroyed without announcing it, so its twin would wait for a sweep that " + "this arm no longer runs"; + } } // ... and that the backend actually installs a consumer for it, rather than the two halves @@ -3635,7 +3652,7 @@ TEST(DirectGLESSlotTable, TwoTablesOfTheSameKindShareOneSlotAndKeepTheirOwnTwin) GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; } -TEST(DirectGLESSlotTable, ObjectChurnAloneDrivesTheSweep) { +TEST(DirectGLESSlotTable, AnnouncedDeathKeepsObjectChurnFromAccumulatingWithoutASweep) { GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; } @@ -3651,7 +3668,7 @@ TEST(DirectGLESSlotTable, AnAnnouncedDeathReturnsTheSlotWithoutASweep) { GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; } -TEST(DirectGLESSlotTable, AProgramAndARenderbufferAnnounceTheirOwnDeath) { +TEST(DirectGLESSlotTable, EveryReKeyedObjectClassAnnouncesItsOwnDeath) { GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; }