diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 2113fec4..5ac0f989 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -505,9 +505,11 @@ namespace MobileGL::MG_Config { // exceed it after a sweep, the server flushes what it has recorded and waits for it // (WaitForWireBufferHostAccess's sync point, mid-frame), which retires every one. It is // not a frame count because a frame is not bounded: a snapshot-exiting pbuffer replay - // delivers one present for 1.3 M calls. 0 IS THE NEGATIVE CONTROL, not "unlimited by - // design": no forced sync, so a one-frame respecify-and-draw loop grows without bound - // and MagmaWireReclaimScenario's watermark case must go red. + // delivers one present for 1.3 M calls. The same sync point also fires above a fixed + // 1024 parked stores (VkBufferManager::kWireDeferredCountCeiling), because small + // orphans never reach a byte budget. 0 IS THE NEGATIVE CONTROL for both, not "unlimited + // by design": no forced sync, so a one-frame respecify-and-draw loop grows without bound + // and MagmaWireReclaimScenario's watermark cases must go red. Uint32 WireDeferredMb = 64; // MOBILEGL_IPC_SPIN_US: spin before parking on a doorbell, either direction. Uint32 SpinUs = 50; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp index 54365f0f..4328b94a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp @@ -612,7 +612,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { void VkBufferManager::EnforceWireDeferredWatermark() { const Uint64 budget = static_cast(MG_Config::Ipc.WireDeferredMb) * 1024u * 1024u; - if (budget == 0 || m_deferredWireBytes <= budget) return; + // TWO TRIGGERS, ONE SWITCH. Bytes are what the knob names, but a VkBuffer costs per + // OBJECT as well as per byte, and small orphans never reach a byte budget: measured on + // bsl-esc-menu-854 (spawn, lavapipe), one stretch with no submission parked 12,498 stores + // in 39.5 MB against 64 MiB. So a fixed count ceiling rides the same sync point; the + // knob's 0 turns both off, which keeps it one negative control. + if (budget == 0) return; + if (m_deferredWireBytes <= budget && m_deferredWireReleases.size() <= kWireDeferredCountCeiling) return; // What is left after the sweep is named by work that is recorded but unsubmitted, or // submitted but unretired. Every entry is tagged at or below the sync point taken HERE, // so waiting it out - the host-access wait's own call, which flushes the recording first diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h index 4e5a5155..5f481cc4 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h @@ -304,6 +304,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { // takes - flush what is recorded, wait for it - and sweep again, which retires every // parked store, because none can be tagged past the sync point it just waited out. void EnforceWireDeferredWatermark(); + // ...and the same sync point when more than this many stores are parked, whatever their + // bytes (see the definition for the measurement). Not a knob: it bounds an object count + // the byte budget cannot see, and MOBILEGL_IPC_WIRE_DEFERRED_MB=0 disables it too. + static constexpr SizeT kWireDeferredCountCeiling = 1024; // Teardown: the caller has proven the device idle (Shutdown / RecreateTransientArenas). void DestroyAllDeferredWireReleases(); // THE one Fatal{ResourceUnavailable, "buffer-write-sync"} site (rule I: no second abort diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 00857180..cefd5214 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -3444,7 +3444,7 @@ endif() # ===================================================================================== # # ID-P7-27 / ID-P7-32: bsl-esc-menu-854 delivers one present per replay, and the split server -# held every orphaned VkBuffer until a frame boundary that never came. The two cases read the +# held every orphaned VkBuffer until a frame boundary that never came. The three cases read the # server's own wbuf[] gauges (PipeStats.h) off its `MGPipe stats:` line, so the flavour pins the # stats channel at one line per swap, and MOBILEGL_IPC_WIRE_DEFERRED_MB=8 keeps the watermark # case at 48 MiB of traffic instead of 400. @@ -3471,7 +3471,8 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ${MGL_ITEST_MAGMA_RECLAIM_FLAVOUR_ENV} ${mglItestArmTransportEnv}) foreach(reclaimCase RespecifiesWithNoDrawBetweenKeepTheLiveStoreCountBounded - RespecifyAndDrawEachStoreInOneFrameStaysWithinTheDeferredBudget) + RespecifyAndDrawEachStoreInOneFrameStaysWithinTheDeferredBudget + ManySmallRespecifyAndDrawRoundsStayUnderTheStoreCountCeiling) gtest_discover_tests(MobileGLIntegrationTest TEST_PREFIX "DirectVulkan.${mglItestArm}.Reclaim." TEST_FILTER "MagmaWireReclaimScenario.${reclaimCase}" diff --git a/MobileGL/MG_IntegrationTest/Scenarios/MagmaWireReclaimScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/MagmaWireReclaimScenario.cpp index 6256d0ce..95d61f6a 100755 --- a/MobileGL/MG_IntegrationTest/Scenarios/MagmaWireReclaimScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/MagmaWireReclaimScenario.cpp @@ -17,7 +17,7 @@ // split server held 25,923 dead stores against 28 live wire buffers and 41k mappings; on the // Redmi it died in scudo at 1.2 GiB while the monolith passed at 807 MiB. // -// WHAT THE TWO CASES PIN, both inside ONE frame (no swap between the first and the last +// WHAT THE THREE CASES PIN, all inside ONE frame (no swap between the first and the last // respecify), because that is the only shape in which the bug existed: // * RespecifiesWithNoDrawBetween... - the stores no GPU command ever named. They are dead the // moment they are orphaned, so the live store count must stay a small multiple of the wire @@ -27,6 +27,8 @@ // MOBILEGL_IPC_WIRE_DEFERRED_MB watermark's forced sync point. The parked bytes must stay // within the budget plus one store, and every draw must still land on its own strip - a // store destroyed while its draw was pending shows up as a wrong strip (or a dead lavapipe). +// * ManySmallRespecifyAndDrawRounds... - the same with stores too small to reach any byte +// budget; the fixed 1024-store ceiling on the same sync point is what bounds them. // // READ FROM THE SERVER, NOT THE PICTURE. The numbers are the wbuf[] gauges the server role's // VkBufferManager publishes (PipeStats.h, Gauge::WireBuffers..WireDeferredSyncs): RUN MAXIMA, @@ -77,6 +79,13 @@ namespace MGITest { constexpr int kDrawnRespecifies = 48; constexpr std::size_t kDrawnStoreBytes = static_cast(kMiB); + // Case 3: many SMALL respecify-and-draw rounds - 3000 x 256 B is 750 KB, nowhere near + // the byte budget, so only the store-count ceiling (VkBufferManager:: + // kWireDeferredCountCeiling, 1024) can bound them inside the frame. + constexpr int kSmallDrawnRespecifies = 3000; + constexpr std::size_t kSmallDrawnStoreBytes = 256; + constexpr long long kDeferredCountCeiling = 1024; + constexpr const char* kVertex = R"(#version 330 core layout(location = 0) in vec2 aPos; void main() { gl_Position = vec4(aPos, 0.0, 1.0); })"; @@ -308,5 +317,47 @@ void main() { fragColor = uColor; })"; << ". " << window.line; } + // -------------------------------------------------------------------------------------- + // CASE 3. 3000 respecify-and-draw rounds of a 256-byte store in one frame. The byte + // budget never trips (750 KB against 8 MiB), which is the bsl-esc-menu-854 shape the + // count ceiling exists for: one stretch there parked 12,498 stores in 39.5 MB. Before M2 + // every one of the 3000 was alive at the end of the loop. + TEST_F(MagmaWireReclaimScenario, ManySmallRespecifyAndDrawRoundsStayUnderTheStoreCountCeiling) { + if (!Ready() || IsSkipped()) return; + const PipeStatsWindow::LogMark mark = SetupFrame(); + + ClearTo(0.0f, 0.0f, 0.0f, 0.0f); + std::vector bytes(kSmallDrawnStoreBytes, 0); + for (int i = 0; i < kSmallDrawnRespecifies; ++i) { + Respecify(bytes, 0, kWidth); + DrawStrip(i % 200); + } + const Image image = ReadPixels(kWidth, kHeight); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + const Rgba8 want = StripColor((kSmallDrawnRespecifies - 1) % 200); + EXPECT_TRUE(Near(image.At(kWidth / 2, kHeight / 2), want)) + << "read " << image.At(kWidth / 2, kHeight / 2) << ", wanted " << want + << ": the last of " << kSmallDrawnRespecifies << " draws did not land on top"; + + const PipeStatsWindow::Window window = CloseFrameAndReadServerWindow(mark); + ASSERT_TRUE(window.found) << "no 'MGPipe stats:' window in the server log " + << PipeStatsWindow::ServerLibraryLogPath(); + const long long wireBuffers = PipeStatsWindow::CounterOrAbsent(window, "wbufs"); + const long long livePeak = PipeStatsWindow::CounterOrAbsent(window, "wlivepk"); + const long long syncs = PipeStatsWindow::CounterOrAbsent(window, "wdefsync"); + ASSERT_GT(wireBuffers, 0) << "the server published no wbufs= gauge: " << window.line; + RecordProperty("wire_buffers", static_cast(wireBuffers)); + RecordProperty("wire_stores_peak", static_cast(livePeak)); + RecordProperty("wire_deferred_syncs", static_cast(syncs)); + // The ceiling is checked after the park that crosses it, so one store past it plus the + // records is the most the arm can hold. + const long long liveBound = wireBuffers + kDeferredCountCeiling + 2; + EXPECT_LE(livePeak, liveBound) + << "the Magma server held " << livePeak << " VkBuffers at once after " + << kSmallDrawnRespecifies << " small respecify-and-draw rounds in one frame; the " + "records plus the " << kDeferredCountCeiling << "-store ceiling allow " << liveBound + << ". A count that tracks the rounds is the bsl-esc-menu-854 shape. " << window.line; + } + } // namespace } // namespace MGITest