From 3594f03c4eeae985827b8e5678e96e8b8876b49d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 6 Sep 2026 08:31:13 -0400 Subject: [PATCH] [Refactor] (State, Magma): take the backend's raw pointers out of the frontend VAO - the hash and state memos become the factory's own per-slot fields - P2 D12.5 (ARCHITECTURE.md 9.5). VertexArrayObject carried three `mutable` memos for the backend: a content hash, a raw pointer into VertexInputStateFactory's heap-allocated cache entry plus that cache's eviction epoch, and two aux words. A frontend state object holding the backend's pointer is what P2 retires - under split the backend is in another process and its cache entry has no address a client could store. - The hash and state memos move into a slot-indexed table the FACTORY owns, keyed on the VAO's {slot, gen} and guarded by exactly the same config version, so nothing is recomputed more often than it was. Fixed and direct-mapped for the same reason m3's VaoDrawMemo table is: nothing frees a VertexElementsCso slot in P2, so a grow-on-demand table would keep one entry per VAO ever created. 2048 x 48 B is 96 KB. - The AUX memo is deleted rather than moved, as the brief says: its two words already live in VulkanRenderer::VaoDrawMemo (layoutHash / layoutAuxMasks) and GetBackendAuxMemo has no live reader anywhere in the tree - the only writer was the line this commit stops executing. - The eviction-epoch dance shrinks with them. The PROCESS-WIDE s_evictionEpochSource exists because the memos live on frontend VAOs and therefore outlive the factory; the handle arm's table dies with the factory, so a per-instance counter is enough there. The epoch itself stays - it guards the POINTEE, which is still a cache entry a frame boundary can erase, and moving the memo does not change that. (The brief reads as if a slot-indexed table removes the need for an epoch; it removes the need for a process-wide one.) - The three draw-path readers that asked the VAO "is your content hash already memoized?" now ask whichever side owns the memo, through a force-inlined wrapper so the PULL build's two loads stay two loads. - All three accessors and their storage are kept under MOBILEGL_PIPE_LEGACY_MEMOS rather than deleted from the file, because that is the arm the pre-handle A/B runs (D14) and because a pull build forces the option ON, where G1 admits no change at all. Configuring with -DMOBILEGL_PIPE_LEGACY_MEMOS=OFF is what makes the deletion real, and that build compiles clean - which is the check that nothing else still reaches for them. - Verification: pull symbol_report --threshold 0 is 0 added / 0 removed / 0 renamed with the contract's four resizes and no fifth; ctest -L unit 1489/1489 in both the pull and the push build; ctest -L integration-gpu -R DirectVulkan 432/432 under the default bitmask and 432/432 under MOBILEGL_PIPE_PUSH=0. The LEGACY_MEMOS=OFF build compiles but cannot RUN on this tree, and that is the D14 gate working rather than a defect: no tracker binds a render-state CSO here, so the handle arm has no key and Fatal{PipeLegacyMemosDisabled} fires at the first draw instead of the memo quietly aliasing every render state onto one entry. Re-run it once p2/tracker has landed. --- .../Renderer/VertexInputStateFactory.cpp | 105 +++++++++++++++++- .../Renderer/VertexInputStateFactory.h | 58 ++++++++++ .../DirectVulkan/Renderer/VulkanRenderer.cpp | 6 +- .../DirectVulkan/Renderer/VulkanRenderer.h | 12 ++ .../VertexArrayState/VertexArrayObject.h | 22 ++++ 5 files changed, 199 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp index c1bc968f..bb550ce0 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp @@ -77,18 +77,114 @@ namespace MobileGL::MG_Backend::DirectVulkan { return XXH64_digest(m_hashState); } +#if MOBILEGL_PIPE_PUSH + VertexInputStateFactory::VaoBackendMemos& VertexInputStateFactory::MemosFor( + const MG_State::GLState::VertexArrayObject& vao) const { + if (m_vaoMemos.empty()) { + m_vaoMemos.resize(kVaoMemoSlotCount); + } + const Uint64 lifetimeId = vao.GetLifetimeId(); + if (!m_lastVaoHandleValid || m_lastVaoLifetimeId != lifetimeId) { + m_lastVaoHandle = MagmaPipeHandleOf(MG_Pipe::MGPipeKind::VertexElementsCso, lifetimeId); + m_lastVaoLifetimeId = lifetimeId; + m_lastVaoHandleValid = true; + } + const MG_Pipe::MGPipeHandle handle = m_lastVaoHandle; + VaoBackendMemos& memos = m_vaoMemos[handle.Slot & (kVaoMemoSlotCount - 1)]; + if (!(memos.Owner == handle)) { + // Someone else's entry (a colliding slot, or a slot whose Gen moved because the + // slot was REUSED for a different object). Claim it, contents cleared - never + // inherited, which is the whole point of keying on the generation. + memos = VaoBackendMemos{}; + memos.Owner = handle; + } + return memos; + } +#endif + +#if MOBILEGL_PIPE_PUSH + Bool VertexInputStateFactory::TryGetMemoizedHash(const MG_State::GLState::VertexArrayObject& vao, + Uint64& outHash) const { + if (MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput)) { + const VaoBackendMemos& memos = MemosFor(vao); + if (memos.HashConfigVersion != vao.GetConfigVersion()) return false; + outHash = memos.Hash; + return true; + } + MagmaPipeRequireLegacyArm("VertexInputStateFactory::TryGetMemoizedHash"); +#if MOBILEGL_PIPE_LEGACY_MEMOS + return vao.GetBackendHashMemo(outHash); +#else + return false; +#endif + } +#endif + VertexInputStateFactory::HashType VertexInputStateFactory::GetOrComputeHash( const MG_State::GLState::VertexArrayObject& vao) const { HashType hash = 0; +#if MOBILEGL_PIPE_PUSH + // P2 D12.5: the same memo, on the backend's side of the boundary. + if (MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput)) { + VaoBackendMemos& memos = MemosFor(vao); + if (memos.HashConfigVersion == vao.GetConfigVersion()) { + return memos.Hash; + } + hash = ComputeHash(vao); + memos.Hash = hash; + memos.HashConfigVersion = vao.GetConfigVersion(); + return hash; + } + MagmaPipeRequireLegacyArm("VertexInputStateFactory::GetOrComputeHash"); +#endif +#if MOBILEGL_PIPE_LEGACY_MEMOS if (!vao.GetBackendHashMemo(hash)) { hash = ComputeHash(vao); vao.SetBackendHashMemo(hash); } +#endif return hash; } const VertexInputStateFactory::BackendVertexInputState& VertexInputStateFactory::GetOrCreateVertexInputState( const MG_State::GLState::VertexArrayObject& vao) { +#if MOBILEGL_PIPE_PUSH + // P2 D12.5: the same per-draw fast path, but the resolved-entry pointer lives in this + // factory's slot-indexed table instead of on the frontend VAO. The eviction epoch + // survives the move and is still what stops a stale pointer being dereferenced: the + // POINTEE is a cache entry this factory can erase at a frame boundary, and moving the + // memo does not change that. + if (MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput)) { + VaoBackendMemos& memos = MemosFor(vao); + if (memos.StateConfigVersion == vao.GetConfigVersion() && memos.State != nullptr && + memos.StateEpoch == m_evictionEpoch) { + const auto* memoEntry = static_cast(memos.State); + memoEntry->lastUsedFrameBoundary = m_frameBoundaryCounter; + return *memoEntry; + } + const BackendVertexInputState& resolved = + GetOrCreateVertexInputState(vao, GetOrComputeHash(vao)); + // MemosFor is re-taken: GetOrComputeHash above went through it, and a colliding + // VAO could have claimed the entry in between (it cannot here, since both calls + // name the same VAO, but the reference is not worth keeping live across a call + // that can resize the table). + VaoBackendMemos& stamp = MemosFor(vao); + stamp.State = &resolved; + stamp.StateEpoch = m_evictionEpoch; + stamp.StateConfigVersion = vao.GetConfigVersion(); + // The AUX memo is deliberately NOT stamped here: its two words already live in + // VulkanRenderer::VaoDrawMemo (layoutHash / layoutAuxMasks) and its getter has no + // live reader anywhere, so the handle arm retires it rather than moving it. + return resolved; + } + MagmaPipeRequireLegacyArm("VertexInputStateFactory::GetOrCreateVertexInputState"); +#endif +#if !MOBILEGL_PIPE_LEGACY_MEMOS + // Unreachable: with no legacy arm compiled, MagmaPipeRequireLegacyArm above aborts. + // Written out rather than left to fall off the end so the function still has a return + // on every path a compiler can see. + return GetOrCreateVertexInputState(vao, GetOrComputeHash(vao)); +#else // Per-draw fast path: the VAO carries a pointer to its resolved entry, // valid while its config version and the cache's eviction epoch both // match - no re-hash, no map lookup. @@ -108,6 +204,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { vao.SetBackendAuxMemo(entry.layoutHash, PackVertexInputAuxMasks(entry.unsupportedAttribMask, entry.attributeLocationMask)); return entry; +#endif // MOBILEGL_PIPE_LEGACY_MEMOS } const VertexInputStateFactory::BackendVertexInputState& VertexInputStateFactory::GetOrCreateVertexInputState( @@ -341,8 +438,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { // Invalidate every VAO's state-pointer memo: the erased node's // address may be reused by a future insert. Advance through the // process-wide source so the value stays unique across factory - // instances (see the member comment). + // instances (see the member comment). With no legacy arm the memos + // live in this factory and die with it, so a per-instance bump is + // enough - P2 D12.5. +#if MOBILEGL_PIPE_LEGACY_MEMOS m_evictionEpoch = ++s_evictionEpochSource; +#else + ++m_evictionEpoch; +#endif } else { ++it; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h index 66dd6958..83174c14 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h @@ -7,6 +7,9 @@ // End of Source File Header #pragma once +// MG_Pipe::MGPipeHandle for the P2 D12.5 memo table below. A header of constexpr constants, +// so the pull build gains nothing from it. +#include #include "Config.h" #include "VertexInputStateBuilder.h" @@ -86,6 +89,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { // Memoized ComputeHash: reuses the VAO's cached hash while its config version // is unchanged. Use this on per-draw paths. HashType GetOrComputeHash(const MG_State::GLState::VertexArrayObject& vao) const; +#if MOBILEGL_PIPE_PUSH + // The VAO's content hash IF it has already been memoized, without computing one. + // P2 D12.5: the three draw-path readers that used to ask the VAO object this + // question ask the factory instead, because that is where the memo lives once the + // frontend object stops carrying the backend's state. + Bool TryGetMemoizedHash(const MG_State::GLState::VertexArrayObject& vao, Uint64& outHash) const; +#endif const BackendVertexInputState& GetOrCreateVertexInputState( const MG_State::GLState::VertexArrayObject& vao, HashType hash); const BackendVertexInputState& GetOrCreateVertexInputState(const MG_State::GLState::VertexArrayObject& vao); @@ -112,6 +122,45 @@ namespace MobileGL::MG_Backend::DirectVulkan { static VkFormat ToFloat32VertexFormat(Int componentCount); Bool SupportsVertexBufferFormat(VkFormat format) const; +#if MOBILEGL_PIPE_PUSH + // ---- P2 D12.5: the backend's memos, off the frontend VAO and into the backend ---- + // + // The two facts that used to live as `mutable` fields on VertexArrayObject + // (Get/SetBackendHashMemo and Get/SetBackendStateMemo), kept here instead, keyed on + // the VAO's {slot, gen} and guarded by exactly the same config version. A frontend + // state object holding the backend's raw pointer is what P2 retires: under split the + // backend is in another process and its cache entry has no address a client could + // store, so the memo has to live on the side that owns the pointee. + // + // The AUX memo is not carried over: its two words moved into VaoDrawMemo::layoutHash + // and layoutAuxMasks long ago and its getter has no live reader anywhere in the tree, + // so the handle arm simply stops writing it (D12.5 says delete rather than move). + struct VaoBackendMemos { + // Whose memos these are. A slot is direct-mapped into the table below, so an + // entry can be claimed by a different VAO; the handle compare is what says the + // contents are this object's. + MG_Pipe::MGPipeHandle Owner = MG_Pipe::kMGPipeNullHandle; + Uint64 Hash = 0; + Uint32 HashConfigVersion = ~0u; + const void* State = nullptr; + Uint64 StateEpoch = 0; + Uint32 StateConfigVersion = ~0u; + }; + // Fixed and direct-mapped for the same reason VulkanRenderer's VaoDrawMemo table is + // (P2 m3): nothing frees a VertexElementsCso slot yet, so a grow-on-demand table + // would keep one entry per VAO ever created. 2048 x 48 B is 96 KB. + static constexpr Uint32 kVaoMemoSlotCount = 2048; // power of two + mutable Vector m_vaoMemos; + // One-entry memo in front of the allocator's lifetimeId -> handle probe, same shape + // and same reason as VulkanRenderer::ResolveVaoHandle. + mutable Uint64 m_lastVaoLifetimeId = 0; + mutable MG_Pipe::MGPipeHandle m_lastVaoHandle = MG_Pipe::kMGPipeNullHandle; + mutable Bool m_lastVaoHandleValid = false; + // The entry belonging to `vao`, claimed (and cleared) if the slot currently holds + // someone else's. + VaoBackendMemos& MemosFor(const MG_State::GLState::VertexArrayObject& vao) const; +#endif + const VulkanRendererConfig& m_config; VkPhysicalDevice m_physicalDevice = VK_NULL_HANDLE; // Values are heap-allocated: UnorderedMap is open-addressing, so INSERT @@ -137,8 +186,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { // than anything a predecessor ever stamped, so a dead factory's memo can // never compare equal here - the same never-reused idiom as the lifetime ids. // Single-threaded like the rest of the factory (renderer-thread only). + // + // P2 D12.5: the process-wide source is the LEGACY arm's need. It exists because the + // memos live on the frontend VAOs and therefore outlive the factory. The handle arm's + // memo table is owned by this factory and dies with it, so a per-instance counter is + // enough there and the epoch shrinks back to what it looks like it should be. +#if MOBILEGL_PIPE_LEGACY_MEMOS static inline Uint64 s_evictionEpochSource = 0; Uint64 m_evictionEpoch = ++s_evictionEpochSource; +#else + Uint64 m_evictionEpoch = 1; +#endif static inline XXH64_state_t* m_hashState = XXH64_createState(); }; } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 02fdf4c5..25e5ad06 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3749,7 +3749,7 @@ void main() { VaoDrawMemo* slot = nullptr; ResolvedVertexBindings* memo = nullptr; Uint64 vaoContentHash = 0; - const Bool vaoHashKnown = vao.GetBackendHashMemo(vaoContentHash); + const Bool vaoHashKnown = VaoContentHashIfKnown(vao, vaoContentHash); if (vaoHashKnown) { slot = LookupVaoDrawMemo(&vao); memo = &slot->bindings; @@ -6385,7 +6385,7 @@ void main() { Uint64 auxMasks = 0; Bool factsKnown = false; Uint64 contentHash = 0; - if (vao.GetBackendHashMemo(contentHash)) { + if (VaoContentHashIfKnown(vao, contentHash)) { const VaoDrawMemo* vaoMemo = LookupVaoDrawMemo(&vao); if (vaoMemo->layoutFactsValid && vaoMemo->contentHash == contentHash) { vaoLayoutHash = vaoMemo->layoutHash; @@ -6402,7 +6402,7 @@ void main() { auxMasks = VertexInputStateFactory::PackVertexInputAuxMasks( vertexInputState.unsupportedAttribMask, vertexInputState.attributeLocationMask); Uint64 stampedHash = 0; - if (vao.GetBackendHashMemo(stampedHash)) { + if (VaoContentHashIfKnown(vao, stampedHash)) { VaoDrawMemo* vaoMemo = LookupVaoDrawMemo(&vao); vaoMemo->contentHash = stampedHash; vaoMemo->layoutHash = vaoLayoutHash; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index f992b9e5..fa95d909 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -1387,6 +1387,18 @@ namespace MobileGL::MG_Backend::DirectVulkan { return handle; } #endif + // "Is this VAO's content hash already memoized?", asked of whichever side owns the + // memo (P2 D12.5). Force-inlined and defined in the class body so that the PULL + // build's three readers keep compiling to the very same two loads they always did - + // G1 admits no resize, and an out-of-line call here would be one. + [[gnu::always_inline]] inline Bool VaoContentHashIfKnown( + const MG_State::GLState::VertexArrayObject& vao, Uint64& outHash) const { +#if MOBILEGL_PIPE_PUSH + return m_vertexInputStateFactory->TryGetMemoizedHash(vao, outHash); +#else + return vao.GetBackendHashMemo(outHash); +#endif + } // Finds the slot holding `vao`, or recycles the older of its two candidate // slots into an empty memo keyed on `vao`. Never returns null. VaoDrawMemo* LookupVaoDrawMemo(const MG_State::GLState::VertexArrayObject* vao); diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h index 41c5065b..9322fd35 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h @@ -106,6 +106,22 @@ namespace MobileGL { // "any vertex-input state changed" with one compare. Uint32 GetConfigVersion() const { return m_configVersion; } +#if MOBILEGL_PIPE_LEGACY_MEMOS + // ---- THE BACKEND'S THREE MEMOS ON THE FRONTEND OBJECT ---- + // + // P2 D12.5 (ARCHITECTURE.md 9.5) retires all three: a frontend state object + // must not hold the backend's raw pointers, and under split it cannot - the + // backend is in another process and its cache entry has no address the client + // could store. Magma's handle arm keeps the same three facts in a slot-indexed + // table it owns itself (VertexInputStateFactory::VaoBackendMemos), keyed on the + // VAO's {slot, gen} and validated by the same config version, so nothing is + // recomputed more often than it was. + // + // They stay compiled under MOBILEGL_PIPE_LEGACY_MEMOS - which a PULL build + // forces ON - because that is the arm the pre-handle A/B runs, and because G1 + // admits no change to the pull build. They are deleted outright with the pull + // path at P13. + // // Backend-owned content-hash memo, valid while the config version matches // (same idea as ProgramObject's hash memo — avoids re-hashing all // attributes on every draw). @@ -154,6 +170,7 @@ namespace MobileGL { m_backendAuxMemo1 = aux1; m_backendAuxMemoVersion = m_configVersion; } +#endif // MOBILEGL_PIPE_LEGACY_MEMOS private: void BumpAttributeFormatVersion(Uint index); @@ -193,6 +210,10 @@ namespace MobileGL { Array m_attributeUsesBindingModel = {}; Uint32 m_configVersion = 0; +#if MOBILEGL_PIPE_LEGACY_MEMOS + // The storage behind the three accessors above; retired with them (D12.5). + // A pull build forces MOBILEGL_PIPE_LEGACY_MEMOS ON, so sizeof(this) does not + // move there and G1 sees no change. mutable Uint64 m_backendHashMemo = 0; mutable Uint32 m_backendHashMemoVersion = ~0u; mutable const void* m_backendStateMemo = nullptr; @@ -201,6 +222,7 @@ namespace MobileGL { mutable Uint64 m_backendAuxMemo0 = 0; mutable Uint64 m_backendAuxMemo1 = 0; mutable Uint32 m_backendAuxMemoVersion = ~0u; +#endif // MOBILEGL_PIPE_LEGACY_MEMOS }; } // namespace GLState } // namespace MG_State