diff --git a/CMakeLists.txt b/CMakeLists.txt index 4a29344c..c354766f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -571,6 +571,11 @@ if (MOBILEGL_BUILD_DISAGGREGATED) MobileGL/MG_Remote/Client/ClientSession.cpp MobileGL/MG_Remote/Client/EmitTables.cpp MobileGL/MG_Remote/Client/CapsMirror.cpp + # P5 b1's two: the conservative GPU-write set the client must build because all six + # MarkGpuWritten producers are on the server's side of the line, and the + # block-granularity persistent-map push that tier T2 makes mandatory. + MobileGL/MG_Remote/Client/GpuWritePending.cpp + MobileGL/MG_Remote/Client/PersistentMapTracker.cpp MobileGL/MG_Remote/Server/ServerSession.cpp MobileGL/MG_Remote/Server/PipeApplier.cpp MobileGL/MG_Remote/Server/ServerLoop.cpp diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index ca8b9ce1..00ec6b81 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -1043,6 +1043,14 @@ namespace MobileGL::MG_Backend::DirectGLES { // sizeable (page-coverable) range to engage, and below it the driver // falls back to waiting out the WAR hazard on the CPU. constexpr SizeT kInvalidateRangeMinBytes = 128u * 1024u; +#if MOBILEGL_PIPE_PUSH + // The push arm's copy of this threshold lives in Managers.h, where a unit case can + // reach it (InvalidateFlushAccessFor). Two constants, one value, and the compiler + // is what keeps them one: the pull arm's FlushPendingRangesNow is byte-frozen + // against 5cb826b0 (ID-15), so the constant it reads may not move to a header. + static_assert(kInvalidateRangeMinBytes == kEsprytInvalidateRangeMinBytes, + "the two arms of the three-tier ladder must use the same tier-1 threshold"); +#endif // Push every queued range of `resource` from the shadow into the backend // store, without ever letting a driver resolve the WAR hazard against @@ -1078,7 +1086,21 @@ namespace MobileGL::MG_Backend::DirectGLES { // 26.3 p99 depends on, and a second copy of it for the handle arm is exactly how a // tier silently changes. The arms differ only in where `hostBase` and `frontendSize` // come from. - void FlushPendingRangesFrom(GLESBufferResource& resource, const Uint8* hostBase, SizeT frontendSize) { + // + // R-11'S PARAMETER, AND IT IS A PARAMETER RATHER THAN AN ASSUMPTION (P5 b1). + // `hostBase` is re-read at every use today precisely because a shadow resize or an + // adoption moves what an earlier base pointed at. Under split the server may hold + // no pointer into the client's shadow at all, so the base becomes a SNAPSHOT taken + // into SEG_STAGE at emission - and a snapshot covers a RANGE, not the store. The + // two extra arguments say which range `hostBase` is good for; the default is the + // whole store, which is exactly what a live shadow is, so every caller today is + // byte-identical. The moment w1 passes a real snapshot extent, tier 1's widening + // refusal below stops being unreachable and a too-narrow snapshot is named instead + // of silently clobbering GPU-written bytes with stale ones. + constexpr SizeT kHostBaseCoversWholeStore = ~static_cast(0); + void FlushPendingRangesFrom(GLESBufferResource& resource, const Uint8* hostBase, SizeT frontendSize, + SizeT hostBaseFrom = 0, + SizeT hostBaseTo = kHostBaseCoversWholeStore) { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif @@ -1126,12 +1148,28 @@ namespace MobileGL::MG_Backend::DirectGLES { // shadow's to rewrite. Widening to page bounds looked free and was // not - the widened bytes clobbered GPU-written data (an SSBO // counter beside the app's SubData) with the stale shadow. - const Bool wholeBuffer = start == 0 && end == limit && limit == resource.storageSize; - if (mapUsable && (wholeBuffer || size >= kInvalidateRangeMinBytes)) { + // The extent the bytes behind `hostBase` are actually good for, clamped + // to the store. With the default it IS [start, end), so the refusal below + // cannot fire and the ladder is unchanged; with a real SEG_STAGE snapshot + // it is the snapshot's window and a disagreement drops this range to the + // staging ring instead of letting a widened INVALIDATE_RANGE declare bytes + // dead that nothing is about to rewrite. + const SizeT coveredFrom = hostBaseFrom > start ? hostBaseFrom : start; + const SizeT coveredTo = hostBaseTo < end ? hostBaseTo : end; +#if MOBILEGL_PIPE_VERIFY + if (coveredFrom != start || coveredTo != end) { + MGLOG_E_ONCE("MGPipe: Fatal{StageSnapshotTooNarrow} flush_pending_ranges: the " + "staged bytes cover [%zu, %zu) and the queued range is [%zu, %zu) " + "- tiers 2 and 3 would copy from outside the snapshot", + hostBaseFrom, hostBaseTo, start, end); + } +#endif + const GLbitfield access = + mapUsable ? InvalidateFlushAccessFor(start, end, coveredFrom, coveredTo, limit, + resource.storageSize) + : 0u; + if (access != 0) { BindBufferId(TempBufferTarget, resource.id); - const GLbitfield access = - GL_MAP_WRITE_BIT | - (wholeBuffer ? GL_MAP_INVALIDATE_BUFFER_BIT : GL_MAP_INVALIDATE_RANGE_BIT); void* dst = g_GLESFuncs.glMapBufferRange(TempBufferTarget, (GLintptr)start, (GLsizeiptr)size, access); if (dst) { @@ -2667,16 +2705,33 @@ namespace MobileGL::MG_Backend::DirectGLES { } const auto* record = ResourceRecordOf(res); if (record == nullptr) return false; -#if MOBILEGL_PIPE_VERIFY +#if MOBILEGL_PIPE_VERIFY && !MOBILEGL_BUILD_DISAGGREGATED MOBILEGL_ASSERT(!record->HasLiveHostWrites, - "MGPipeResourceRecord::HasLiveHostWrites is set, but P3a has no producer for it"); + "MGPipeResourceRecord::HasLiveHostWrites is set, but this build has no producer " + "for it - P5 b1's producer is MGPSubData::HasLiveHostWrites and is split-only"); #endif if (record->HasLiveHostWrites) return false; - // The same question the legacy arm asks at this exact point in the order, for the - // reason written at the top of this function. A null object is the "no frontend to - // ask" case (nothing reaches this probe without one today) and is treated as "not - // mapped", which is what the record already says. - if (frontend != nullptr && frontend->IsMapped()) return false; + // THE LAST FRONTEND READ IN THIS FUNCTION, AND P5 b1 RETIRES IT - under split + // only, because that is the only build where it is both wrong and replaceable. + // + // It asked the object whether it was mapped because HasLiveHostWrites was pinned + // false and an emulated persistent map mutates the shadow with no call, no serial + // and no epoch; answering from the record alone made such a buffer read + // draw-CLEAN forever, SyncPersistentMappedRange was never reached again, and the + // frame drew the last uploaded bytes with no diagnostic + // (MG_Test/SanityTest.cpp's DirectGLESBufferDrawProbe is exactly that case). + // + // TWO THINGS REPLACE IT AND BOTH HAD TO LAND FIRST: the record now carries the map + // state (the line above, from MGPSubData::HasLiveHostWrites), and the client + // pushes the mapped span by block at every validate point, so the serial moves for + // a write the application made with no API call. Under a spawn there is no object + // on this side to ask, which is why this was never going to stay a choice. + // + // A null object is the "no frontend to ask" case and is treated as "not mapped", + // which is what the record already says. + const Bool askTheObjectWhetherItIsMapped = + MG_Config::Transport == MG_Config::TransportMode::Monolith; + if (askTheObjectWhetherItIsMapped && frontend != nullptr && frontend->IsMapped()) return false; if (resource->pendingRespecify || !resource->storageInitialized) return false; if (!resource->pendingRanges.empty()) return false; if (resource->storageSize != static_cast(record->Desc.Width)) return false; diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 9a59d9a2..38359e4d 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -901,6 +901,44 @@ namespace MobileGL::MG_Backend::DirectGLES { // has to receive the handle in a payload instead. MG_Pipe::MGPipeHandle HandleOfBuffer(const MG_State::GLState::BufferObject* bufferObject); + // TIER 1 OF THE THREE-TIER FLUSH LADDER, AS A PURE FUNCTION (P5 b1). + // + // `GL_MAP_INVALIDATE_RANGE_BIT` is not a hint, it is an ASSERTION THAT THE OLD BYTES + // ARE DEAD - and it is only true of the bytes this call is about to rewrite from the + // authoritative shadow. Managers.cpp:1125-1128 records what happens when it is not: + // widening the map to page bounds "looked free and was not - the widened bytes + // clobbered GPU-written data (an SSBO counter beside the app's SubData) with the stale + // shadow". It fails SILENTLY, unlike tier 3, which only stalls. + // + // WHY IT IS A FUNCTION NOW, AND WHY IT TAKES THE MAP RANGE SEPARATELY FROM THE QUEUED + // ONE. Under split the bytes are not re-read at every use any more: the server may not + // hold a pointer into the client's shadow at all (R-11), so `hostBase` becomes a + // SNAPSHOT taken into SEG_STAGE at emission, and the window between the snapshot and + // the apply is new. A snapshot that covers less than the map does is exactly the + // widening that drew blood, with a thread boundary instead of a page alignment as the + // cause - so the two extents are separate parameters and a disagreement returns 0 + // ("do not take tier 1"), which drops the range onto the staging ring and costs a copy + // rather than a corruption. + // + // Returns the glMapBufferRange access bits, or 0 when tier 1 must not be taken. + inline constexpr SizeT kEsprytInvalidateRangeMinBytes = 128u * 1024u; + constexpr GLbitfield InvalidateFlushAccessFor(SizeT queuedStart, SizeT queuedEnd, SizeT mapStart, + SizeT mapEnd, SizeT limit, SizeT storageSize) { + if (mapEnd <= mapStart) return 0u; + // THE WIDENING REFUSAL. Not >=, not "covers": exactly, in both directions. A map + // narrower than the queued range leaves bytes unwritten inside a range it has just + // declared dead, which is the same corruption read the other way round. + if (mapStart != queuedStart || mapEnd != queuedEnd) return 0u; + const SizeT size = mapEnd - mapStart; + const Bool wholeBuffer = mapStart == 0 && mapEnd == limit && limit == storageSize; + // A partial range below the threshold goes to the ring instead: the map's + // page-substitution fast path needs a page-coverable range to engage, and below it + // the driver falls back to waiting out the WAR hazard on the CPU. + if (!wholeBuffer && size < kEsprytInvalidateRangeMinBytes) return 0u; + return GL_MAP_WRITE_BIT | + (wholeBuffer ? GL_MAP_INVALIDATE_BUFFER_BIT : GL_MAP_INVALIDATE_RANGE_BIT); + } + // The handle arms of the two draw-path entry points below. IsBufferDrawCleanByHandle // asks the applier the same five questions IsBufferDrawClean asks the frontend object, // with identical semantics (D-A4); EnsureBufferResourceForHandle is the ensure path diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 260e1c96..4b289545 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -12,6 +12,9 @@ #include #include #include +#if MOBILEGL_BUILD_DISAGGREGATED +#include +#endif #include "../Getter/GL_Getter.h" namespace MobileGL::MG_Impl::GLImpl { @@ -1315,6 +1318,20 @@ namespace MobileGL::MG_Impl::GLImpl { static_cast(bufferIndex)); const auto& buffer = bindingPoint.GetBoundObject(); if (buffer == nullptr) continue; +#if MOBILEGL_BUILD_DISAGGREGATED + // This fixup READS the captured bytes back through the shadow, so it is the one + // consumer that cannot simply inherit the deferral EndTransformFeedback's dropped + // fence introduces. Under split it pays the reconciliation itself, which is the + // same cost the fence used to charge every caller - here charged only to the + // capture shapes that actually need reordering. + // + // TRANSPORT-GATED LIKE EVERY OTHER NEW SITE (D-J). Without the test this fires in + // a build-split lane running MOBILEGL_TRANSPORT=monolith on Magma - whose + // BeginXfbCaptureForDraw does mark the capture targets - where the fence at the + // caller still runs, so the readback it emits is pure new work on the monolith + // path and integration-gpu cannot see it. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) buffer->SyncGpuWrites(); +#endif const Range1D range = bindingPoint.GetRange(); const Uint8* mapped = buffer->MappedData(); if (mapped == nullptr) continue; @@ -1355,12 +1372,28 @@ namespace MobileGL::MG_Impl::GLImpl { MGP_FILL(EndTransformFeedback); endXfb(); } +#if MOBILEGL_BUILD_DISAGGREGATED + // P5 (b1), the second producer the client-side GPU-write set ADDS, and it has to be + // taken HERE - before GLContext::EndTransformFeedback clears the live bindings, since + // a mark taken after it marks nothing. + MG_Remote::Client::MarkEndTransformFeedbackCaptureTargets(); +#endif MG_State::pGLContext->EndTransformFeedback(); // Captured results must be visible to MapBuffer/GetBufferSubData after // End; the capture targets are host-coherent GPU memory, so completing // the GPU work is all that is required. + // + // P5 (b1): UNDER SPLIT THE UNBOUNDED WAIT GOES AND THE MARK ABOVE REPLACES IT. The + // wait exists for one reason - so that a later MapBuffer sees real captured results - + // and that is precisely what m_gpuWritePending says; SyncGpuWrites then pays for it + // once, on the first read that actually wants the bytes, instead of on every + // glEndTransformFeedback. A ~0ull ClientWaitSync on the GL thread is also the one + // shape a verb barrier cannot make cheap, because it is the driver's wait and not the + // barrier's. auto& backendGL = MG_Backend::gBackendFunctionsTable.GL; - if (backendGL.FenceSync && backendGL.ClientWaitSync) { + const Bool waitForTheCapture = + MG_Config::Transport == MG_Config::TransportMode::Monolith; + if (waitForTheCapture && backendGL.FenceSync && backendGL.ClientWaitSync) { MGP_FILL(FenceSync); if (auto sync = backendGL.FenceSync()) { MGP_FILL(ClientWaitSync); diff --git a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp index 0be9c8c5..9f09eafa 100644 --- a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp +++ b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp @@ -16,6 +16,9 @@ #include #include #include +#if MOBILEGL_BUILD_DISAGGREGATED +#include +#endif #if MOBILEGL_PIPE_PUSH // P4a, ID-19(c). This file is the ONLY place every DSA framebuffer entry point lives, and the // emitter it reaches is this package's own header rather than a declaration in one of the @@ -3093,6 +3096,18 @@ namespace MobileGL::MG_Impl::GLImpl { void ReadPixels_Backend(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, void* pixels) { MGP_FILL(ReadPixels); MG_Backend::gBackendFunctionsTable.GL.ReadPixels(x, y, width, height, format, type, pixels); +#if MOBILEGL_BUILD_DISAGGREGATED + // P5 (b1), one of the two producers the client-side GPU-write set ADDS. A read into a + // bound GL_PIXEL_PACK_BUFFER is a GPU write to that buffer exactly as a shader's store + // is, and marking it is what makes the next glMapBuffer / glGetBufferSubData of the + // PBO reconcile. It is a no-op on the monolith path, where the backend still maps the + // PBO and copies it into the shadow inside the call + // (DirectGLES.cpp:10983-10993) - an unconditional stall on every glReadPixels whether + // or not anything ever reads the shadow. Deferring that to the first read that wants + // it is STRICTLY BETTER, which is the only reason a split build is allowed to differ + // here at all. + MG_Remote::Client::MarkReadPixelsPackBuffer(); +#endif } /* @INSERTION_POINT:FUNCTION_IMPLEMENTATION@ */ diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 88be4a6b..ca41fc34 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -702,6 +702,15 @@ namespace MobileGL::MG_Pipe { // one was emitted, so this cannot be false - but a zeroed record (null // handle, size 0) is not the answer if that pre-pass is ever relaxed. if (!MGPipeBuildSubDataRecord(handle, at, length, record, /*verbatimShadow=*/true)) return; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5 (b1): the live-host-writes bit rides the content record, because + // "someone may be writing these bytes without telling you" is a fact about the + // CONTENT and not about the storage. It is set from the object's PUBLISHED + // value rather than from a live IsMapped() read so that the record and the + // edge that announced it can never disagree. MGPipeBuildSubDataRecord does not + // take the object, which is why it is set here and not in the builder. + record.HasLiveHostWrites = buffer.HasLiveHostWritesForWire() ? 1 : 0; +#endif MGPipeApplyResourceSubData(record, base + at); }); if (!encodable) { @@ -723,6 +732,15 @@ namespace MobileGL::MG_Pipe { // store, or the pattern FillSubData expanded locally, and neither is this // client's untransformed shadow of the level. if (!MGPipeBuildSubDataRecord(handle, at, length, record, /*verbatimShadow=*/false)) return; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5 (b1): THE SECOND CONTENT EMITTER, and it has to speak for the same reason + // the first does. ApplyBufferWrite ASSIGNS the bit - a content record emitted + // while nothing maps the buffer is how the state goes back to false - so a + // resident sub-data that stayed silent would write false over a live write + // map. `glBufferSubData` against a persistently mapped arena is legal and is + // the ordinary Flywheel/Create shape, so that is not a corner. + record.HasLiveHostWrites = buffer.HasLiveHostWritesForWire() ? 1 : 0; +#endif // The application's STAGING store, valid for the duration of the call only. MGPipeApplyBufferSubDataResident(record, base + (at - offset)); }); diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index f21f6841..b97fc229 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -1075,7 +1075,27 @@ namespace MobileGL::MG_Pipe { // Replaces the backend's `uploadData == mipData` pointer comparison: are these // bytes an untransformed level shadow? Uint8 SourceIsVerbatimLevelShadow; - Uint8 Pad0[3]; + // P5 (b1): DOES THIS RESOURCE HAVE A LIVE HOST WRITER RIGHT NOW? One byte out of the + // pad, so MGP_ASSERT_POD(MGPSubData, 72) below does not move. + // + // It is here rather than on MGPResourceDesc, and that is a ruling with a reason. The + // fact is CONTENT-shaped - "someone may be writing these bytes without telling you" - + // and MGPResourceDesc's only carrier is resource_respecify, which for a BUFFER is + // never classified as metadata-only (PipeApply.cpp's RespecifyRedefinesNoStorage + // refuses the buffer target outright, so that ID-18 M4's "no reallocation ack" is true + // by construction). Announcing a map on a descriptor would therefore have cost a + // spurious reallocation per map - precisely the failure the brief warned about - or a + // change to that rule, which is a P4a contract the buffer family should not be + // re-opening for a flag. + // + // WHO SETS IT: MGPipeEmitResourceSubData, from BufferObject::HasLiveHostWritesForWire. + // WHO READS IT: ApplyBufferWrite, into MGPipeResourceRecord::HasLiveHostWrites, which + // is what IsBufferDrawCleanByHandle asks instead of the frontend object's IsMapped(). + // ZERO IS THE ANSWER, NOT THE ABSENCE OF ONE: a record that does not set it says "no + // live writer", which is what every monolith emission means and why the monolith path + // needs no edit at all. + Uint8 HasLiveHostWrites; + Uint8 Pad0[2]; MGPBox UnionBox; Uint32 RegionCount; // MGPSubRegion[] in the variable tail Uint32 Pad1; diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index 19e4b194..15b3e96a 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -32,6 +32,13 @@ #include #endif +#if MOBILEGL_BUILD_DISAGGREGATED +// R-6's tier gate. One spelling, asked at the one place the decline is decided. Outside the +// MOBILEGL_PIPE_VERIFY block above on purpose: the tier is a property of the BUILD, not of the +// comparator, and a split build without the comparator still declines every acquisition. +#include +#endif + #include #include #include @@ -841,12 +848,18 @@ namespace MobileGL::MG_Pipe { return true; } -#if MOBILEGL_PIPE_VERIFY - // D-A4's pin. HasLiveHostWrites is ALWAYS false in this phase and is written by - // nobody: it exists so the phase that pushes persistent-mapped host writes can set it - // with no new record kind. A producer that landed under it would change what - // IsBufferDrawClean answers with no other visible edit, so a verify build refuses to - // let one arrive unannounced. +#if MOBILEGL_PIPE_VERIFY && !MOBILEGL_BUILD_DISAGGREGATED + // D-A4's pin, AND P5 (b1) IS THE PHASE IT WAS WAITING FOR. It said "HasLiveHostWrites + // is always false in this phase and is written by nobody: it exists so the phase that + // pushes persistent-mapped host writes can set it with no new record kind", and a + // verify build refused to let such a producer arrive unannounced. + // + // The producer is announced: MGPSubData::HasLiveHostWrites, set by + // MGPipeEmitResourceSubData from BufferObject::HasLiveHostWritesForWire and read by + // ApplyBufferWrite below. So the pin is LIFTED FOR A SPLIT BUILD ONLY, and left + // standing everywhere else - in a monolith build nothing sets the bit and the wire is + // still the thing that would say so if something started to. That is the whole value + // of the pin and it survives the phase it was written for. void PinNoLiveHostWrites(const MGPipeResourceRecord& record, MGPipeHandle res, const char* call) { if (!record.HasLiveHostWrites) return; MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("PipeLiveHostWrites") @@ -882,6 +895,32 @@ namespace MobileGL::MG_Pipe { void PinWholeResourceRespecifyScope(const MGPResourceDesc&, MGPipeHandle, const char*) {} #endif +#if MOBILEGL_PIPE_VERIFY + // P5 b1's wire, and the one statement about HasLiveHostWrites that IS still always + // true once the field has a producer: the bit is the BUFFER family's. It answers "does + // this resource have a live host writer right now", which only a buffer can have - a + // texture's host writes are the unpack path's and are announced by the upload itself - + // and the two emitters that set it (MGPipeEmitResourceSubData, + // MGPipeEmitBufferSubDataResident) are both buffer-only. A texture sub-data carrying + // it is therefore an encoder that copied a field across payload halves, which is + // precisely the class of defect a shared record type invites. + // + // It replaces the always-false pin in a split build. That pin could not survive the + // producer it existed to announce; this one can, and unlike that one it is compiled in + // EVERY verify build, including build-verify-split - the matrix cell where the field + // can actually be non-zero. + void PinLiveHostWritesNamesABuffer(const MGPSubData& record, const char* call) { + if (record.HasLiveHostWrites == 0) return; + MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("PipeLiveHostWritesTarget") + " %s {slot=%u, gen=%u}: a record whose resource target is not a " + "buffer (target=%u) declares live host writes, and only the buffer " + "family has any", + call, record.Res.Slot, record.Res.Gen, record.Target); + } +#else + void PinLiveHostWritesNamesABuffer(const MGPSubData&, const char*) {} +#endif + // The one gate every content-carrying buffer write goes through. resource_subdata and // buffer_subdata_resident differ only in which backend hook takes the bytes and in the // fact that one of them is allowed to be absent, so a second copy of this arithmetic @@ -911,6 +950,16 @@ namespace MobileGL::MG_Pipe { return false; } PinNoLiveHostWrites(*stored, record.Res, call); +#if MOBILEGL_BUILD_DISAGGREGATED + // P5 (b1): THE PRODUCER. The record's own statement about the resource, applied + // before the serial moves so that a probe re-entered from inside the backend hook + // below already sees it. It is an assignment and not an OR: the bit is a STATE, + // and a content record emitted while nothing maps the buffer is exactly how the + // state goes back to false - which is why the falling edge pushes one block + // (BufferObject::NotePersistentMapStateChanged) rather than relying on the next + // ordinary write to arrive. + stored->HasLiveHostWrites = record.HasLiveHostWrites != 0; +#endif // THE SERIAL MOVES BEFORE THE BACKEND IS TOLD, and that order is load-bearing: // the backend stamps its own synced serial from this record inside the hook, so a @@ -1777,6 +1826,7 @@ namespace MobileGL::MG_Pipe { // accumulate a pending upload onto whatever TEXTURE holds slot N in the texture slot // space. Renderbuffers have no sub-data path at all, so no correct client can produce // one and this is a protocol fault rather than a dropped call. + PinLiveHostWritesNamesABuffer(record, "resource_subdata"); const Uint8 resourceTarget = MGPipeSubDataResourceTargetOf(record.Target); if (resourceTarget == kMGPipeResourceTargetBuffer || resourceTarget == static_cast(MGPipeResourceTarget::Renderbuffer) || @@ -1938,6 +1988,26 @@ namespace MobileGL::MG_Pipe { // the flag exists to announce, and the wire is what refuses to let it arrive unnamed. PinNoLiveHostWrites(*record, handle.Handle, "map_persistent"); +#if MOBILEGL_BUILD_DISAGGREGATED + // R-6: A SPLIT BUILD RUNS AT TIER T2 AND DECLINES EVERY ACQUISITION, ALWAYS. + // + // The mint returns a raw void* that the client stores as the store's base + // (BufferObject.cpp:238 / :603 / :658). Across a process boundary that address is + // meaningless, and under `inproc` it is WORSE than meaningless: it happens to work, + // so a lane that kept adoption alive would be green for a reason spawn cannot + // reproduce, and persistent-map-push - an exit-gate counter - would be structurally + // zero (PipeStats.cpp says so in as many words). Forcing T2 here rather than at the + // three client call sites is what keeps `mpr` identical between the arms: the + // roundtrip is COUNTED above, unconditionally, because a decline costs the same round + // trip as a mint. + // + // The frontend already tolerates a decline in all three places, and has since P3a. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith && + MG_Remote::Client::AdoptTierIsEmulate()) { + return nullptr; + } +#endif + // NO SERIAL MOVES and NO DESCRIPTOR CHANGES: the donation re-mints the backend's own // driver object, which is a server-local event that the backend's own id generation // already catches, and the client's view of the store's extent is untouched by it. diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index e736f06b..bfd409bf 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -219,9 +219,20 @@ namespace MobileGL::MG_Pipe { // sub-data). It is what replaces the frontend change serial the backend used to // mirror, and no MGPipe call may require the client to provide or know one. Uint64 Serial = 0; - // ALWAYS FALSE IN P3a, AND WRITTEN BY NOBODY. It exists so the phase that pushes - // persistent-mapped host writes can set it with zero new record kinds; a verify build - // pins that it is false, so that phase cannot land a silent semantic change under it. + // "DOES THIS RESOURCE HAVE A LIVE HOST WRITER RIGHT NOW?" + // + // False and written by nobody through P3a and P4a; P5 (b1) is the phase it was waiting + // for and gives it its producer, with zero new record kinds exactly as planned: the + // bit rides MGPSubData::HasLiveHostWrites - a byte out of that payload's existing pad - + // and ApplyBufferWrite assigns it here. In a MONOLITH build nothing sets it and the + // MOBILEGL_PIPE_VERIFY wire (PinNoLiveHostWrites) still refuses a producer, so the pin + // survives the phase it was written for instead of being deleted by it. + // + // It is what IsBufferDrawCleanByHandle asks under split INSTEAD of the frontend + // object's IsMapped(), because a spawned server has no frontend object to ask. Getting + // that substitution wrong once already cost a silent regression - an emulated + // persistent map read draw-clean for ever - which MG_Test/SanityTest.cpp's + // DirectGLESBufferDrawProbe pair now pins from both sides. Bool HasLiveHostWrites = false; // ---- P4a. Only a record of kind Texture ever carries these; a buffer's stay at diff --git a/MobileGL/MG_Pipe/PipeFields.def b/MobileGL/MG_Pipe/PipeFields.def index 72264221..486c993b 100644 --- a/MobileGL/MG_Pipe/PipeFields.def +++ b/MobileGL/MG_Pipe/PipeFields.def @@ -173,7 +173,8 @@ F(X) F(Y) F(Z) F(W) F(H) F(D) F(SrcOffset) F(SrcRowStride) F(SrcSliceStride) #define MGP_FIELDS_MGPSubData(F) \ - F(Res) F(Target) F(Level) F(SourceIsVerbatimLevelShadow) F(UnionBox) F(RegionCount) F(Blob) + F(Res) F(Target) F(Level) F(SourceIsVerbatimLevelShadow) F(HasLiveHostWrites) F(UnionBox) \ + F(RegionCount) F(Blob) #define MGP_FIELDS_MGPSubDataComplete(F) \ F(Res) F(Target) F(FirstLevel) F(LevelCount) F(PullSerial) diff --git a/MobileGL/MG_Remote/Client/GpuWritePending.cpp b/MobileGL/MG_Remote/Client/GpuWritePending.cpp new file mode 100644 index 00000000..112c5713 --- /dev/null +++ b/MobileGL/MG_Remote/Client/GpuWritePending.cpp @@ -0,0 +1,200 @@ +// MobileGL - MobileGL/MG_Remote/Client/GpuWritePending.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#include "GpuWritePending.h" + +#include "ClientSession.h" + +#include +#include +#include +#include +#include +#include + +#include + +namespace MobileGL::MG_Remote::Client { + + using MG_State::GLState::BufferObject; + using MobileGL::BufferTarget; + using MG_State::GLState::ImageTextureBinding; + + namespace { + // Per-row tallies. Diagnostics, and the only thing a unit case can assert on: an + // over-approximating set has no observable difference when a row fires too OFTEN, so + // "did this row fire at all, for this buffer" has to be readable directly. + Array(GpuWriteProducer::Count)> g_producerMarks{}; + + // The transform-feedback rows, shared by the draw walk (row 3) and by + // glEndTransformFeedback (row 5). Both mark the SAME set - the capture targets of the + // capture program - and the split exists only so the two can be counted apart. + void MarkTransformFeedbackTargets(GpuWriteProducer producer) { + auto& context = MG_State::pGLContext; + if (!context) return; + if (!context->IsTransformFeedbackActive()) return; + const auto& program = context->GetTransformFeedbackProgram(); + if (program == nullptr) return; + const SizeT declared = program->GetTransformFeedbackBufferCount(); + const SizeT count = + declared < MG_State::GLState::GLContext::MAX_TRANSFORM_FEEDBACK_BUFFERS + ? declared + : static_cast(MG_State::GLState::GLContext::MAX_TRANSFORM_FEEDBACK_BUFFERS); + for (SizeT i = 0; i < count; ++i) { + const auto& point = + context->GetBufferBindingPoint(BufferTarget::TransformFeedback, static_cast(i)); + MarkBufferForProducer(point.GetBoundObject(), producer); + } + } + + void MarkShaderStorageBindings() { + auto& context = MG_State::pGLContext; + if (!context) return; + // The TOUCHED count, exactly as the backend twin uses it + // (DirectGLES.cpp:559-560): the binding-point array is 84 entries wide and + // walking all of them on every draw is what the high-water mark exists to avoid. + const SizeT points = context->GetTouchedBufferBindingPointCount(BufferTarget::ShaderStorage); + for (SizeT i = 0; i < points; ++i) { + const auto& point = context->GetBufferBindingPoint(BufferTarget::ShaderStorage, static_cast(i)); + MarkBufferForProducer(point.GetBoundObject(), GpuWriteProducer::ShaderStorageBinding); + } + } + + void MarkAtomicCounterBindings() { + auto& context = MG_State::pGLContext; + if (!context) return; + // WIDER THAN ITS BACKEND TWIN, ON PURPOSE AND IN THE SAFE DIRECTION. + // SyncAtomicCounterBuffers (DirectGLES.cpp:578-583) walks the GL bindings the + // TRANSPILED program declared, which is a subset of what is bound; the client has + // the bindings but not that per-program list at this point, so it marks every + // touched atomic-counter point. Over-approximating costs a readback the narrowing + // channel then removes. Under-approximating reads a stale counter and says + // nothing, which is the failure every atomic-counter conformance case is. + const SizeT points = context->GetTouchedBufferBindingPointCount(BufferTarget::AtomicCounter); + for (SizeT i = 0; i < points; ++i) { + const auto& point = context->GetBufferBindingPoint(BufferTarget::AtomicCounter, static_cast(i)); + MarkBufferForProducer(point.GetBoundObject(), GpuWriteProducer::AtomicCounterBinding); + } + } + + void MarkWritableImageBufferTextures() { + auto& context = MG_State::pGLContext; + if (!context) return; + // The backend keeps a bitset of writable image-buffer units + // (DirectGLES.cpp:2350-2352, maintained from its own SyncImageTextureBinding) and + // the client has no equivalent, so it sweeps. The sweep is bounded by the array, + // not by a device limit read: MaxImageUnits would be a backend read, and a stale + // or absent backend object would silently shorten the walk - which is the one + // direction this set may not fail in. The loop body is a null test on a + // contiguous array until a unit is actually bound. P5 records cost and does not + // gate on it (2026-09-08 rule); an image-unit high-water mark on GLContext is the + // obvious narrowing and belongs with P8's binding-walk migration. + for (Int unit = 0; unit < MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS; ++unit) { + const auto& binding = context->GetImageTextureBinding(unit); + if (!ImageUnitIsAWritableBufferTexture(binding)) continue; + auto* textureBuffer = static_cast(binding.Texture.get()); + MarkBufferForProducer(textureBuffer->GetBufferBindingSlot().GetBoundObject(), + GpuWriteProducer::WritableImageBufferTexture); + } + } + } // namespace + + Bool GpuWriteSetIsClientSide() { + return MG_Config::Transport != MG_Config::TransportMode::Monolith; + } + + Bool ImageUnitIsAWritableBufferTexture(const ImageTextureBinding& binding) { + // Verbatim from IsWritableImageBufferTexture (DirectGLES.cpp:2354-2357). All three + // terms are client state; none of them is a driver question. + return binding.Texture != nullptr && binding.Access != GL_READ_ONLY && + binding.Texture->GetStorageType() == TextureStorageType::Buffer; + } + + void MarkBufferForProducer(const SharedPtr& buffer, GpuWriteProducer producer) { + if (!GpuWriteSetIsClientSide()) return; + if (buffer == nullptr) return; + if (producer >= GpuWriteProducer::Count) return; + buffer->MarkGpuWritten(); + ++g_producerMarks[static_cast(producer)]; + } + + void MarkGpuWritesForDraw() { + if (!GpuWriteSetIsClientSide()) return; + MarkShaderStorageBindings(); + MarkAtomicCounterBindings(); + MarkWritableImageBufferTextures(); + MarkTransformFeedbackTargets(GpuWriteProducer::TransformFeedbackCapture); + } + + void MarkGpuWritesForDispatch() { + if (!GpuWriteSetIsClientSide()) return; + MarkShaderStorageBindings(); + MarkAtomicCounterBindings(); + MarkWritableImageBufferTextures(); + } + + void MarkReadPixelsPackBuffer() { + if (!GpuWriteSetIsClientSide()) return; + auto& context = MG_State::pGLContext; + if (!context) return; + MarkBufferForProducer(context->GetBufferBindingSlot(BufferTarget::PixelPack).GetBoundObject(), + GpuWriteProducer::ReadPixelsPackBuffer); + } + + void MarkEndTransformFeedbackCaptureTargets() { + if (!GpuWriteSetIsClientSide()) return; + MarkTransformFeedbackTargets(GpuWriteProducer::EndTransformFeedbackCapture); + } + + Uint64 ProducerMarkCount(GpuWriteProducer producer) { + if (producer >= GpuWriteProducer::Count) return 0; + return g_producerMarks[static_cast(producer)]; + } + + void ResetProducerMarkCountsForTest() { + g_producerMarks.fill(0); + } + + Bool BufferWritebackIsReachable(const BufferObject& buffer) { + if (buffer.GetSize() == 0) return false; +#if MOBILEGL_PIPE_PUSH + return MG_Pipe::MGPipeResourceSubsystemEnabled(); +#else + return false; +#endif + } + + void AwaitBufferWriteback(BufferObject& buffer) { + // THE WAIT IS THE BARRIER'S WAIT (R-3). The reply-slot id IS the record's seq, so + // "appliedSeq reached my readback" and "my answer is back" are one condition, and + // ClientSession::EmitAndWait is what pays for it. With no session - a build-split lane + // running monolith, and every unit case - the emission WAS the application, + // synchronously, so the writeback has already landed and there is nothing to wait for. + // Spelling that as "return" rather than as a loop is deliberate: a loop here would be + // a hang in exactly that configuration, which is the configuration every gate lane + // runs. + if (ClientSession::Active() == nullptr) return; + + // AND THE OTHER ARM IS A NAMED FATAL, NOT AN EMPTY BODY. A session exists, so the + // apply side is no longer synchronous, and if the flag is still set the shadow this + // caller is about to read is STALE - which is the whole failure the third state was + // introduced to stop. An empty body here would make that failure silent and would let + // s1/c1 land a session without noticing that nobody ever wrote the wait; a stub that + // aborts by name is the house shape for exactly this (EmitTables.cpp's + // UnmigratedVerbFatal), and it is what gives the hole a red spelling before the + // transport arrives. + if (!buffer.HasOutstandingGpuWrite()) return; + MGLOG_F("MGPipe: Fatal{UnimplementedWritebackWait} - a ClientSession is active and buffer %u " + "still has an outstanding GPU write after its readback was emitted. The wait is " + "ClientSession::EmitAndWait's (R-3: the reply slot id IS the record seq); P5 package " + "b1 landed the third state and s1/c1 own the wait itself.", + buffer.GetExternalIndex()); + std::abort(); + } + +} // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_Remote/Client/GpuWritePending.h b/MobileGL/MG_Remote/Client/GpuWritePending.h new file mode 100644 index 00000000..873bcf18 --- /dev/null +++ b/MobileGL/MG_Remote/Client/GpuWritePending.h @@ -0,0 +1,152 @@ +// MobileGL - MobileGL/MG_Remote/Client/GpuWritePending.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// THE CLIENT-SIDE CONSERVATIVE GPU-WRITE SET (ARCHITECTURE.md:575 names this component; +// CONTRACT-P5.md section 3, table 2's first set). Owner: package b1. +// +// WHY IT HAS TO MOVE SIDES. `BufferObject::SyncGpuWrites` (BufferObject.cpp:369) runs +// synchronously, on the application's thread, the moment the application calls glMapBuffer or +// glGetBufferSubData. It has to answer "did the GPU write this buffer since I last read it?" +// - and today all six producers of that answer are BACKEND-side (DirectGLES.cpp:570, :618, +// :2603, UniformManager.cpp:1075, :1231, VulkanRenderer.cpp:11618), i.e. on the server's side +// of a split. Asking the server is a round trip the design forbids in steady state, so the +// client must build the set itself, from state it already owns: the SSBO and atomic-counter +// binding points, the image units whose Access is not GL_READ_ONLY, and the transform-feedback +// capture targets. +// +// CONSERVATIVE MEANS OVER-APPROXIMATE, AND THAT IS THE WHOLE DESIGN. The reverse channel's +// OnGpuWritten is a NARROWING channel (ResourceTracker.h:577-581): the client marks +// everything a shader COULD have written and the server only ever removes entries. So a row +// here that fires too often costs a readback; a row that fires too rarely reads a stale shadow +// and is silent. Every row below therefore mirrors its backend twin exactly, including the two +// places the twin is deliberately NARROW - a GL_READ_ONLY image binding is left alone, and the +// image walk runs from draw preparation rather than from glBindImageTexture's eager sync. +// +// NO NARROWING IN P5. ResourceTracker.h:587-592's `rangeCount == 1` assertion STAYS. Zero +// ranges will mean "a fully narrowed set - nothing is dirty" at P8/P9, and a package that +// reads ResourceTracker.h:577-581 alone will think it owns that already. It does not. +// +// THE TWO NEW PRODUCERS. Rows 4 and 5 have no backend twin: they are behaviour P5 ADDS +// (ARCHITECTURE.md:508), and both are strictly better than what monolith does. +// * glReadPixels into a pack PBO becomes fire-and-forget plus a client-side mark. Monolith +// maps the PBO and copies it into the shadow inside the call (DirectGLES.cpp:10983-10993), +// which is an unconditional stall on every glReadPixels whether or not anyone reads the +// shadow; marking instead defers the cost to the first read that actually wants it. +// * glEndTransformFeedback drops its unbounded ClientWaitSync (GL_Drawing.cpp:1367, timeout +// ~0ull) and marks the capture targets, for the same reason: the wait exists only so that +// a later MapBuffer sees real results, which is precisely what the flag is for. +// +// WHAT NO ROW COVERS, WRITTEN DOWN SO THE NEXT READER DOES NOT HAVE TO ASK. The set is built +// from the APPLICATION's bindings, so it says nothing about a backend's own scratch buffers - +// Espryt's converted-vertex-stream and primitive-restart substitution buffers, Magma's UBO +// ring. None of those has a MarkGpuWritten today either, so the client set is no NARROWER than +// monolith's and this is not a regression; it is a standing hole in both, and it stays one +// until the phase that migrates the backend's own allocations. +// +// GATED ON THE TRANSPORT, like everything else in this package: on the monolith path the six +// backend sites still run and a second marker would be new behaviour (D-J), and rows 4 and 5 +// would remove a stall monolith is entitled to keep. + +#pragma once +#include + +#include + +namespace MobileGL::MG_State::GLState { + class BufferObject; + struct ImageTextureBinding; +} // namespace MobileGL::MG_State::GLState + +namespace MobileGL::MG_Remote::Client { + + // ONE ROW PER PRODUCER, and the enum is the inventory: six that mirror a backend + // MarkGpuWritten site one-for-one, two that P5 adds. Each has exactly one unit case, and + // the per-row counters below are what those cases assert on - a row that stops firing is + // otherwise invisible, because an over-approximating set fails SILENTLY in the direction + // that matters. + enum class GpuWriteProducer : Uint8 { + // DirectGLES.cpp:570 (MarkShaderStorageBuffersGpuWritten) and + // UniformManager.cpp:1231 (ResolveStorageBufferDescriptor): every SSBO binding point, + // unconditional, once the points are bound and the draw or dispatch is going out. + ShaderStorageBinding = 0, + // DirectGLES.cpp:618 (SyncAtomicCounterBuffers): every bound atomic counter. The + // point of a counter is that the shader increments it and every conformance case + // reads the increment back with glMapBufferRange or glGetBufferSubData. + AtomicCounterBinding, + // DirectGLES.cpp:2603 (MarkWritableImageBufferTexturesGpuWritten) and + // UniformManager.cpp:1075 (ResolveStorageTexelBufferDescriptor): a buffer texture on + // an image unit, ONLY when Access != GL_READ_ONLY. Marking a read-only binding would + // make the next map wait on - and then re-read - a dispatch that could not have + // changed a byte of it. + WritableImageBufferTexture, + // VulkanRenderer.cpp:11618 (BeginXfbCaptureForDraw): the capture targets, because + // "the capture is a GPU write like any shader's". + TransformFeedbackCapture, + // P5, new: glReadPixels into a bound GL_PIXEL_PACK_BUFFER. + ReadPixelsPackBuffer, + // P5, new: glEndTransformFeedback, in place of the unbounded fence wait. + EndTransformFeedbackCapture, + Count + }; + + // Transport != Monolith. False means every entry point below is a no-op and the six + // backend sites are still the only producers, which is exactly today's behaviour. + Bool GpuWriteSetIsClientSide(); + + // ---- the walks ------------------------------------------------------------------- + // + // Called from the client's own draw / dispatch emission point, BEFORE the verb record + // goes out, for the same ordering reason the persistent-map push has: the set must + // describe the work the record is about to start. + + // Rows 0, 1, 2 and 3. + void MarkGpuWritesForDraw(); + // Rows 0, 1 and 2. A dispatch has no transform feedback. + void MarkGpuWritesForDispatch(); + + // ---- the two new producers ------------------------------------------------------- + + // Row 4. Marks whatever is bound to GL_PIXEL_PACK_BUFFER, or nothing when the read goes + // to client memory - which is the case the backend's map-and-copy never had to consider, + // because it only ran when a PBO was bound in the first place. + void MarkReadPixelsPackBuffer(); + // Row 5. Must be called while the capture state is still ACTIVE: GLContext's + // EndTransformFeedback clears the live bindings, so a mark taken after it marks nothing. + void MarkEndTransformFeedbackCaptureTargets(); + + // ---- the row predicates, exposed so a unit case can drive one row at a time ------- + + // Row 2's discriminator, verbatim from DirectGLES.cpp:2354-2357. It is a function rather + // than three inline conditions at the call site because it is the one row whose backend + // twin is narrow on purpose, and a client that re-derived it slightly wider would mark + // read-only image bindings with nothing able to see that it had. + Bool ImageUnitIsAWritableBufferTexture(const MG_State::GLState::ImageTextureBinding& binding); + + // The one place a row actually marks. Null and duplicate marks are absorbed here so the + // walks stay readable, and the per-row counter moves only when a buffer really was + // marked. + void MarkBufferForProducer(const SharedPtr& buffer, + GpuWriteProducer producer); + + Uint64 ProducerMarkCount(GpuWriteProducer producer); + void ResetProducerMarkCountsForTest(); + + // ---- SyncGpuWrites' third state (CONTRACT-P5.md section 3) ----------------------- + + // Blocks until this buffer's OnBufferWriteback has landed. With no session - a build-split + // lane running monolith, and every unit case - the emission was synchronous and the answer + // is already in, so this returns at once; that is why it is a call rather than a loop the + // caller writes, because the loop would be a hang in exactly that configuration. + void AwaitBufferWriteback(MG_State::GLState::BufferObject& buffer); + + // Is there a readback route at all? A buffer with no size, or one whose backend registered + // no resource ops, can never catch up, and SyncGpuWrites must clear rather than block for + // ever. It is the ONE case monolith's unconditional clear covers that a writeback cannot. + Bool BufferWritebackIsReachable(const MG_State::GLState::BufferObject& buffer); + +} // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_Remote/Client/PersistentMapTracker.cpp b/MobileGL/MG_Remote/Client/PersistentMapTracker.cpp new file mode 100644 index 00000000..d3cda550 --- /dev/null +++ b/MobileGL/MG_Remote/Client/PersistentMapTracker.cpp @@ -0,0 +1,145 @@ +// MobileGL - MobileGL/MG_Remote/Client/PersistentMapTracker.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#include "PersistentMapTracker.h" + +#include +#include + +#include + +namespace MobileGL::MG_Remote::Client { + + using MG_State::GLState::BufferObject; + using MobileGL::BufferMappingAccessBit; + + PersistentMapTracker& PersistentMapTracker::Instance() { + // Leaked on purpose, once, like every other role-local singleton (ID-8): a buffer's + // destructor runs from exit handlers after this TU's globals would already be gone, + // and it calls Forget(). + static PersistentMapTracker* instance = new PersistentMapTracker{}; + return *instance; + } + + Uint64 PersistentMapTracker::BlockBytes() { + return static_cast(MG_Config::Ipc.PersistentBlockKb) * 1024ull; + } + + Bool PersistentMapTracker::PushIsArmed() { + return MG_Config::Transport != MG_Config::TransportMode::Monolith; + } + + // SyncPersistentMappedRange's early-out chain (BufferObject.cpp:341-353), in its order, + // read as a membership test. Every line here has a line there; if one of them moves, the + // unit case that drives both against each other is what says so. + Bool PersistentMapTracker::IsLivePersistentMap(const BufferObject& buffer) { + if (!buffer.IsMapped()) return false; + // GPU-resident: the application already wrote into coherent GPU memory and there is + // nothing to ship. At tier T2 this arm is unreachable - MapPersistent declines - but + // the predicate must still read the chain, not the tier: a build that reaches T0/T1 + // later must see this row answer for itself. + if (buffer.IsBackendPersistentMapped()) return false; + const auto access = buffer.GetMappingAccess(); + if (!(access & BufferMappingAccessBit::Persistent)) return false; + if (!(access & BufferMappingAccessBit::Write)) return false; + // FLUSH_EXPLICIT: the application promises to announce its own writes with + // glFlushMappedBufferRange, which already crosses as resource_flush_range. Pushing + // here as well would ship the same bytes twice and take the upload-shape decision + // away from the side that pays for it. + if (access & BufferMappingAccessBit::FlushExplicit) return false; + const auto range = buffer.GetMappedRange(); + if (range.start >= range.end) return false; + return true; + } + + void PersistentMapTracker::NoteMapStateChanged(BufferObject& buffer) { + const Uint64 key = buffer.GetLifetimeId(); + if (IsLivePersistentMap(buffer)) { + m_livePersistentMaps[key] = &buffer; + return; + } + m_livePersistentMaps.erase(key); + } + + void PersistentMapTracker::Forget(const BufferObject& buffer) { + m_livePersistentMaps.erase(buffer.GetLifetimeId()); + } + + void PersistentMapTracker::PushBlocksFor(BufferObject& buffer) { + if (!PushIsArmed()) return; + // Re-checked rather than trusted. The set is maintained at five events and a sixth + // one arriving without a NoteMapStateChanged would otherwise push a buffer whose + // shadow has been released - an adopted store's Bytes() is the GPU map, and reading + // it as if it were the shadow is how a "conservative" push turns into a fault. + if (!IsLivePersistentMap(buffer)) { + Forget(buffer); + return; + } + const Uint64 blockBytes = BlockBytes(); + // 0 IS THE NEGATIVE CONTROL, NOT "unlimited" (E3(a)). Pushing one whole-span block + // here would make the control green for the wrong reason - it has to disable the + // push, so that PersistentCoherentMapScenario draws the last uploaded bytes and goes + // red exactly the way an unpushed map does. + if (blockBytes == 0) return; + + const auto range = buffer.GetMappedRange(); + const Uint64 begin = static_cast(range.start); + const Uint64 end = static_cast(range.end); + for (Uint64 at = begin; at < end; at += blockBytes) { + const Uint64 length = (end - at) < blockBytes ? (end - at) : blockBytes; + buffer.PushMappedSpanBlock(static_cast(at), static_cast(length)); + ++m_blocksPushed; + m_bytesPushed += length; + } + } + + void PersistentMapTracker::PushAllMembers() { + if (!PushIsArmed()) return; + if (m_livePersistentMaps.empty()) return; + // Copied out first: PushBlocksFor can erase its own entry (a member that stopped + // being one), and ska::flat_hash_map invalidates on erase. + Vector members; + members.reserve(m_livePersistentMaps.size()); + for (const auto& entry : m_livePersistentMaps) members.push_back(entry.second); + for (BufferObject* buffer : members) { + if (buffer != nullptr) PushBlocksFor(*buffer); + } + } + + void PushPersistentMapsBeforeVerb() { + PersistentMapTracker::Instance().PushAllMembers(); + } + + Bool AdoptTierIsEmulate() { + const Uint32 tier = MG_Config::Ipc.AdoptTier; + if (tier == 2) return true; + // A NAMED refusal, not a silent fall back to T2. T0 (a real cross-process shared + // mapping) and T1 (a server-side staging map) are P11's, and the reason the knob + // parses them today is that the negative control needs a spelling before the thing + // it controls exists. Falling back would make `MOBILEGL_IPC_ADOPT_TIER=0` look like + // a working T0 run and silently produce pmap bytes it must not produce. + // 0 and 1 are the two CONTRACT §5 promises - a real cross-process shared mapping and a + // server-side staging map - and they name P11. Anything else is not a tier at all, and + // saying "P11 implements it" of a 7 would be a lie the operator then repeats. Both die + // here rather than at parse, which is late: the abort lands at the first + // map_persistent, so a mis-set run gets through EGL bring-up and a frame of setup + // first. Moving it to the parse means a knob-validity rule in ConfigLoader, which is + // c0's file; filed for the integrator rather than taken here. + if (tier <= 1) { + MGLOG_F("MGPipe: MOBILEGL_IPC_ADOPT_TIER=%u names adoption tier T%u, which P11 implements " + "and P5 does not; P5 runs at T2 (emulate) only.", + static_cast(tier), static_cast(tier)); + } else { + MGLOG_F("MGPipe: MOBILEGL_IPC_ADOPT_TIER=%u is not an adoption tier; the only values are 0 " + "and 1 (P11) and 2 (emulate, the P5 default).", + static_cast(tier)); + } + std::abort(); + } + +} // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_Remote/Client/PersistentMapTracker.h b/MobileGL/MG_Remote/Client/PersistentMapTracker.h new file mode 100644 index 00000000..c8326a73 --- /dev/null +++ b/MobileGL/MG_Remote/Client/PersistentMapTracker.h @@ -0,0 +1,139 @@ +// MobileGL - MobileGL/MG_Remote/Client/PersistentMapTracker.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// THE BLOCK-GRANULARITY PERSISTENT-MAP PUSH (P5 R-6, CONTRACT-P5.md section 3, table 2's +// second set). Owner: package b1. +// +// WHY THIS EXISTS AT ALL. A coherent persistent map is the one buffer shape with no per-write +// API call: the application memcpys through the pointer and neither a serial, an epoch nor a +// record moves. In monolith that is free, because the pointer IS the backend's GPU storage - +// MEASUREMENTS.md:87 prices the adoption at p99 163 -> 21 ms and ~400 MB saved, and +// ARCHITECTURE.md:481 requires it to hold unmoved for the whole monolith track. Across a +// process boundary the adopted address is meaningless, so P5 runs at tier T2 (emulate): the +// client keeps the shadow, MGPipeApplyMapPersistent declines, and the bytes the application +// wrote with no call have to be SHIPPED. `persistent-map-push` (PipeStats `pmap`) is exactly +// those bytes, and it is structurally zero while adoption survives - which is why forcing T2 +// and wiring the counter are one deliverable and not two. +// +// THE SET. `m_livePersistentMaps` is SyncPersistentMappedRange's own early-out chain +// (BufferObject.cpp:341-353) read as a membership test - mapped, NOT GPU-resident, Persistent, +// Write, NOT FlushExplicit, non-empty mapped range - and nothing else. Reading it as a +// predicate rather than re-deriving one is deliberate: the day that chain grows a sixth +// early-out, a re-derived predicate silently keeps pushing a buffer the monolith path stopped +// pushing, and the two arms diverge with no test able to say so. IsLivePersistentMap() is the +// single spelling; BufferObject::SyncPersistentMappedRange is held to it by a unit case. +// +// THE GRANULARITY. MOBILEGL_IPC_PERSISTENT_BLOCK_KB (default 64) blocks, keyed +// {MGPipeHandle, blockIndex} - the key is implicit, because a block ships as an ORDINARY +// resource_subdata record whose destination range is [blockIndex * blockBytes, + blockBytes). +// NO NEW RECORD KIND: the existing record is already chunked by +// MGPipeForEachSubDataRecordRange and already acceptance-gated, and a second way to say +// "these bytes go there" is a second way to get it wrong. A block size of 0 is E3(a)'s +// NEGATIVE CONTROL and means "push nothing" - not "one unlimited block" - so +// PersistentCoherentMapScenario must go red under it. +// +// PHASE 1 IS CONSERVATIVE, AND SAYS SO. The whole mapped span is pushed, by block, at every +// validate point; no dirty bits, no memcmp. Phase 2 (MOBILEGL_IPC_SHADOW_SHM, P6+) makes it +// precise, and ARCHITECTURE.md:499 grants it the right to be pulled forward if Phase 1 is +// unacceptable on the Create/Flywheel fixtures - the one place in the plan where a +// measurement may reorder phases. +// +// ORDERING IS THE CORRECTNESS PROPERTY, NOT THE GRANULARITY. In monolith the push is a memcpy +// on the same thread as the draw that follows it, so the bytes the application wrote before +// the draw are the bytes the draw sees. Under split both travel SEG_CMD in order, so the ring +// preserves it - PROVIDED the push is emitted AT the validate point and not lazily. A push +// deferred past its own draw record is the C-1 regression re-committed at the transport layer. + +#pragma once +#include + +#include + +namespace MobileGL::MG_State::GLState { + class BufferObject; +} + +namespace MobileGL::MG_Remote::Client { + + class PersistentMapTracker { + public: + // Client-role singleton (table 3: the MG_Impl/MG_Remote/Client singletons are + // client-exclusive). Leaks at exit for ID-8's reason, once per role-local singleton. + static PersistentMapTracker& Instance(); + + // MOBILEGL_IPC_PERSISTENT_BLOCK_KB * 1024. Zero means the push is OFF (E3(a)). + static Uint64 BlockBytes(); + // Transport != Monolith. The whole module is inert on the monolith path: an extra + // resource_subdata record there would be new behaviour, which D-J forbids. + static Bool PushIsArmed(); + + // SyncPersistentMappedRange's early-out chain as a predicate. THE only spelling. + static Bool IsLivePersistentMap(const MG_State::GLState::BufferObject& buffer); + + // Membership maintenance, both idempotent and both safe to call on a buffer that is + // not a member. Called from BufferObject on every event that can move the predicate: + // map, unmap, respecify, adoption, destruction. + void NoteMapStateChanged(MG_State::GLState::BufferObject& buffer); + void Forget(const MG_State::GLState::BufferObject& buffer); + + // One member's whole mapped span, by block. Re-checks the predicate first, so a + // member that stopped being one (an adoption, an unmap that did not route through + // NoteMapStateChanged) is dropped rather than pushed. + void PushBlocksFor(MG_State::GLState::BufferObject& buffer); + + // THE VALIDATE-POINT HOOK. Every member, before the verb record is emitted. + void PushAllMembers(); + + SizeT MemberCount() const { return m_livePersistentMaps.size(); } + Uint64 BlocksPushed() const { return m_blocksPushed; } + Uint64 BytesPushed() const { return m_bytesPushed; } + // Unit tests only: the counters are diagnostics, the set is not reset by it. + void ResetCountersForTest() { + m_blocksPushed = 0; + m_bytesPushed = 0; + } + void ClearForTest() { + m_livePersistentMaps.clear(); + ResetCountersForTest(); + } + + private: + // Keyed on BufferObject::GetLifetimeId(), which is globally unique and never reused - + // never the GL name (LIFO-recycled by glGenBuffers) and never the heap address + // (recycled by the allocator). The raw pointer is safe because every removal path is + // explicit: ~BufferObject and ReleaseMemory both call Forget/NoteMapStateChanged, and + // PushBlocksFor re-checks the predicate before it dereferences anything it kept. + UnorderedMap m_livePersistentMaps; + Uint64 m_blocksPushed = 0; + Uint64 m_bytesPushed = 0; + }; + + // What the client's emit table calls immediately BEFORE emitting any verb that can read a + // buffer (draw, dispatch, readback, blit, present). It is a free function rather than a + // method so the emit table does not have to name the singleton, and so the one-line call + // reads as what it is: "publish everything the application wrote with no call". + // + // In P5 the 21 SyncPersistentMappedRange sites (CONTRACT-P5.md section 3: 9 Espryt + 12 + // Magma, MEASUREMENTS.md:111's 20 being one low) still stand where they are and route + // into PushBlocksFor through BufferObject::SyncPersistentMappedRange, so the push already + // happens at every point monolith pushes at. They retire into THIS call at P8, when the + // draw-path binding walks move to the client. + void PushPersistentMapsBeforeVerb(); + + // R-6's tier gate, and the ONE spelling of it. True for MOBILEGL_IPC_ADOPT_TIER=2, the + // only tier P5 implements; 0 (a real cross-process shared mapping) and 1 (a server-side + // staging map) parse - so the negative control has a name before the thing it controls + // exists - and are a NAMED refusal here rather than a silent fall back to T2. It is asked + // by MGPipeApplyMapPersistent, which is where the decline is decided, so the client's + // three adoption call sites keep their existing "null means declined" branch and the + // map-persistent-roundtrips counter keeps counting ATTEMPTS in both arms (E3(c) asserts + // mpr is equal between the monolith and the split arm, which is only true if the decline + // happens after the count, on the applier's side of the emission). + Bool AdoptTierIsEmulate(); + +} // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp index c038550a..942d572d 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp +++ b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp @@ -12,6 +12,14 @@ #include #include +#if MOBILEGL_BUILD_DISAGGREGATED +// The client role's persistent-map tracker. MG_State reaching into MG_Remote/Client is the +// layering ARCHITECTURE.md:575 names - the CLIENT role IS MG_State plus MG_Impl - and the +// edge exists only in a build that has the transport at all. +#include +#include +#include +#endif namespace MobileGL::MG_State::GLState { namespace { @@ -47,6 +55,13 @@ namespace MobileGL::MG_State::GLState { } BufferObject::~BufferObject() { +#if MOBILEGL_BUILD_DISAGGREGATED + // Unconditional, not behind PushIsArmed(): the transport mode cannot change, but the + // tracker is a leaked singleton whose entries are raw pointers, and an entry that + // outlives its object is the one failure this set must not have. Forget is a no-op + // for a buffer that was never a member. + MG_Remote::Client::PersistentMapTracker::Instance().Forget(*this); +#endif #if MOBILEGL_PIPE_PUSH // P3a D-L: the buffer's death crosses as resource_destroy, which is the catalogue // call for it - no seventh NotifyStateObjectDestroyed raiser is added, because that @@ -166,6 +181,12 @@ namespace MobileGL::MG_State::GLState { } m_size = size; m_resource.ResizeShadow(size); +#if MOBILEGL_BUILD_DISAGGREGATED + // The store this buffer's membership was about no longer exists, and ResizeShadow is + // reserve+resize - a grow past the reserve reallocates - so a pushed block's source + // base has moved too. Both are the same event to the tracker: re-read the predicate. + NotePersistentMapStateChanged(); +#endif } void BufferObject::Respecify(SizeT size, const void* data) { @@ -238,6 +259,14 @@ namespace MobileGL::MG_State::GLState { if (void* base = MG_Pipe::MGPipeEmitMapPersistent(*this)) m_resource.AdoptPersistentMap(base); return; } +#endif +#if MOBILEGL_BUILD_DISAGGREGATED + // R-6 IS "ALWAYS", AND THIS IS ITS SECOND DOOR. MGPipeApplyMapPersistent declines + // every acquisition under split - but a split build whose backend registered no + // MGPipe resource ops falls through to the LEGACY hook below, which would mint a real + // pointer and adopt it. A donated address is meaningless across a process, and an + // inproc lane that adopted would be green for a reason spawn cannot reproduce. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return; #endif if (g_bufferBackendOps == nullptr || g_bufferBackendOps->AcquirePersistentMap == nullptr) return; if (void* base = g_bufferBackendOps->AcquirePersistentMap(*this)) { @@ -303,6 +332,12 @@ namespace MobileGL::MG_State::GLState { m_mappedRange = {0, 0}; m_stagingBias = 0; m_ownsStagingData = false; +#if MOBILEGL_BUILD_DISAGGREGATED + // AFTER the reset, so the predicate reads the post-unmap state, and after the landing + // above, so the last bytes of a write map are already on the wire when the record + // that says "no live writer" goes out behind them. + NotePersistentMapStateChanged(); +#endif } void BufferObject::FlushMemoryRange(SizeT offset, SizeT length) { @@ -339,6 +374,22 @@ namespace MobileGL::MG_State::GLState { } void BufferObject::SyncPersistentMappedRange() { +#if MOBILEGL_BUILD_DISAGGREGATED + // SPLIT: the same span, cut into MOBILEGL_IPC_PERSISTENT_BLOCK_KB blocks, and the + // membership test is this function's own early-out chain read by the tracker rather + // than re-derived there. The 21 sites that call this (CONTRACT-P5.md section 3: 9 + // Espryt + 12 Magma) therefore keep pushing at exactly the points monolith pushes + // at, which is what makes the split arm comparable to the monolith one at all; they + // retire into MG_Remote::Client::PushPersistentMapsBeforeVerb at P8. + // + // A block size of 0 disables the push (E3(a)'s negative control) and this is where + // that is felt: the bytes the application wrote through the pointer never leave, the + // frame draws the last uploaded ones, and PersistentCoherentMapScenario goes red. + if (MG_Remote::Client::PersistentMapTracker::PushIsArmed()) { + MG_Remote::Client::PersistentMapTracker::Instance().PushBlocksFor(*this); + return; + } +#endif if (!m_isMapped) return; // GPU-resident: the app already wrote directly into coherent GPU memory. This is // the whole point of the persistent-map path - the per-draw whole-buffer re-upload @@ -352,6 +403,83 @@ namespace MobileGL::MG_State::GLState { NotifySubData(m_mappedRange.start, m_mappedRange.end - m_mappedRange.start); } +#if MOBILEGL_BUILD_DISAGGREGATED + void BufferObject::PushMappedSpanBlock(SizeT offset, SizeT size) { + // NotifySubData and not MGPipeEmitResourceSubData directly: the serial bump, the + // defined-content promotion and the legacy-ops fallback are what the monolith span + // push does, and a second route to the same record is a second thing to keep in step. + if (size == 0) return; + NotifySubData(offset, size); + if (MG_Util::PipeStats::Enabled()) { + // THE persistent-map-push SITE: the bytes an application wrote through a map with + // no API call, which a split build therefore has to ship. + // + // THESE BYTES ARE ALSO COUNTED AS stage-buffer, and that overlap is stated in the + // inventory rather than avoided. At tier T2 a pushed block IS an ordinary + // resource_subdata, so it reaches Ops_H_SubData, is queued into pendingRanges and + // is staged like any other write. Subtracting it at the staging site would make + // stage-buffer under-report what the BACKEND actually moves, which is the question + // that class exists to answer; the two counters are different questions about the + // same bytes and PipeStats.cpp:33-35 now says so. + MG_Util::PipeStats::AddBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush, + static_cast(size)); + } + } + + void BufferObject::NotePersistentMapStateChanged() { + if (!MG_Remote::Client::PersistentMapTracker::PushIsArmed()) return; + MG_Remote::Client::PersistentMapTracker::Instance().NoteMapStateChanged(*this); + + // THE LIVE-HOST-WRITES BIT (ARCHITECTURE.md 12, CONTRACT-P5.md section 3). A live + // WRITE map - persistent or not - mutates the shadow with no call, no serial and no + // epoch, which is exactly why IsBufferDrawCleanByHandle had to ask the frontend object + // whether it was mapped. Under a spawn there is no object on that side, so the fact + // has to cross; it rides MGPSubData's pad (see MGPipeTypes.h, and b1-v1.md 3.1 for + // why not MGPResourceDesc's). + // + // BOTH EDGES EMIT A RECORD, AND THE RISING ONE IS NOT OPTIONAL. An earlier cut let the + // rising edge emit nothing on the grounds that "the next content record carries the + // bit anyway" - and for the single idiom this feature exists to serve, a persistently + // mapped streaming arena behind a static VAO, there IS no next content record. The + // only one is the block push, the push for a vertex/uniform/SSBO map runs inside + // EnsureBufferResourceForHandle, and a draw-clean answer SKIPS that ensure + // (DirectGLES.cpp:691) and then LATCHES it (:697, vboCleanEpoch - and a coherent + // persistent map bumps no mutation epoch, which is its whole point). So: draw 1 + // pushes, draw 2 probes clean, draw 3 onward never probes again, and the frame draws + // frame 1's bytes for ever with no diagnostic. That is Managers.cpp:2650-2655's + // regression one layer down, and the rising-edge record is what breaks the cycle: the + // server learns a host writer is live BEFORE the first probe, answers dirty while the + // map lives, and the ensure - and with it the push - runs every draw. + const Bool live = m_isMapped && (m_mappingAccess & BufferMappingAccessBit::Write) && + !m_resource.IsGpuResident(); + if (live == m_publishedLiveHostWrites) return; + m_publishedLiveHostWrites = live; + + // ONE BLOCK, NOT THE SPAN. On the falling edge the span's bytes have already shipped + // on this same path (ReleaseMemory lands the staged writes and calls + // NotifyFlushMappedRange before this runs); on the rising edge the span has not been + // written yet. Either way the record exists to carry the STATE, and the bytes it + // carries are real, current and the cheapest honest ones there are. A zero-length + // record would have been the alternative and it is illegal by contract rule A. + if (m_size == 0) return; + const Uint64 blockBytes = MG_Remote::Client::PersistentMapTracker::BlockBytes(); + // MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 TURNS THE WHOLE MECHANISM OFF, STATE RECORD + // INCLUDED (E3(a)). An earlier cut skipped the blocks and still shipped a whole buffer + // here, which meant an unmap delivered the bytes the control exists to withhold - and + // a negative control that still delivers them is not a control. With the knob at 0 + // nothing is published and nothing is pushed, so the probe answers clean, the frame + // draws the last uploaded bytes, and PersistentCoherentMapScenario goes red. + if (blockBytes == 0) return; + const SizeT length = blockBytes >= static_cast(m_size) ? m_size + : static_cast(blockBytes); + NotifySubData(0, length); + } + + Bool BufferObject::HasLiveHostWritesForWire() const { + return m_publishedLiveHostWrites; + } +#endif + void BufferObject::WritebackFromBackend(DataPtr data, SizeT atOffset) { MOBILEGL_ASSERT(atOffset + data.size <= m_size, "WritebackFromBackend out of bounds: atOffset (%zu) + data.size (%zu) > m_size (%zu)", atOffset, @@ -359,6 +487,19 @@ namespace MobileGL::MG_State::GLState { Memcpy(m_resource.Bytes() + atOffset, data.data, data.size); ++m_changeSerial; MGP_NOTE_AGGREGATE(BufferChange); +#if MOBILEGL_BUILD_DISAGGREGATED + // THE THIRD STATE'S ONLY EXIT. In a split build SyncGpuWrites does NOT clear the + // flag before emitting, because between the emission and the answer the shadow is + // stale and the object has no way to say so; the answer landing here is what makes it + // current, so the answer is what clears. Gated: in monolith SyncGpuWrites has already + // cleared by the time the applier calls back, and a second clear here would also + // retire a GPU write announced by something other than a readback - a ReadPixels into + // a pack PBO writes back through this same function. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith && + atOffset == 0 && data.size >= m_size) { + m_gpuWritePending = false; + } +#endif } void BufferObject::MarkGpuWritten() { @@ -368,6 +509,44 @@ namespace MobileGL::MG_State::GLState { void BufferObject::SyncGpuWrites() { if (!m_gpuWritePending) return; +#if MOBILEGL_BUILD_DISAGGREGATED + // THE THIRD STATE (CONTRACT-P5.md section 3, ARCHITECTURE.md:509). This function has + // only ever been able to say "pending" or "not pending", and clearing first is safe + // in monolith because the readback is synchronous INSIDE the applier: the caller sees + // the reconciled shadow on return. Under a transport there is a third state - the + // readback was emitted and the answer has not arrived - which the flag cannot express, + // and clearing optimistically leaves the shadow silently stale for the object's life. + // + // So: emit, then BLOCK until OnBufferWriteback lands, and let the writeback do the + // clearing. ARCHITECTURE.md:509 lists "the first CPU read of a GPU-write-pending + // buffer" among the UNAVOIDABLE blocking points, because monolith already glFinish()es + // here; under R-1's verb barrier the block is nearly free, since the client is already + // waiting for appliedSeq to reach the record it just emitted. + // + // NO NARROWING (ResourceTracker.h:587-592's rangeCount == 1 assertion stays): this + // phase only gives the client a conservative set, and a zero-range announcement stays + // illegal until P8/P9 make it mean "fully narrowed - nothing is dirty". + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { +#if MOBILEGL_PIPE_PUSH + if (m_size != 0 && MG_Pipe::MGPipeResourceSubsystemEnabled()) { + MG_Pipe::MGPipeEmitResourceReadback(*this); + // The wait is the barrier's wait: the reply slot id IS the record's seq, so + // "my answer is back" and "appliedSeq reached me" are one condition. With no + // session (a build-split lane running monolith, and every unit case) the + // emission was synchronous and the writeback has already cleared the flag. + MG_Remote::Client::AwaitBufferWriteback(*this); + } +#endif + // A buffer with no readback route - no size, or a backend that registered no + // resource ops - can never catch up, and retrying on every subsequent read would + // only repeat the same no-op. That is the ONE case the monolith clear covers that + // the writeback cannot, so it is spelled out here rather than inherited. + if (m_gpuWritePending && !MG_Remote::Client::BufferWritebackIsReachable(*this)) { + m_gpuWritePending = false; + } + return; + } +#endif // Cleared unconditionally: without a readback op the shadow can never catch up, // and retrying on every subsequent read would only repeat the same no-op. m_gpuWritePending = false; @@ -564,6 +743,12 @@ namespace MobileGL::MG_State::GLState { m_mappingAccess = (read ? BufferMappingAccessBit::Read : BufferMappingAccessBit::Null) | (write ? BufferMappingAccessBit::Write : BufferMappingAccessBit::Null); m_mappedRange = {0, m_size}; +#if MOBILEGL_BUILD_DISAGGREGATED + // glMapBuffer never takes the Persistent bit, so this buffer can never join the + // push set - but a WRITE map still mutates the shadow with no call, which is the + // half of the live-host-writes bit that is not about the push at all. + NotePersistentMapStateChanged(); +#endif if (m_mappingAccess & BufferMappingAccessBit::Write) { // glMapBuffer maps from offset 0, so no bias: the allocation's own @@ -605,6 +790,10 @@ namespace MobileGL::MG_State::GLState { m_resource.AdoptPersistentMap(pushedBase); return true; } +#endif +#if MOBILEGL_BUILD_DISAGGREGATED + // R-6's second door, as in TryAdoptLargeStorage above. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return false; #endif if (m_size == 0 || g_bufferBackendOps == nullptr || g_bufferBackendOps->AcquirePersistentMap == nullptr) { return false; @@ -641,6 +830,15 @@ namespace MobileGL::MG_State::GLState { m_isMapped = true; m_mappingAccess = access; m_mappedRange = range; +#if MOBILEGL_BUILD_DISAGGREGATED + // BEFORE the adoption attempt below, deliberately. Under R-6 the acquisition always + // declines, so the predicate this publishes is already final; if a later phase ever + // mints one, the adoption path notes the change itself (it does now - the call is + // beside AdoptPersistentMap) and the entry is withdrawn there rather than never + // having been made, which is the order that keeps the set conservative under both + // answers. + NotePersistentMapStateChanged(); +#endif if (access & BufferMappingAccessBit::Persistent) { m_ownsStagingData = false; @@ -656,13 +854,27 @@ namespace MobileGL::MG_State::GLState { MG_Pipe::MGPipeResourceSubsystemEnabled()) { if (void* pushedBase = MG_Pipe::MGPipeEmitMapPersistent(*this)) { m_resource.AdoptPersistentMap(pushedBase); +#if MOBILEGL_BUILD_DISAGGREGATED + // An adoption takes the buffer OUT of the push set and out of the + // live-host-writes state: the application now writes coherent GPU memory + // and there is nothing to ship. Under R-6 this is unreachable; it is here + // because the comment below used to claim the adoption path withdrew the + // entry and nothing did, which would have left the published bit true + // across an adoption until unmap (latent for P11). + NotePersistentMapStateChanged(); +#endif } return m_resource.Bytes() + range.start; } #endif if (!m_resource.IsGpuResident() && (access & BufferMappingAccessBit::Write) && - !(access & BufferMappingAccessBit::FlushExplicit) && g_bufferBackendOps && - g_bufferBackendOps->AcquirePersistentMap) { + !(access & BufferMappingAccessBit::FlushExplicit) && +#if MOBILEGL_BUILD_DISAGGREGATED + // R-6's second door, as in TryAdoptLargeStorage: no legacy mint under a + // transport, whatever the backend registered. + MG_Config::Transport == MG_Config::TransportMode::Monolith && +#endif + g_bufferBackendOps && g_bufferBackendOps->AcquirePersistentMap) { if (void* base = g_bufferBackendOps->AcquirePersistentMap(*this)) { m_resource.AdoptPersistentMap(base); } diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.h b/MobileGL/MG_State/GLState/BufferState/BufferObject.h index f9674530..9d853a2c 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.h +++ b/MobileGL/MG_State/GLState/BufferState/BufferObject.h @@ -187,6 +187,49 @@ namespace MobileGL { // from every path that reads the shadow on the app's behalf. void SyncGpuWrites(); +#if MOBILEGL_BUILD_DISAGGREGATED + // ---- P5 b1: the two things a split build has to do that a monolith does not --- + // + // Everything here is behind the build option AND behind + // `MG_Config::Transport != Monolith` at the call site, because an extra + // resource_subdata record or an extra respecify on the monolith path is new + // behaviour and D-J forbids it. The pull build compiles none of it, which is also + // how G1 holds over a file this central. + + // ONE MOBILEGL_IPC_PERSISTENT_BLOCK_KB BLOCK of the mapped span, emitted through + // the private NotifySubData - the same serial, the same defined-content flag, the + // same record - so the split arm and the monolith arm differ in HOW the span is + // cut and in nothing else. Called only by + // MG_Remote::Client::PersistentMapTracker, which owns the cutting. + void PushMappedSpanBlock(SizeT offset, SizeT size); + + // Called on every event that can move the tracker's membership predicate: map, + // unmap, respecify, adoption, destruction. It is one call rather than an + // insert/erase pair on purpose - the predicate is read from this object, so a + // caller that had to decide which of the two to call could decide differently + // from IsLivePersistentMap and the set would drift from the thing it models. + // + // It also PUBLISHES the live-host-writes bit when it changes: the server must + // know that a write map is live, because such a map mutates the shadow with no + // call, no serial and no epoch, and the applier's IsBufferDrawCleanByHandle can + // no longer ask this object (there is no object on that side of a spawn). + void NotePersistentMapStateChanged(); + + // What MGPipeEmitResourceSubData and MGPipeEmitBufferSubDataResident write into + // MGPSubData::HasLiveHostWrites. The PUBLISHED value, not the live predicate: the + // two are the same by the time any content record is built, and reading the + // published one is what makes a record and the edge that announced it agree by + // construction. + Bool HasLiveHostWritesForWire() const; + + // Is a GPU write still unreconciled? Under split this is the THIRD STATE made + // readable: SyncGpuWrites no longer clears optimistically, so between the readback + // emission and OnBufferWriteback landing this stays true, and nothing else in the + // object can express that. Split-only so the pull build's layout and inlining do + // not move (G1). + Bool HasOutstandingGpuWrite() const { return m_gpuWritePending; } +#endif + Bool IsMapped() const; Bool IsImmutableStorage() const; SizeT GetSize() const; @@ -264,8 +307,16 @@ namespace MobileGL { Uint64 m_changeSerial = 0; // See HasDefinedContent(). Bool m_hasDefinedContent = true; - // Set by MarkGpuWritten, cleared by SyncGpuWrites once the shadow is refreshed. + // Set by MarkGpuWritten, cleared by SyncGpuWrites once the shadow is refreshed - + // and, in a split build, cleared by WritebackFromBackend instead, because there + // the answer arrives later than the request. See SyncGpuWrites' definition. Bool m_gpuWritePending = false; +#if MOBILEGL_BUILD_DISAGGREGATED + // The last value of MGPResourceDesc::HasLiveHostWrites this object published. + // Behind the option so the pull build's layout - and therefore every inlined + // constructor and accessor in it - does not move (G1). + Bool m_publishedLiveHostWrites = false; +#endif Range1D m_mappedRange; // The write-map staging store. MapAlignedData because the application is handed a // pointer into it, and biased by m_stagingBias because ARB_map_buffer_alignment diff --git a/MobileGL/MG_Test/Buffer/CMakeLists.txt b/MobileGL/MG_Test/Buffer/CMakeLists.txt index 2ac5fb76..6d031888 100644 --- a/MobileGL/MG_Test/Buffer/CMakeLists.txt +++ b/MobileGL/MG_Test/Buffer/CMakeLists.txt @@ -23,5 +23,33 @@ if (MSVC) target_compile_options(BufferTest PRIVATE /Zc:preprocessor) endif() +# P5 b1: the split buffer side - the client GPU-write set, the persistent-map block push and +# tier 1 of the flush ladder. Built in EVERY configuration on purpose; the cases that need +# MOBILEGL_BUILD_DISAGGREGATED skip elsewhere rather than disappearing, so the ctest name set +# is the same in the pull and the push lanes (G2) and build-split removes none of them (G14). +add_executable( + SplitBufferTest + SplitBufferTest.cpp +) + +target_include_directories(SplitBufferTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL + ${MGL_ROOT}/3rdparty/xxHash + ${MGL_ROOT}/3rdparty/Vulkan-Headers/include + ${MGL_ROOT}/3rdparty/SPIRV-Reflect +) + +target_link_libraries( + SplitBufferTest PRIVATE + GTest::gtest_main + ${LINK_LIBRARIES} +) + +if (MSVC) + target_compile_options(SplitBufferTest PRIVATE /Zc:preprocessor) +endif() + include(GoogleTest) gtest_discover_tests(BufferTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) +gtest_discover_tests(SplitBufferTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) diff --git a/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp b/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp new file mode 100644 index 00000000..a1370ddb --- /dev/null +++ b/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp @@ -0,0 +1,496 @@ +// MobileGL - MobileGL/MG_Test/Buffer/SplitBufferTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// P5 (b1): the buffer side of the split - the client-side conservative GPU-write set, the +// block-granularity persistent-map push, and tier 1 of the flush ladder. +// +// EVERY CASE IS COMPILED IN ALL FOUR BUILDS AND SKIPS OUTSIDE build-split, deliberately. The +// names then exist identically in the pull and the push lane (G2 stays at 0 diff lines) and +// build-split adds none of its own (G14 stays at 0 removed), while a lane that cannot run a +// case says so instead of quietly not having it. + +#include + +#include +#include +#include +#include +#include +#include + +#if MOBILEGL_PIPE_PUSH +#include +#endif + +#if MOBILEGL_BUILD_DISAGGREGATED +#include +#include +#endif + +using namespace MobileGL; + +namespace { + +#if MOBILEGL_BUILD_DISAGGREGATED + using MG_State::GLState::BufferObject; + using MG_Remote::Client::GpuWriteProducer; + using MG_Remote::Client::PersistentMapTracker; + + // Everything in this package is gated on `Transport != Monolith`, so every case has to + // put the process into a split configuration and put it back. A fixture rather than a + // lambda because the tracker is a process-wide singleton and a case that left an entry in + // it would poison the next one through a raw pointer to a destroyed buffer - which is + // exactly the failure mode the tracker's own Forget() exists to prevent. + class SplitBufferSet : public ::testing::Test { + protected: + void SetUp() override { + m_transport = MG_Config::Transport; + m_blockKb = MG_Config::Ipc.PersistentBlockKb; + m_adoptTier = MG_Config::Ipc.AdoptTier; + m_pipeStats = MG_Config::Features.PipeStats; + m_context = Move(MG_State::pGLContext); + MG_State::pGLContext = MakeUnique(); + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.PersistentBlockKb = 64; + MG_Config::Ipc.AdoptTier = 2; + MG_Config::Features.PipeStats = true; + MG_Util::PipeStats::Init(); + PersistentMapTracker::Instance().ClearForTest(); + MG_Remote::Client::ResetProducerMarkCountsForTest(); + } + void TearDown() override { + PersistentMapTracker::Instance().ClearForTest(); + MG_State::pGLContext = Move(m_context); + MG_Config::Transport = m_transport; + MG_Config::Ipc.PersistentBlockKb = m_blockKb; + MG_Config::Ipc.AdoptTier = m_adoptTier; + MG_Config::Features.PipeStats = m_pipeStats; + MG_Util::PipeStats::Init(); + } + + static SharedPtr MakeBuffer(Uint index, SizeT size) { + auto buffer = MakeShared(index); + buffer->Respecify(size, nullptr); + return buffer; + } + + MG_Config::TransportMode m_transport = MG_Config::TransportMode::Monolith; + Uint32 m_blockKb = 64; + Uint32 m_adoptTier = 2; + Bool m_pipeStats = false; + UniquePtr m_context; + }; +#endif + +#define MGL_SPLIT_ONLY_OR_SKIP() \ + do { \ + GTEST_SKIP() << "the client-side GPU-write set and the persistent-map push exist only in " \ + "a MOBILEGL_BUILD_DISAGGREGATED build"; \ + } while (0) + +} // namespace + +// ===================================================================================== +// The GPU-write set: one case per row of CONTRACT-P5.md section 3's first table. +// ===================================================================================== + +#if MOBILEGL_BUILD_DISAGGREGATED + +// Row 0 - DirectGLES.cpp:570 / UniformManager.cpp:1231. +TEST_F(SplitBufferSet, Row0EverySsboBindingPointIsMarkedByADraw) { + auto ssbo = MakeBuffer(11u, 256); + MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, 0).Bind(ssbo); + MG_State::pGLContext->TouchBufferBindingPoint(BufferTarget::ShaderStorage, 0); + + MG_Remote::Client::MarkGpuWritesForDraw(); + + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::ShaderStorageBinding), 1u) + << "a draw with an SSBO bound must mark it: the shader writes into the driver's buffer, " + "behind the shadow glMapBuffer and glGetBufferSubData read"; +} + +// Row 1 - DirectGLES.cpp:618. The point of a counter is that the shader increments it. +TEST_F(SplitBufferSet, Row1EveryBoundAtomicCounterIsMarkedByADraw) { + auto counter = MakeBuffer(12u, 64); + MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::AtomicCounter, 0).Bind(counter); + MG_State::pGLContext->TouchBufferBindingPoint(BufferTarget::AtomicCounter, 0); + + MG_Remote::Client::MarkGpuWritesForDraw(); + + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::AtomicCounterBinding), 1u); +} + +// Row 2's DISCRIMINATOR - DirectGLES.cpp:2354-2357. This is the one row whose backend twin is +// narrow on purpose, so the narrowness is what the case is about: a GL_READ_ONLY image binding +// must NOT be marked, or the next map waits on and re-reads a dispatch that cannot have changed +// a byte of it. +TEST_F(SplitBufferSet, Row2OnlyAWritableImageBufferTextureCounts) { + MG_State::GLState::ImageTextureBinding empty{}; + EXPECT_FALSE(MG_Remote::Client::ImageUnitIsAWritableBufferTexture(empty)) + << "an unbound image unit is not a GPU write"; + + auto texture = MakeShared(7u); + auto backing = MakeBuffer(13u, 128); + texture->GetBufferBindingSlot().Bind(backing); + + MG_State::GLState::ImageTextureBinding readOnly{}; + readOnly.Texture = texture; + readOnly.Access = GL_READ_ONLY; + EXPECT_FALSE(MG_Remote::Client::ImageUnitIsAWritableBufferTexture(readOnly)) + << "a GL_READ_ONLY image binding is left alone by the backend twin and must be left " + "alone here"; + + MG_State::GLState::ImageTextureBinding writable = readOnly; + writable.Access = GL_READ_WRITE; + EXPECT_TRUE(MG_Remote::Client::ImageUnitIsAWritableBufferTexture(writable)); +} + +// Row 2's WALK, through the image unit the context actually holds. +TEST_F(SplitBufferSet, Row2AWritableImageBufferTextureIsMarkedByADraw) { + auto texture = MakeShared(8u); + auto backing = MakeBuffer(14u, 128); + texture->GetBufferBindingSlot().Bind(backing); + auto& binding = MG_State::pGLContext->GetImageTextureBinding(0); + binding.Texture = texture; + binding.Access = GL_WRITE_ONLY; + + MG_Remote::Client::MarkGpuWritesForDraw(); + + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::WritableImageBufferTexture), 1u); + + binding = MG_State::GLState::ImageTextureBinding{}; +} + +// Row 3 - VulkanRenderer.cpp:11618. With no capture active there is nothing to mark, and that +// gate is the half worth pinning: a mark taken with no active capture would mark whatever the +// binding points happened to hold from a previous one. +TEST_F(SplitBufferSet, Row3TransformFeedbackTargetsAreOnlyMarkedWhileACaptureIsActive) { + auto target = MakeBuffer(15u, 256); + MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::TransformFeedback, 0).Bind(target); + MG_State::pGLContext->TouchBufferBindingPoint(BufferTarget::TransformFeedback, 0); + + ASSERT_FALSE(MG_State::pGLContext->IsTransformFeedbackActive()); + MG_Remote::Client::MarkGpuWritesForDraw(); + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::TransformFeedbackCapture), 0u); + + // The marking itself, driven at the row rather than through the capture state machine. + MG_Remote::Client::MarkBufferForProducer(target, GpuWriteProducer::TransformFeedbackCapture); + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::TransformFeedbackCapture), 1u); +} + +// Row 4 - P5's own: glReadPixels into a bound GL_PIXEL_PACK_BUFFER. +TEST_F(SplitBufferSet, Row4AReadPixelsIntoAPackPboMarksThePbo) { + MG_Remote::Client::MarkReadPixelsPackBuffer(); + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::ReadPixelsPackBuffer), 0u) + << "a read into client memory binds no PBO and must mark nothing"; + + auto pbo = MakeBuffer(16u, 1024); + MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelPack).Bind(pbo); + MG_Remote::Client::MarkReadPixelsPackBuffer(); + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::ReadPixelsPackBuffer), 1u); +} + +// Row 5 - P5's own: glEndTransformFeedback, in place of the unbounded ClientWaitSync. +TEST_F(SplitBufferSet, Row5EndTransformFeedbackMarksTheCaptureTargets) { + auto target = MakeBuffer(17u, 256); + ASSERT_FALSE(MG_State::pGLContext->IsTransformFeedbackActive()); + MG_Remote::Client::MarkEndTransformFeedbackCaptureTargets(); + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::EndTransformFeedbackCapture), 0u); + + MG_Remote::Client::MarkBufferForProducer(target, GpuWriteProducer::EndTransformFeedbackCapture); + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(GpuWriteProducer::EndTransformFeedbackCapture), 1u); +} + +// THE GATE ITSELF. On the monolith path the six backend sites are still the only producers and +// a second marker would be new behaviour (D-J) - and rows 4 and 5 would remove a stall that +// monolith is entitled to keep. +TEST_F(SplitBufferSet, TheWholeSetIsInertOnTheMonolithPath) { + MG_Config::Transport = MG_Config::TransportMode::Monolith; + auto ssbo = MakeBuffer(18u, 256); + MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, 0).Bind(ssbo); + MG_State::pGLContext->TouchBufferBindingPoint(BufferTarget::ShaderStorage, 0); + + MG_Remote::Client::MarkGpuWritesForDraw(); + MG_Remote::Client::MarkGpuWritesForDispatch(); + MG_Remote::Client::MarkReadPixelsPackBuffer(); + MG_Remote::Client::MarkEndTransformFeedbackCaptureTargets(); + + for (SizeT row = 0; row < static_cast(GpuWriteProducer::Count); ++row) { + EXPECT_EQ(MG_Remote::Client::ProducerMarkCount(static_cast(row)), 0u) + << "row " << row << " fired with Transport == Monolith"; + } +} + +// ===================================================================================== +// The persistent-map push +// ===================================================================================== + +// The membership predicate IS SyncPersistentMappedRange's early-out chain, and the two must +// answer the same thing about the same buffer. A re-derived predicate that drifted would push +// a buffer monolith stopped pushing, and no other test could see it. +TEST_F(SplitBufferSet, MembershipIsSyncPersistentMappedRangesOwnEarlyOutChain) { + auto buffer = MakeBuffer(20u, 4096); + EXPECT_FALSE(PersistentMapTracker::IsLivePersistentMap(*buffer)) << "not mapped"; + + buffer->AcquireMemoryRange(Range1D{0, 4096}, + BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + ASSERT_FALSE(buffer->IsBackendPersistentMapped()) + << "the acquisition was minted, so this is the adopted arm and not the one under test"; + EXPECT_TRUE(PersistentMapTracker::IsLivePersistentMap(*buffer)); + EXPECT_EQ(PersistentMapTracker::Instance().MemberCount(), 1u); + + buffer->ReleaseMemory(false); + EXPECT_FALSE(PersistentMapTracker::IsLivePersistentMap(*buffer)); + EXPECT_EQ(PersistentMapTracker::Instance().MemberCount(), 0u); + + // FLUSH_EXPLICIT is the early-out that is easiest to lose: the application announces its + // own writes with glFlushMappedBufferRange, which already crosses as resource_flush_range. + buffer->AcquireMemoryRange(Range1D{0, 4096}, BufferMappingAccessBit::Write | + BufferMappingAccessBit::Persistent | + BufferMappingAccessBit::FlushExplicit); + EXPECT_FALSE(PersistentMapTracker::IsLivePersistentMap(*buffer)); + EXPECT_EQ(PersistentMapTracker::Instance().MemberCount(), 0u); + buffer->ReleaseMemory(false); +} + +// pmap is non-zero, and it is non-zero in BLOCKS. +TEST_F(SplitBufferSet, ThePushCutsTheMappedSpanIntoBlocksAndMovesPmap) { + constexpr SizeT kSize = 4u * 64u * 1024u; // exactly four 64 KiB blocks + auto buffer = MakeBuffer(21u, kSize); + buffer->AcquireMemoryRange(Range1D{0, kSize}, + BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + ASSERT_TRUE(PersistentMapTracker::IsLivePersistentMap(*buffer)); + + const Uint64 before = MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush); + MG_Remote::Client::PushPersistentMapsBeforeVerb(); + + EXPECT_EQ(PersistentMapTracker::Instance().BlocksPushed(), 4u) + << "a 256 KiB span at a 64 KiB block size is four records, not one"; + EXPECT_EQ(PersistentMapTracker::Instance().BytesPushed(), static_cast(kSize)); + EXPECT_EQ(MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush) - before, + static_cast(kSize)) + << "persistent-map-push is wired and counts the bytes the client had to ship because " + "MapPersistent declined"; + + buffer->ReleaseMemory(false); +} + +// A span that is not a whole number of blocks keeps its tail. +TEST_F(SplitBufferSet, TheLastBlockIsTheRemainderAndNotAWholeBlock) { + constexpr SizeT kSize = 64u * 1024u + 7u; + auto buffer = MakeBuffer(22u, kSize); + buffer->AcquireMemoryRange(Range1D{0, kSize}, + BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + MG_Remote::Client::PushPersistentMapsBeforeVerb(); + EXPECT_EQ(PersistentMapTracker::Instance().BlocksPushed(), 2u); + EXPECT_EQ(PersistentMapTracker::Instance().BytesPushed(), static_cast(kSize)); + buffer->ReleaseMemory(false); +} + +// THE STATE RECORD ON BOTH EDGES (B-1). The rising edge is what breaks the cycle the probe +// would otherwise latch: a coherent persistent map behind a static VAO emits NO content record +// of its own until the push runs, the push runs inside the ensure path, and a draw-clean answer +// skips that ensure and then latches it. So the map itself has to publish, and the unmap has to +// publish the retraction - one block each, observable here as exactly one serial bump each. +TEST_F(SplitBufferSet, BothEdgesOfAWriteMapPublishOneStateRecord) { + constexpr SizeT kSize = 4096; + auto buffer = MakeBuffer(26u, kSize); + EXPECT_FALSE(buffer->HasLiveHostWritesForWire()); + + const Uint64 beforeMap = buffer->GetChangeSerial(); + buffer->AcquireMemoryRange(Range1D{0, kSize}, + BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + EXPECT_TRUE(buffer->HasLiveHostWritesForWire()); + EXPECT_EQ(buffer->GetChangeSerial(), beforeMap + 1) + << "the rising edge published nothing, so the server cannot know a host writer is live " + "until a content record it may never emit"; + + const Uint64 beforeUnmap = buffer->GetChangeSerial(); + buffer->ReleaseMemory(/*landStagedWrites=*/true); + EXPECT_FALSE(buffer->HasLiveHostWritesForWire()); + EXPECT_EQ(buffer->GetChangeSerial(), beforeUnmap + 2) + << "the unmap's own NotifyFlushMappedRange plus the falling-edge state record: without " + "the second the record stays dirty for the buffer's life"; +} + +// E3(a)'s NEGATIVE CONTROL: 0 disables the push, it does not mean "one unlimited block" - and +// it disables the STATE RECORDS with it. An earlier cut skipped the blocks and still shipped a +// whole buffer at unmap, so a scenario that unmapped before reading back went green and the +// control was dead. +TEST_F(SplitBufferSet, AZeroBlockSizeTurnsThePushOffRatherThanMakingItUnlimited) { + MG_Config::Ipc.PersistentBlockKb = 0; + constexpr SizeT kSize = 128u * 1024u; + auto buffer = MakeBuffer(23u, kSize); + + const Uint64 beforeMap = buffer->GetChangeSerial(); + buffer->AcquireMemoryRange(Range1D{0, kSize}, + BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + ASSERT_TRUE(PersistentMapTracker::IsLivePersistentMap(*buffer)); + EXPECT_EQ(buffer->GetChangeSerial(), beforeMap) + << "the rising-edge state record went out with the push disabled"; + + const Uint64 before = MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush); + MG_Remote::Client::PushPersistentMapsBeforeVerb(); + EXPECT_EQ(PersistentMapTracker::Instance().BlocksPushed(), 0u); + EXPECT_EQ(MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush), before) + << "MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 must ship nothing, so that " + "PersistentCoherentMapScenario goes red under it"; + + const Uint64 beforeUnmap = buffer->GetChangeSerial(); + buffer->ReleaseMemory(/*landStagedWrites=*/true); + EXPECT_EQ(buffer->GetChangeSerial(), beforeUnmap + 1) + << "with the push off the unmap must emit only its own NotifyFlushMappedRange - a " + "falling-edge record here delivers a whole buffer the control exists to withhold, " + "and a negative control that still delivers the bytes is not a control"; +} + +// The set does not keep a pointer to a dead buffer. +TEST_F(SplitBufferSet, ADestroyedBufferLeavesTheSet) { + { + auto buffer = MakeBuffer(24u, 4096); + buffer->AcquireMemoryRange(Range1D{0, 4096}, + BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + ASSERT_EQ(PersistentMapTracker::Instance().MemberCount(), 1u); + } + EXPECT_EQ(PersistentMapTracker::Instance().MemberCount(), 0u) + << "~BufferObject must Forget() itself: the set holds raw pointers keyed on the " + "lifetime id, and an entry that outlives its object is the one failure it cannot have"; +} + +// SyncGpuWrites' THIRD STATE. Monolith clears the flag before it emits, which is safe only +// because the readback runs synchronously inside the applier; under a transport that clear +// leaves the shadow silently stale for the object's life, so the WRITEBACK clears it instead. +TEST_F(SplitBufferSet, UnderSplitTheWritebackClearsThePendingFlagAndNotTheRequest) { + auto buffer = MakeBuffer(25u, 256); + buffer->MarkGpuWritten(); + ASSERT_TRUE(buffer->HasOutstandingGpuWrite()); + + Vector bytes(256, static_cast(0x5A)); + buffer->WritebackFromBackend(DataPtr{bytes.data(), bytes.size()}, 0); + EXPECT_FALSE(buffer->HasOutstandingGpuWrite()) + << "the answer landing is what makes the shadow current, so the answer is what clears"; + + // A PARTIAL writeback is not an answer to a whole-buffer readback and must not clear: + // GL_Drawing's transform-feedback strip fixup writes back three vertices at a time. + buffer->MarkGpuWritten(); + buffer->WritebackFromBackend(DataPtr{bytes.data(), 16}, 0); + EXPECT_TRUE(buffer->HasOutstandingGpuWrite()); + + // And with no readback route at all - no size, or a backend that registered no resource + // ops - SyncGpuWrites must clear rather than block for ever. That is the ONE case + // monolith's unconditional clear covers that a writeback cannot. + EXPECT_FALSE(MG_Remote::Client::BufferWritebackIsReachable(*buffer)); + buffer->SyncGpuWrites(); + EXPECT_FALSE(buffer->HasOutstandingGpuWrite()); +} + +// R-6's tier gate. T2 is the only tier P5 implements; the other two are a NAMED refusal and +// their spelling exists now so the P11 negative control has one. +TEST_F(SplitBufferSet, OnlyAdoptTierTwoIsImplemented) { + EXPECT_TRUE(MG_Remote::Client::AdoptTierIsEmulate()); + MG_Config::Ipc.AdoptTier = 0; + EXPECT_DEATH(MG_Remote::Client::AdoptTierIsEmulate(), ""); + MG_Config::Ipc.AdoptTier = 1; + EXPECT_DEATH(MG_Remote::Client::AdoptTierIsEmulate(), ""); + MG_Config::Ipc.AdoptTier = 2; +} + +#else + +// THE SAME FIFTEEN NAMES, SO THE ctest NAME SET DOES NOT MOVE BETWEEN LANES. G2 compares the +// pull and push name lists line for line and G14 allows build-split to ADD names but never to +// remove one, so a case that exists only where it can run would break both gates for a reason +// that has nothing to do with what it tests. It skips instead, and says why. +TEST(SplitBufferSet, Row0EverySsboBindingPointIsMarkedByADraw) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, Row1EveryBoundAtomicCounterIsMarkedByADraw) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, Row2OnlyAWritableImageBufferTextureCounts) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, Row2AWritableImageBufferTextureIsMarkedByADraw) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, Row3TransformFeedbackTargetsAreOnlyMarkedWhileACaptureIsActive) { + MGL_SPLIT_ONLY_OR_SKIP(); +} +TEST(SplitBufferSet, Row4AReadPixelsIntoAPackPboMarksThePbo) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, Row5EndTransformFeedbackMarksTheCaptureTargets) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, TheWholeSetIsInertOnTheMonolithPath) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, MembershipIsSyncPersistentMappedRangesOwnEarlyOutChain) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, ThePushCutsTheMappedSpanIntoBlocksAndMovesPmap) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, TheLastBlockIsTheRemainderAndNotAWholeBlock) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, BothEdgesOfAWriteMapPublishOneStateRecord) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, AZeroBlockSizeTurnsThePushOffRatherThanMakingItUnlimited) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, ADestroyedBufferLeavesTheSet) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, UnderSplitTheWritebackClearsThePendingFlagAndNotTheRequest) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, OnlyAdoptTierTwoIsImplemented) { MGL_SPLIT_ONLY_OR_SKIP(); } + +#endif // MOBILEGL_BUILD_DISAGGREGATED + +// ===================================================================================== +// Tier 1 of the three-tier flush ladder - the INVALIDATE_RANGE edge. +// +// It is a PUSH-build case and not a split-build one: the widening hazard is real in every +// build that compiles FlushPendingRangesFrom, and under split it simply gains a second cause +// (a SEG_STAGE snapshot that no longer matches the queued range). +// ===================================================================================== + +#if MOBILEGL_PIPE_PUSH +namespace { + using MobileGL::MG_Backend::DirectGLES::BufferImpl::InvalidateFlushAccessFor; + using MobileGL::MG_Backend::DirectGLES::BufferImpl::kEsprytInvalidateRangeMinBytes; + constexpr SizeT kStore = 1024u * 1024u; +} // namespace + +TEST(EsprytFlushLadder, AWholeBufferRangeOrphansTheStore) { + EXPECT_EQ(InvalidateFlushAccessFor(0, kStore, 0, kStore, kStore, kStore), + static_cast(GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT)); +} + +TEST(EsprytFlushLadder, ALargePartialRangeInvalidatesExactlyThatRange) { + const SizeT start = 4096; + const SizeT end = start + kEsprytInvalidateRangeMinBytes; + EXPECT_EQ(InvalidateFlushAccessFor(start, end, start, end, kStore, kStore), + static_cast(GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_RANGE_BIT)); +} + +TEST(EsprytFlushLadder, ASmallPartialRangeFallsThroughToTheStagingRing) { + EXPECT_EQ(InvalidateFlushAccessFor(4096, 4096 + 64, 4096, 4096 + 64, kStore, kStore), 0u) + << "below the threshold the map WAITS out the WAR hazard on the CPU instead of " + "substituting pages, which is the whole reason tier 2 exists"; +} + +// THE EDGE THAT HAS ALREADY DRAWN BLOOD (Managers.cpp:1125-1128): widening the map past the +// queued range clobbered GPU-written data - an SSBO counter beside the app's SubData - with +// the stale shadow, SILENTLY. Under split the same shape arrives with a different cause: the +// server may hold no pointer into the client's shadow (R-11), so `hostBase` becomes a +// SEG_STAGE snapshot, and a snapshot that does not cover exactly the queued range is the same +// lie told by a thread boundary instead of by a page alignment. +TEST(EsprytFlushLadder, AMapWiderThanTheQueuedRangeRefusesTierOne) { + const SizeT queuedStart = 4096; + const SizeT queuedEnd = queuedStart + kEsprytInvalidateRangeMinBytes; + // Page-aligned outward, the exact widening the in-tree note records. + EXPECT_EQ(InvalidateFlushAccessFor(queuedStart, queuedEnd, 0, queuedEnd + 4096, kStore, kStore), 0u) + << "a widened INVALIDATE_RANGE declares bytes dead that the shadow is not about to " + "rewrite, and overwrites whatever the GPU put there"; + // And narrower, which is the same corruption read the other way round: bytes left + // unwritten inside a range that has just been declared dead. + EXPECT_EQ(InvalidateFlushAccessFor(queuedStart, queuedEnd, queuedStart, queuedEnd - 8, kStore, kStore), 0u); +} + +TEST(EsprytFlushLadder, AnEmptyRangeIsNeverTierOne) { + EXPECT_EQ(InvalidateFlushAccessFor(4096, 4096, 4096, 4096, kStore, kStore), 0u); +} +#else +// The same five names in a pull build, for the G2/G14 reason above: the ladder's push arm +// (FlushPendingRangesFrom) is the only one that carries this decision as a function - the pull +// arm's FlushPendingRangesNow is byte-frozen against 5cb826b0 (ID-15) and may not grow one. +#define MGL_PUSH_ONLY_OR_SKIP() \ + GTEST_SKIP() << "the three-tier ladder's push arm (FlushPendingRangesFrom) is what carries " \ + "InvalidateFlushAccessFor; a pull build compiles the frozen arm instead" +TEST(EsprytFlushLadder, AWholeBufferRangeOrphansTheStore) { MGL_PUSH_ONLY_OR_SKIP(); } +TEST(EsprytFlushLadder, ALargePartialRangeInvalidatesExactlyThatRange) { MGL_PUSH_ONLY_OR_SKIP(); } +TEST(EsprytFlushLadder, ASmallPartialRangeFallsThroughToTheStagingRing) { MGL_PUSH_ONLY_OR_SKIP(); } +TEST(EsprytFlushLadder, AMapWiderThanTheQueuedRangeRefusesTierOne) { MGL_PUSH_ONLY_OR_SKIP(); } +TEST(EsprytFlushLadder, AnEmptyRangeIsNeverTierOne) { MGL_PUSH_ONLY_OR_SKIP(); } +#endif // MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp b/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp index 6c8cd3c3..dbd576ee 100644 --- a/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp @@ -1260,6 +1260,18 @@ namespace { // until now by POISON being invisible in this TU at all (see the include at the top). #elif !MOBILEGL_PIPE_VERIFY GTEST_SKIP() << "Fatal{PipeLiveHostWrites} is a MOBILEGL_PIPE_VERIFY wire and is compiled out here"; + // P5 b1: AND IT IS RETIRED IN A SPLIT BUILD, because this is the phase the wire was + // waiting for. "HasLiveHostWrites is always false and is written by nobody" cannot + // survive the producer it exists to announce - MGPSubData::HasLiveHostWrites, set by + // MGPipeEmitResourceSubData - so under MOBILEGL_BUILD_DISAGGREGATED the always-false + // pin is gone and two other things carry the invariant instead: + // PinLiveHostWritesNamesABuffer (the bit is buffer-family only, and that IS still + // always true) and the production-path probe pair in MG_Test/SanityTest.cpp and + // MG_Test/Buffer/SplitBufferTest.cpp, both of which go red when the producer is + // deleted. This skip is what the build-verify-split lane exists to make visible. +#elif MOBILEGL_BUILD_DISAGGREGATED + GTEST_SKIP() << "P5 gave HasLiveHostWrites a producer, so the always-false wire is retired " + "in a split build; PinLiveHostWritesNamesABuffer replaces it"; #elif !MGTEST_HAVE_FORK GTEST_SKIP() << "no fork on this platform; the wire's verdict is std::abort()"; #else diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 484fb8cd..45393eff 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -26,6 +26,11 @@ #include #include #include +#if MOBILEGL_PIPE_PUSH +// P5 b1: MGPipeResourceTrackerInstance(), so the split probe case can ask the PRODUCTION +// tracker for a buffer's handle instead of minting one by hand. +#include +#endif #include #include #include @@ -4652,6 +4657,129 @@ TEST(DirectGLESBufferDrawProbe, ALiveHostMapKeepsTheHandleArmProbeDirtyBetweenTw record = {}; } +// P5 b1's half of the case above: THE PIN IS LIFTED, AND THIS IS WHAT REPLACED IT. +// +// The case above exists because answering the live-map question from HasLiveHostWrites alone +// read draw-CLEAN forever. P5 gives that field a producer and retires the last frontend read +// in IsBufferDrawCleanByHandle under split, because under a spawn there is no frontend object +// on that side to ask. +// +// EVERYTHING THE CASE OBSERVES IS WRITTEN BY THE PRODUCER, NOT BY THE CASE. A first cut of +// this test set `record.HasLiveHostWrites = true` by hand and therefore passed with the +// producer deleted - a test that constructs the state it is supposed to be observing cannot +// fail for the reason it exists. So the resource subsystem is armed with an empty ops table +// (MG_Test/Pipe's PushArm shape), the map and the unmap are made through the ordinary +// frontend entry points, and the record is only ever READ. Delete +// BufferObject::NotePersistentMapStateChanged's emission, or PipeFill.cpp's +// `record.HasLiveHostWrites = ...`, and this goes red. +TEST(DirectGLESBufferDrawProbe, UnderSplitTheRecordAloneAnswersTheLiveHostMapQuestion) { + using namespace MobileGL; + using namespace MobileGL::MG_Backend::DirectGLES; + using namespace MobileGL::MG_State::GLState; + + if (!EsprytSlotTablesEnabled()) { + GTEST_SKIP() << "the handle-keyed resource table only exists on the {slot, gen} arm"; + } +#if !MOBILEGL_BUILD_DISAGGREGATED + GTEST_SKIP() << "MG_Config::Transport is a constexpr Monolith without the transport built in, " + "so the split arm of this probe cannot be entered"; +#else + // The subsystem, armed the way MG_Test/Pipe arms it: an EMPTY op table is enough, because + // MGPipeResourceSubsystemEnabled() only asks whether one is registered, and every hook this + // case reaches is optional. + const Uint64 previousPush = MG_Config::Features.PipePush; + const auto previousTransport = MG_Config::Transport; + const Uint32 previousBlockKb = MG_Config::Ipc.PersistentBlockKb; + MG_Pipe::MGPipeResourceOps ops{}; + MG_Config::Features.PipePush |= MG_Pipe::kMGPipeSubsystemResources; + MG_Pipe::MGPipeSetResourceOps(&ops); + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.PersistentBlockKb = 64; + + { + // The constructor mints the handle and emits resource_create; Respecify emits the + // descriptor. Both through the production path. + auto owner = MakeShared(0u); + owner->Respecify(256, nullptr); + + const MG_Pipe::MGPipeHandle res = MG_Pipe::MGPipeResourceTrackerInstance().Find(*owner); + ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(res)); + auto& applier = MG_Pipe::MGPipeApplier(); + ASSERT_GT(applier.Resources.size(), static_cast(res.Slot)); + auto& record = applier.Resources[res.Slot]; + ASSERT_TRUE(record.Live) << "resource_create did not reach the applier"; + ASSERT_EQ(record.Desc.Width, 256u) << "resource_respecify did not reach the applier"; + EXPECT_FALSE(record.HasLiveHostWrites) << "nothing maps this buffer yet"; + + auto& twin = BufferImpl::g_backendBufferResources.GetOrCreate(res); + twin = MakeShared(); + auto* const resource = twin.get(); + resource->id = 1; // a name, never used: this probe issues no GL + resource->contextGeneration = BufferImpl::CurrentBufferContextGeneration(); + resource->storageInitialized = true; + resource->storageSize = 256; + const auto stampSynced = [&]() { + resource->syncedChangeSerial = record.Serial; + const std::lock_guard lock(resource->pendingMutex); + resource->pendingRanges.clear(); + resource->pendingResidentWrites.clear(); + }; + stampSynced(); + + // The probe is given NO frontend object at all, which is the point: this is the + // question a spawned server has to answer, and it has nothing to ask. + ASSERT_TRUE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, nullptr)) + << "the fixture is not clean before the map, so this case cannot isolate the " + "live-map question it exists for"; + + // ---- map. The RISING EDGE is what has to publish, and nothing else can. ----------- + void* const mapped = owner->AcquireMemoryRange( + Range1D{0, 256}, BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + ASSERT_NE(mapped, nullptr); + ASSERT_TRUE(owner->IsMapped()); + ASSERT_FALSE(owner->IsBackendPersistentMapped()) + << "the acquisition was minted, so R-6's decline did not happen and this is the " + "adopted arm rather than the emulated one"; + + EXPECT_TRUE(record.HasLiveHostWrites) + << "the map published nothing the server can see. Without it the probe below answers " + "CLEAN, the ensure path is skipped and then latched (DirectGLES.cpp:691/:697), " + "SyncPersistentMappedRange is never reached again, and the frame draws the last " + "uploaded bytes for ever with no diagnostic"; + + // Absorb the rising-edge record so the ONLY thing left dirty is the flag. + stampSynced(); + EXPECT_FALSE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, nullptr)) + << "a live host map read draw-CLEAN from the record alone"; + + // ---- a write with no API call announcing it, then the per-draw push -------------- + static_cast(mapped)[0] = 0x5Au; + const Uint64 serialBeforePush = record.Serial; + owner->SyncPersistentMappedRange(); + EXPECT_GT(record.Serial, serialBeforePush) + << "the block push emitted nothing, so a write made through the pointer never left " + "the client"; + + // ---- unmap. The FALLING EDGE has to put it back. --------------------------------- + owner->ReleaseMemory(false); + EXPECT_FALSE(record.HasLiveHostWrites) + << "the unmap published nothing, so the record stays dirty for the buffer's life and " + "every later draw re-uploads it"; + stampSynced(); + EXPECT_TRUE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, nullptr)) + << "the probe stayed dirty after the unmap, i.e. it is not the record that is " + "being read"; + + BufferImpl::g_backendBufferResources.ReleaseByHandle(res); + } + + MG_Config::Ipc.PersistentBlockKb = previousBlockKb; + MG_Config::Transport = previousTransport; + MG_Pipe::MGPipeSetResourceOps(nullptr); + MG_Config::Features.PipePush = previousPush; +#endif +} + // P3a REWORK M-1's gate (contract-review M2). The minting overload's symmetric `!=` is safe // because its handle comes out of the allocator and can never be behind the entry; the HANDLE // overload's input ARRIVES in a payload, so a generation BEHIND the live entry's is reachable - @@ -4782,4 +4910,8 @@ TEST(DirectGLESSlotTable, ADeathNoticeForEveryP4aKindIsIdempotent) { TEST(DirectGLESBufferDrawProbe, ALiveHostMapKeepsTheHandleArmProbeDirtyBetweenTwoDraws) { GTEST_SKIP() << "the handle-keyed resource table is compiled only under MOBILEGL_PIPE_PUSH"; } + +TEST(DirectGLESBufferDrawProbe, UnderSplitTheRecordAloneAnswersTheLiveHostMapQuestion) { + GTEST_SKIP() << "the handle-keyed resource table is compiled only under MOBILEGL_PIPE_PUSH"; +} #endif // MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_Util/Metrics/PipeStats.cpp b/MobileGL/MG_Util/Metrics/PipeStats.cpp index bfa292cd..181c0c19 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.cpp +++ b/MobileGL/MG_Util/Metrics/PipeStats.cpp @@ -30,9 +30,20 @@ // OnSubData / OnFlushMappedRange, the AcquirePersistentMap seed, the // AcquireResidentSlice initial upload and the AcquireStreamedSlice // arena fill. -// NOT covered: bytes an app writes THROUGH a persistent map. Those -// never pass through either backend (D4/D-B4) - see -// persistent-map-push. +// NOT covered IN A MONOLITH BUILD: bytes an app writes THROUGH a +// persistent map. Those never pass through either backend (D4/D-B4) - +// see persistent-map-push. +// COVERED UNDER SPLIT, AND DELIBERATELY OVERLAPPING WITH +// persistent-map-push (P5 b1, R-6). At adoption tier T2 there is no +// adoption, so a pushed block IS an ordinary resource_subdata: it +// reaches Ops_H_SubData, is queued into pendingRanges and is staged +// here like any other write. The same bytes are therefore in BOTH +// classes, on purpose - stage-buffer answers "what did the backend +// move", persistent-map-push answers "what did the client have to ship +// because the acquisition was declined", and subtracting one from the +// other would make the first under-report the thing it exists to +// measure. Read them as two questions about the same bytes, never as a +// partition, and do not add them. // stage-texture ESPRYT (Managers.cpp texture upload): the bytes of whichever of // the three upload shapes ran (rect list / union box / whole level). // MAGMA (VkTextureManager.cpp): the packed staging slice of an @@ -65,9 +76,22 @@ // payload, not resource bytes. // NOT covered: Magma builds no such array (it issues one vkCmdDraw* // per sub-draw), so this class is Espryt-only by construction. -// persistent-map-push Not wired in P0: today a persistent map is a permanent address -// space donation (D4/D-B4) that survives the whole monolith track, -// so there is no push to count until the IPC track breaks it. +// persistent-map-push WIRED IN P5 (b1), and only a split build can ever move it. The one +// site is BufferObject::PushMappedSpanBlock, i.e. the client shipping +// one MOBILEGL_IPC_PERSISTENT_BLOCK_KB block of a persistently mapped +// span because MGPipeApplyMapPersistent declined the adoption (R-6, +// tier T2). Zero in every monolith build, and that zero is CORRECT +// rather than missing: a persistent map there is a permanent address +// space donation (D4/D-B4), the application writes straight into GPU +// memory, and there is no push to count. A split run where this stays +// 0 has NOT reached T2. +// Read it against map-persistent-roundtrips (mpr), which it is +// ANTI-CORRELATED with: mpr counts acquisition ATTEMPTS - one per +// storage definition, the same number in both modes - and this counts +// the bytes the client had to ship because the attempt was declined. +// DOUBLE-COUNTED WITH stage-buffer, deliberately: see that entry +// above. The two are different questions about the same bytes under +// split, and adding them is wrong. // residual-value-block Placeholder, always 0 until P2 (plan section 6.3). // // Call classes diff --git a/docs/Disaggregated/MEASUREMENTS.md b/docs/Disaggregated/MEASUREMENTS.md index ecb578f3..a69a02cf 100644 --- a/docs/Disaggregated/MEASUREMENTS.md +++ b/docs/Disaggregated/MEASUREMENTS.md @@ -108,7 +108,7 @@ python3 tools/trace_replay/run_android_retrace_local.py \ | `PipeInputs` 字段 | 63 | 计划写 61；`GetBoundTransformFeedbackLifetimeId`、`HasOpenTransformFeedbackSpan` 是 D21 之后新增的读点 | | 后端调用的不同访问器 | 62（Espryt 32、Magma 56） | `GetBoundTransformFeedbackName` 已无人读，留作已标注的死行 | | 填充点 | 83 条 `MGP_FILL`，覆盖 69 个 verb、9 个类 | `MG_Pipe/FillPoints.def` | -| `SyncPersistentMappedRange` / `SyncGpuWrites` | 20 / 6 | 与计划一致 | +| `SyncPersistentMappedRange` / `SyncGpuWrites` | **21** / 6 | P5 b1 复核：21 = Espryt 9 + Magma 12，不是 20。原来的 20 与 `Managers.cpp:5047-5048` 的"十一处 Espryt"都恰好少一行，少的是 `DirectGLES.cpp:361`（`ResolveIndirectCommandBytes`）——它是共享 helper 而不是 draw-path 站点，多半因此被排除，但一个能读到持久映射区间的 helper 与 draw 站点一样会读到陈旧字节。`ARCHITECTURE.md:290` 引的那张 §5.7 逐站点归属表**在树里不存在**。 | ## 7. verify 通道发现的两类真问题