diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp index 4814410e..3d4e71ed 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp @@ -240,7 +240,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { res.Slot, res.Gen); std::abort(); } - DeferRelease(std::move(resource->buffer)); + // THE SERIAL IS READ BEFORE THE RESET BELOW ZEROES IT. Whether it is zero - whether any + // GPU command has named this store - decides whether the orphan can go at once or has + // to wait for a submission (DeferredWireRelease), and three lines from now the record + // no longer carries it. + const Uint64 orphanedUseSerial = resource->lastUseSerial; + DeferWireRelease(std::move(resource->buffer), orphanedUseSerial); resource->size = desc.Width; resource->lastUseSerial = 0; resource->gpuWritesPending = false; @@ -262,6 +267,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { res.Slot, res.Gen, static_cast(resource->size)); std::abort(); } + ++m_wireStoreCount; // Under transport, defined bytes follow as resource_subdata records. An // undefined store has no shadow to upload and no implicit zero snapshot. if (initialBytes != nullptr && desc.HasDefinedContent) { @@ -570,10 +576,77 @@ namespace MobileGL::MG_Backend::DirectVulkan { void VkBufferManager::DestroyWireBuffer(MG_Pipe::MGPipeHandle res) { const auto found = m_wireBuffers.find(WireBufferKey(res)); if (found == m_wireBuffers.end()) return; - DeferRelease(std::move(found->second.buffer)); + DeferWireRelease(std::move(found->second.buffer), found->second.lastUseSerial); m_wireBuffers.erase(found); ++m_sliceEpochCounter; } + + void VkBufferManager::DeferWireRelease(VkBufferObject&& buffer, Uint64 lastUseSerial) { + if (!buffer.IsValid()) return; + // Two stores go NOW rather than into the list. With no frame slots (DeferRelease's own + // teardown guard, and for its reason) there is no manager left to reclaim anything + // later. And a store whose last-use serial is 0 was never named by a GPU command since + // it was minted or since a host-access wait retired everything recorded before it - see + // DeferredWireRelease - so there is nothing to wait for. + if (m_deferredBufferReleases.empty() || lastUseSerial == 0) { + buffer.Destroy(); + if (m_wireStoreCount > 0) --m_wireStoreCount; + return; + } + DeferredWireRelease entry; + entry.lastUseSerial = lastUseSerial; + // The last submission that can name this store: see DeferredWireRelease. Zero when there + // is no renderer, which is also when there is no queue and nothing to wait for. + entry.submitIndex = pVulkanRenderer != nullptr ? pVulkanRenderer->GetSyncPointSubmitIndex() : 0; + entry.bytes = static_cast(buffer.GetSize()); + entry.buffer = std::move(buffer); + m_deferredWireBytes += entry.bytes; + m_deferredWireReleases.push_back(std::move(entry)); + // EVERY PARK SWEEPS. The frame boundary is not a reclaim point this arm can lean on - a + // pbuffer replay that snapshot-exits delivers ONE present for 1.3 M calls (ID-P7-32) - + // so the only cadence that tracks the workload is the workload itself. + SweepDeferredWireReleases(); + } + + SizeT VkBufferManager::SweepDeferredWireReleases() { + if (m_deferredWireReleases.empty() || pVulkanRenderer == nullptr) return 0; + SizeT dead = 0; + if (pVulkanRenderer->IsSubmitIndexComplete(pVulkanRenderer->GetSyncPointSubmitIndex())) { + // Idle: nothing recorded, every submission retired. Every parked store is dead, + // including one tagged for a batch that was abandoned instead of submitted. + dead = m_deferredWireReleases.size(); + } else { + // The list is in park order and GetSyncPointSubmitIndex() never decreases, so the + // submit indices are non-decreasing and the dead entries are a PREFIX: the first + // entry the renderer will not call complete proves none after it is either. That + // bounds the walk to a couple of fence polls, which matters because this runs on + // every glBufferData the wire carries. + while (dead < m_deferredWireReleases.size() && + pVulkanRenderer->IsSubmitIndexComplete(m_deferredWireReleases[dead].submitIndex)) { + ++dead; + } + } + if (dead == 0) return 0; + for (SizeT i = 0; i < dead; ++i) { + DeferredWireRelease& entry = m_deferredWireReleases[i]; + m_deferredWireBytes -= entry.bytes; + entry.buffer.Destroy(); + } + m_deferredWireReleases.erase(m_deferredWireReleases.begin(), + m_deferredWireReleases.begin() + static_cast(dead)); + const Uint64 destroyedCount = static_cast(dead); + m_wireStoreCount = destroyedCount <= m_wireStoreCount ? m_wireStoreCount - destroyedCount : 0; + return dead; + } + + void VkBufferManager::DestroyAllDeferredWireReleases() { + for (auto& entry : m_deferredWireReleases) { + entry.buffer.Destroy(); + if (m_wireStoreCount > 0) --m_wireStoreCount; + } + m_deferredWireReleases.clear(); + m_deferredWireBytes = 0; + } #endif Bool VkBufferManager::Initialize(const VkBufferManagerInitInfo& initInfo) { @@ -617,7 +690,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { DestroyAllDeferredReleases(); ReleaseAllLiveResources(); #if MOBILEGL_BUILD_DISAGGREGATED + // DestroyAllDeferredReleases above emptied the parked list; this destroys the stores the + // records still hold, so nothing this arm minted outlives the count. m_wireBuffers.clear(); + m_wireStoreCount = 0; #endif m_copyProvider = nullptr; m_initInfo = {}; @@ -1334,6 +1410,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { "VkBufferManager::CollectDeferredReleases frame index out of range"); m_deferredBufferReleases[frameIndex].clear(); m_deferredResourceReleases[frameIndex].clear(); +#if MOBILEGL_BUILD_DISAGGREGATED + // The wire list is not per-slot and does not wait for a frame boundary; a boundary is + // just one more point to sweep at (BeginFrame after its slot fence, and every slot of + // the idle drain, where the renderer-idle rule empties the list). + SweepDeferredWireReleases(); +#endif } BufferSlice VkBufferManager::AcquireUnboundStorageDescriptor() { @@ -1429,6 +1511,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { } void VkBufferManager::DestroyAllDeferredReleases() { +#if MOBILEGL_BUILD_DISAGGREGATED + // Both callers (Shutdown, RecreateTransientArenas) have proven the device idle. + DestroyAllDeferredWireReleases(); +#endif for (auto& releases : m_deferredBufferReleases) { for (auto& buffer : releases) { buffer.Destroy(); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h index 77f2211d..b923cf6f 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h @@ -244,6 +244,63 @@ namespace MobileGL::MG_Backend::DirectVulkan { } WireBufferResource* FindWireBuffer(MG_Pipe::MGPipeHandle res); Bool WaitForWireBufferHostAccess(WireBufferResource& resource); + + // P7 wave 4 (M2), ID-P7-27: THE WIRE ARM'S ORPHANS NEED A RECLAIM THAT IS NOT A FRAME + // BOUNDARY. + // + // RespecifyWireBuffer - which is every glBufferData that crosses the wire - orphans the + // old store unconditionally, because unlike OnRespecify it has no shadow to upload in + // place from and no cheap way to know the client is re-sending the same size. The orphan + // is legitimate (M1 measured the conditional-orphan mirror: 6701 live against 6702). + // What was missing is the RECLAIM. DeferRelease parks into m_deferredBufferReleases, + // whose only sweep is CollectDeferredReleases from a frame boundary, and on a pbuffer + // replay the server sees ONE present record for the whole run - so on + // minecraft-1.21.4-fabric-iris-bsl-esc-menu-854 the buckets held 25,923 dead stores + // against 28 live wire buffers, one memfd mapping each, and the server died in scudo's + // secondary allocator. + // + // So a wire store is parked HERE instead, with the facts that say when it is dead, and + // THE DEFER PATH ITSELF RECLAIMS - every park sweeps, and the frame boundary is only one + // more sweep point (CollectDeferredReleases), never the one this depends on: + // + // * `lastUseSerial == 0` - read BEFORE RespecifyWireBuffer zeroes it. Zero means no + // GPU command has named the store since it was minted or since the last host-access + // wait proved every recorded command complete (WaitForWireBufferHostAccess); every + // path that hands the store to the GPU stamps m_frameSerial, which starts at 1. Such + // a store is destroyed at the park and never enters the list. + // * `submitIndex` - the renderer's GetSyncPointSubmitIndex() at park time, which is by + // construction the LAST submission that can name this store: every command that + // names it was recorded before the park (the record left behind a bumped slice + // epoch, so no memo can hand it out again), and a recorded command is either already + // submitted (<= m_submitCounter) or in the batch that becomes m_submitCounter + 1. + // IsSubmitIndexComplete(submitIndex) is a fence observation, so this is the gate + // that empties the set MID-FRAME - the shape CollectWireObjects already uses for + // the wire image/view tables. + // * THE RENDERER IS IDLE - IsSubmitIndexComplete(GetSyncPointSubmitIndex()): nothing + // is recorded and every submission has retired. Then every parked store is dead, + // including one tagged for a batch that was abandoned rather than submitted (a + // minimized Present drops its recording), whose own index may never complete. + // + // THE FRAME-SERIAL FLOOR IS DELIBERATELY NOT A PROOF HERE. It cannot move inside a + // frame (NotifyFrameSerialComplete refuses the current serial), so it frees nothing in + // the one-present replay this exists for, and on this package's base it is the floor + // B3 found unsound (a serial with two submissions was declared complete on the first + // fence, 6a92a10dd): a free on that floor is a GPU use-after-free, not a wrong pixel. + struct DeferredWireRelease { + VkBufferObject buffer; + Uint64 lastUseSerial = 0; + Uint64 submitIndex = 0; + // Cached: GetSize() is gone once the object is destroyed, and the watermark's + // running total has to stay exact as entries leave. + Uint64 bytes = 0; + }; + // Park a wire store, then sweep. `lastUseSerial` is the releasing resource's, captured + // before the caller resets it. + void DeferWireRelease(VkBufferObject&& buffer, Uint64 lastUseSerial); + // Destroy every parked store proven dead by the rules above. Returns how many. + SizeT SweepDeferredWireReleases(); + // Teardown: the caller has proven the device idle (Shutdown / RecreateTransientArenas). + void DestroyAllDeferredWireReleases(); #endif Bool InitializeTransientArenas(); static VkBufferUsageFlags GetVkBufferUsage(BufferKind kind); @@ -280,6 +337,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { Vector> m_liveResources; #if MOBILEGL_BUILD_DISAGGREGATED std::unordered_map m_wireBuffers; + // See DeferredWireRelease. ONE FLAT LIST rather than the per-frame-slot buckets above: + // the whole point is that these entries do not wait for a frame slot to come round. + Vector m_deferredWireReleases; + // Bytes currently parked in that list, kept exact so the watermark needs no walk. + Uint64 m_deferredWireBytes = 0; + // Live VkBuffers this arm owns: the stores held by m_wireBuffers plus the parked ones. + // Maintained rather than counted, because the publish runs on every park. + Uint64 m_wireStoreCount = 0; #endif // Size m_liveResources had just after the last sweep; the next sweep waits for it to double. SizeT m_liveResourcesLastPruned = 0;