[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.
This commit is contained in:
2026-09-22 18:44:07 -04:00
parent a2e244c036
commit 511c1f1641
2 changed files with 98 additions and 0 deletions
@@ -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<unsigned long long>(WireDeclineTally::UnsoundSerialCompleteEvents()),
static_cast<unsigned long long>(advanceTo),
static_cast<unsigned long long>(remaining.submitIndex),
static_cast<unsigned long long>(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<unsigned long long>(submitIndex),
static_cast<unsigned long long>(advanceTo), m_inFlightSubmits.size(),
m_inFlightSubmits.empty() ? -1LL : static_cast<long long>(minRemaining),
static_cast<unsigned long long>(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);
@@ -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<unsigned long long>(UnsoundSerialCompleteEvents()));
if (TotalDeclines() == 0 && PeakPendingUploadBytes() == 0) return;
MGLOG_I("MGWIRE-DECLINES[%s] total=%llu pendingEntries=%llu pendingBytes=%llu peakPendingBytes=%llu", tag,
static_cast<unsigned long long>(TotalDeclines()),
@@ -125,6 +142,10 @@ private:
static std::atomic<Uint64> value{0};
return value;
}
static std::atomic<Uint64>& UnsoundSerial() {
static std::atomic<Uint64> value{0};
return value;
}
};
// Every decline goes through this: the tally moves ALWAYS, the log line is once per site.