diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp index 1776f8d1..26220392 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp @@ -239,6 +239,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { resource->indirectReadBarrierPending = false; resource->stagedCoverage.clear(); DropWireShadow(*resource); + ReleaseRenameSpares(*resource); if (resource->size == 0) return; if (!CreateWireStoreBuffer(*resource)) { @@ -279,6 +280,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { resource.buffer.Map() != nullptr; } + void VkBufferManager::ReleaseRenameSpares(WireBufferResource& resource) { + for (auto& spare : resource.renameSpares) + DeferWireRelease(std::move(spare.buffer), spare.lastUseSerial != 0 ? spare.lastUseSerial : m_frameSerial); + resource.renameSpares.clear(); + } + void VkBufferManager::DropWireShadow(WireBufferResource& resource) { if (resource.shadow.empty()) return; m_wireShadowBytes -= resource.shadow.size(); @@ -300,19 +307,45 @@ namespace MobileGL::MG_Backend::DirectVulkan { MG_Config::Features.MagmaWriteRename == MG_Config::QuirkOverride::ForceOff) return false; VkBufferObject old = std::move(resource.buffer); - if (!CreateWireStoreBuffer(resource) || - !resource.buffer.Upload(resource.shadow.data(), static_cast(resource.size), 0)) { - resource.buffer.Destroy(); + Bool reused = false; + for (SizeT i = 0; pVulkanRenderer != nullptr && i < resource.renameSpares.size(); ++i) { + if (!pVulkanRenderer->IsSubmitIndexComplete(resource.renameSpares[i].submitIndex)) continue; + resource.buffer = std::move(resource.renameSpares[i].buffer); + resource.renameSpares.erase(resource.renameSpares.begin() + static_cast(i)); + reused = true; + break; + } + if (!reused) { + if (!CreateWireStoreBuffer(resource)) { + resource.buffer.Destroy(); + resource.buffer = std::move(old); + return false; + } + ++m_wireStoreCount; + } + if (!resource.buffer.Upload(resource.shadow.data(), static_cast(resource.size), 0)) { + // Keep the store on its old buffer; the new one goes the deferred way (it is idle). + DeferWireRelease(std::move(resource.buffer), 0); resource.buffer = std::move(old); return false; } - // Busy means a GPU command recorded so far may name the old buffer: release it behind - // this frame's serial and the current sync point, whatever its own stamps said. - DeferWireRelease(std::move(old), m_frameSerial); + // Busy means a GPU command recorded so far may name the old buffer: it waits behind this + // frame's serial and the current sync point, whatever its own stamps said - as a spare + // for the next rename, or (past the bound) through DeferWireRelease. + constexpr SizeT kRenameSpares = 4; + if (resource.renameSpares.size() >= kRenameSpares) { + auto& oldest = resource.renameSpares.front(); + DeferWireRelease(std::move(oldest.buffer), oldest.lastUseSerial); + resource.renameSpares.erase(resource.renameSpares.begin()); + } + WireBufferResource::RenameSpare spare; + spare.buffer = std::move(old); + spare.submitIndex = pVulkanRenderer != nullptr ? pVulkanRenderer->GetSyncPointSubmitIndex() : 0; + spare.lastUseSerial = m_frameSerial; + resource.renameSpares.push_back(std::move(spare)); resource.lastUseSerial = 0; resource.lastUseSubmitIndex = 0; resource.gpuWritesPending = false; - ++m_wireStoreCount; ++m_wireRenames; NoteWireStorePeaks(); PublishWireReclaimGauges(); @@ -1154,6 +1187,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { const auto found = m_wireBuffers.find(WireBufferKey(res)); if (found == m_wireBuffers.end()) return; DropWireShadow(found->second); + ReleaseRenameSpares(found->second); DeferWireRelease(std::move(found->second.buffer), found->second.imported ? T0ReleaseSerial(found->second.lastUseSerial) : found->second.lastUseSerial); // P11 B2, as Respecify diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h index ebce7b79..024369eb 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h @@ -204,6 +204,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { // WriteWireBuffer). Empty: no shadow (a large store, the budget was spent, or a GPU // write, shader-writable bind or client mapping made the host copy unknowable). Vector shadow; + // P15: buffers this store was renamed away from, kept for the next rename instead of + // allocating one (a UI store is renamed several times a frame). A spare is reused only + // once the submission that can name it has completed - DeferWireRelease's own rule - + // and its handle never died, so no handle-keyed memo can mistake it for another + // buffer. Bounded; the rest go through DeferWireRelease. + struct RenameSpare { + VkBufferObject buffer; + Uint64 submitIndex = 0; + Uint64 lastUseSerial = 0; + }; + Vector renameSpares; Bool gpuWritesPending = false; // Only ranges actually submitted by resource_subdata are covered. No // shadow is retained: flush cannot replay stale bytes over GPU writes. @@ -220,6 +231,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint64 m_wireRenames = 0; SizeT m_wireShadowBytes = 0; void DropWireShadow(WireBufferResource& resource); + void ReleaseRenameSpares(WireBufferResource& resource); Bool CreateWireStoreBuffer(WireBufferResource& resource); Bool RenameBusyWireStore(WireBufferResource& resource); static Uint64 WireBufferKey(MG_Pipe::MGPipeHandle res) { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/WriteRenameScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/WriteRenameScenario.cpp index 0629894d..85a3ab8b 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/WriteRenameScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/WriteRenameScenario.cpp @@ -220,6 +220,32 @@ void main() { o0 = vColor * uTint; } Destroy(s); } + // Two renames of ONE store inside one frame: the second must not reuse the buffer the first + // draw still reads (nothing is submitted yet, so a reused spare would be overwritten under it). + TEST_F(WriteRenameScenario, ThreeDrawsAroundTwoRenamesInOneFrameEachReadTheirOwnBytes) { + if (!Ready()) return; + std::vector v(4); + PutQuad(v, 0, 0.0f, 0.0f, 0.33f, 1.0f, 255, 0, 0); // left third, red + std::string error; + Scene s = MakeScene(v, &error); + ASSERT_NE(s.program, 0u) << error; + + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + std::vector middle(4), right(4); + PutQuad(middle, 0, 0.33f, 0.0f, 0.66f, 1.0f, 0, 255, 0); + glBufferSubData(GL_ARRAY_BUFFER, 0, static_cast(middle.size() * sizeof(Vertex)), middle.data()); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + PutQuad(right, 0, 0.66f, 0.0f, 1.0f, 1.0f, 0, 0, 255); + glBufferSubData(GL_ARRAY_BUFFER, 0, static_cast(right.size() * sizeof(Vertex)), right.data()); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + EXPECT_EQ(FirstGLError(), 0u); + + EXPECT_TRUE(Near(At(kSize / 6, kSize / 2), 255, 0, 0)) << "left: the first draw's bytes"; + EXPECT_TRUE(Near(At(kSize / 2, kSize / 2), 0, 255, 0)) << "middle: after the first rename"; + EXPECT_TRUE(Near(At(kSize * 5 / 6, kSize / 2), 0, 0, 255)) << "right: after the second rename"; + Destroy(s); + } + TEST_F(WriteRenameScenario, ARenamedStoreKeepsDrawingRightInLaterFrames) { if (!Ready()) return; std::vector v(4); diff --git a/tools/device_bench/disagg/bench_session.sh b/tools/device_bench/disagg/bench_session.sh index ba507585..5f75b349 100644 --- a/tools/device_bench/disagg/bench_session.sh +++ b/tools/device_bench/disagg/bench_session.sh @@ -31,6 +31,9 @@ case "${1:-status}" in SER=$SER bash "$HERE/pin_clocks.sh" pin "$0" status ;; stop) + # Trace-replay packages a job started must not idle on the device after it (they hold GPU + # memory and a surface): every MobileGL plugin package is force-stopped at session end. + su_sh "for p in \$(pm list packages top.mobilegl.plugin | sed s/package://); do am force-stop \$p; done" su_sh "if [ -f $MARK ]; then grep \"^svc \" $MARK | while read k s st; do [ \"\$st\" = running ] && setprop ctl.start \$s; done; else for s in $SERVICES; do setprop ctl.start \$s; done; fi" sleep 3 bad=$(su_sh "if [ -f $MARK ]; then grep \"^svc \" $MARK | while read k s st; do [ \"\$st\" = running ] && [ \"\$(getprop init.svc.\$s)\" != running ] && echo \$s; done; fi" | tr -d '\r')