From 511c1f164148cc7dc21af69eeb2a46bc2b56ad78 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 22 Sep 2026 18:42:35 -0400 Subject: [PATCH] [MG_Backend] (P7): the completed-frame-serial floor only advances to a serial nothing in flight still carries THE OpenRA DEVICE DIVERGENCE. "Frame-serial completion piggybacks on submission completion" holds only while a frame serial has ONE submission. On the wire arm it has two, measured on this trace: 73 submissions over 32 serials on inproc and on spawn, 30 over 32 on monolith. S1 is the per-frame palette glTexImage2D, which reaches UploadPendingWireLevels -> FlushWirePendingCommandsForTextureUpdate -> FlushPendingCommands at the first palette-sampling draw and submits under a POOLED fence. S2 is Present, and it carries the frame's draws AND the barriered vkCmdCopyBuffer that StagedWireRangeCopy records for every streamed glBufferSubData. RegisterSubmit stamps both with the serial current at submit time, so they share it. S1 is tiny and signals almost immediately. RefreshCompletedSubmits retires it alone, and OnSubmitsCompletedUpTo called NotifyFrameSerialComplete per retired record - advancing the floor to N-1 while S2(N-1), the submission actually carrying frame N-1's buffer copies, was still executing. GetCompletedSerial then returned N-1, so frame N's first write to that buffer failed WriteWireBuffer's busy predicate and took the unordered host memcpy at :268 - and S2(N-1)'s queued copies landed on top of it afterwards. That is why the wrong pictures are a PREFIX TEAR of one draw's vertex range rather than a missing draw: 318 vertices / 6 = 53 quads at 11448/53 = 216 bytes each, and 6696 = exactly 31 of them. 6696 and 11448 are both real write sizes of that same buffer (26 and 5 occurrences in this replay), so the damage is the previous frame's write list replayed over the new bytes, and its length is the largest write in whatever suffix of that list had not executed yet. It also explains the device's FRAMESINFLIGHT=4/8 result (6/6 whole-write loss): more frames in flight puts the GPU further behind, so the pending suffix contains the larger writes. The floor now advances only to a serial NO remaining in-flight submission still carries. NotifyFrameSerialComplete is monotone and already refuses the still-recording serial, so this can only advance the floor later, never further; the disaggregated build's monolith arm is behaviour-identical (one submission per serial, and its counter reads zero either way). The #else branch is the pull build's statement verbatim, which is what keeps G1 byte-identical. The counter is the red-once's reading, per lens C: an advance naming a serial some remaining submission still carries. Base 26 on inproc, 26 on spawn, 0 on monolith; zero on all three after this commit. It is logged per event rather than dumped at teardown because this trace's renderer is never Shutdown() - the process exits - so a teardown dump reports the start-up context's zero. --- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 77 +++++++++++++++++++ .../DirectVulkan/Renderer/WireDeclineTally.h | 21 +++++ 2 files changed, 98 insertions(+) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index ea2f2689..b2003509 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -13608,13 +13608,46 @@ void main() { void VulkanRenderer::OnSubmitsCompletedUpTo(Uint64 submitIndex) { m_completedSubmitCounter = std::max(m_completedSubmitCounter, submitIndex); +#if MOBILEGL_BUILD_DISAGGREGATED + // ---- P7 wave 2 package B3: THE COMPLETED-FRAME-SERIAL FLOOR MUST BE PROVABLE -------- + // + // "Frame-serial completion piggybacks on submission completion" holds only while a + // frame serial has ONE submission. On the wire arm it has at least two: the per-frame + // palette glTexImage2D reaches UploadPendingWireLevels -> + // FlushWirePendingCommandsForTextureUpdate -> FlushPendingCommands at the first + // palette-sampling draw and submits S1 under a POOLED fence, and the frame's draws + // (and the barriered vkCmdCopyBuffer that StagedWireRangeCopy records for every + // streamed glBufferSubData) then go out in the Present submission S2. + // + // S1 is tiny and signals almost immediately. Retiring it used to advance the floor to + // ITS frameSerial - which is also S2's - so VkBufferManager was told "frame N-1 is + // complete" while S2(N-1), the submission actually carrying frame N-1's buffer copies, + // was still executing. The next frame's first write to that buffer then failed + // WriteWireBuffer's busy predicate and took the unordered host memcpy, and S2(N-1)'s + // queued copies landed on top of it afterwards: a PREFIX TEAR of one draw's vertex + // range, no log line, no GL error, no Fatal. + // + // The floor now advances only to a serial NO remaining in-flight submission still + // carries. NotifyFrameSerialComplete is monotone and already refuses the still- + // recording serial, so this can only ever advance the floor later, never further. + // On the disaggregated build's MONOLITH arm this is behaviour-identical (one + // submission per serial). The #else branch below is the pull build's statement, + // unchanged, which is what keeps G1's .text byte-identical. + Bool retiredAny = false; + Uint64 advanceTo = 0; +#endif while (!m_inFlightSubmits.empty() && m_inFlightSubmits.front().submitIndex <= submitIndex) { SubmitRecord record = m_inFlightSubmits.front(); m_inFlightSubmits.erase(m_inFlightSubmits.begin()); +#if MOBILEGL_BUILD_DISAGGREGATED + retiredAny = true; + advanceTo = std::max(advanceTo, record.frameSerial); +#else // Frame-serial completion piggybacks on submission completion. // NotifyFrameSerialComplete refuses the current (still-recording) // serial, so mid-frame flush records do not mark it early. m_bufferManager.NotifyFrameSerialComplete(record.frameSerial); +#endif if (!record.pooledFence || m_device == VK_NULL_HANDLE) { continue; // frame-slot fences are reset/destroyed by FrameContext } @@ -13624,6 +13657,50 @@ void main() { vkDestroyFence(m_device, record.fence, nullptr); } } +#if MOBILEGL_BUILD_DISAGGREGATED + if (retiredAny) { + // Clamp to one below the lowest serial still in flight. + for (const auto& remaining : m_inFlightSubmits) { + advanceTo = std::min(advanceTo, remaining.frameSerial > 0 ? remaining.frameSerial - 1 : Uint64{0}); + } + // Lens C's probe, and the red-once's reading: an advance that names a serial some + // remaining submission still carries. Unreachable with the clamp above; the R-16 + // revert is to delete the clamp loop, and then this counts. + for (const auto& remaining : m_inFlightSubmits) { + if (remaining.frameSerial <= advanceTo) { + WireDeclineTally::CountUnsoundSerialComplete(); + // Logged per event, not once: this is the red-once's reading and the log is + // the only place it can be read from: the renderer this trace builds is + // never Shutdown() (the process exits), so a teardown dump reports the + // start-up context's zero instead of the replay's count. + MGLOG_W("MGWIRE-FLOOR unsound-serial-complete #%llu: advancing the completed " + "frame-serial floor to %llu, which submission %llu (serial %llu) still carries", + static_cast(WireDeclineTally::UnsoundSerialCompleteEvents()), + static_cast(advanceTo), + static_cast(remaining.submitIndex), + static_cast(remaining.frameSerial)); + break; + } + } + { + static const Bool probe = [] { + const char* value = std::getenv("MOBILEGL_MAGMA_WIREBUF_PROBE"); + return value && value[0] == '1'; + }(); + if (probe) { + Uint64 minRemaining = ~Uint64{0}; + for (const auto& r : m_inFlightSubmits) minRemaining = std::min(minRemaining, r.frameSerial); + MGLOG_I("FLOOR retire upTo=%llu advanceTo=%llu remaining=%zu minRemainingSerial=%lld " + "frameSerial=%llu", + static_cast(submitIndex), + static_cast(advanceTo), m_inFlightSubmits.size(), + m_inFlightSubmits.empty() ? -1LL : static_cast(minRemaining), + static_cast(m_bufferManager.GetFrameSerial())); + } + } + m_bufferManager.NotifyFrameSerialComplete(advanceTo); + } +#endif // Mid-frame-flushed command buffers whose submission just completed can // be freed now; present-less flush loops have no other reclaim point. m_frameContext.FreeRetiredCommandBuffersCompletedUpTo(m_completedSubmitCounter); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDeclineTally.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDeclineTally.h index 11098717..0c90010b 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDeclineTally.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDeclineTally.h @@ -74,6 +74,18 @@ public: while (bytes > peak && !PeakPendingBytes().compare_exchange_weak(peak, bytes, std::memory_order_relaxed)) { } } + // The frame-serial floor's own counter. An "unsound serial complete" is an advance of + // VkBufferManager's completed-frame-serial floor to a serial that ANOTHER submission still + // in flight also carries - i.e. the renderer telling the buffer manager "frame N-1 is done" + // while the submission holding frame N-1's buffer copies is still executing. It is what + // lets a streamed glBufferSubData take the unordered host path and tear one draw's vertex + // range. With a provable floor it is unreachable, so this counter is the red-once's reading: + // non-zero on the wire arms before the fix, zero after, zero on monolith either way. + static void CountUnsoundSerialComplete() { + UnsoundSerial().fetch_add(1, std::memory_order_relaxed); + } + static Uint64 UnsoundSerialCompleteEvents() { return UnsoundSerial().load(std::memory_order_relaxed); } + static Uint64 PendingUploadEntries() { return PendingEntries().load(std::memory_order_relaxed); } static Uint64 PendingUploadBytes() { return PendingBytes().load(std::memory_order_relaxed); } static Uint64 PeakPendingUploadBytes() { return PeakPendingBytes().load(std::memory_order_relaxed); } @@ -81,6 +93,7 @@ public: static void Reset() { for (SizeT i = 0; i < kSiteCount; ++i) Slots()[i].store(0, std::memory_order_relaxed); Total().store(0, std::memory_order_relaxed); + UnsoundSerial().store(0, std::memory_order_relaxed); PendingEntries().store(0, std::memory_order_relaxed); PendingBytes().store(0, std::memory_order_relaxed); PeakPendingBytes().store(0, std::memory_order_relaxed); @@ -90,6 +103,10 @@ public: // from the forced-condition harness; silent when nothing declined, so a green lane stays // quiet and a lane that dropped a draw cannot. static void Dump(const char* tag) { + // The floor counter prints unconditionally: "zero unsound advances" is the reading the + // red-once needs, and an absent line cannot be told from a lane that never ran. + MGLOG_I("MGWIRE-FLOOR[%s] unsoundSerialComplete=%llu", tag, + static_cast(UnsoundSerialCompleteEvents())); if (TotalDeclines() == 0 && PeakPendingUploadBytes() == 0) return; MGLOG_I("MGWIRE-DECLINES[%s] total=%llu pendingEntries=%llu pendingBytes=%llu peakPendingBytes=%llu", tag, static_cast(TotalDeclines()), @@ -125,6 +142,10 @@ private: static std::atomic value{0}; return value; } + static std::atomic& UnsoundSerial() { + static std::atomic value{0}; + return value; + } }; // Every decline goes through this: the tally moves ALWAYS, the log line is once per site.