From b1f36a71b59a44bf24bcc3b111a1bc1b1dd06245 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 15 Oct 2026 10:15:21 -0400 Subject: [PATCH] [Docs] (MG_Backend, MG_Backend/DirectGLES, MG_Impl/Pipe, MG_IntegrationTest/Scenarios, MG_State/GLState, MG_Test/Buffer, MG_Test/Util, MG_Util/Metrics): correct comments about retired switches and moved code --- MobileGL/MG_Backend/BackendObjects.h | 13 +++++------ MobileGL/MG_Backend/DirectGLES/Managers.h | 19 +++++----------- MobileGL/MG_Impl/Pipe/Verb/GpuWriteSet.h | 22 ++++++++++--------- .../Scenarios/LargeArenaAdoptionScenario.cpp | 2 +- .../ResourceSubsystemControlScenario.cpp | 4 ++-- .../Scenarios/StorageBufferRegrowScenario.cpp | 4 ++-- .../BufferState/PersistentMapTracker.h | 11 +++++----- MobileGL/MG_Test/Buffer/SplitBufferTest.cpp | 10 ++++----- MobileGL/MG_Test/Util/PipeStatsTest.cpp | 5 +++-- MobileGL/MG_Util/Metrics/PipeStats.cpp | 4 ++-- MobileGL/MG_Util/Metrics/PipeStats.h | 4 ++-- 11 files changed, 45 insertions(+), 53 deletions(-) diff --git a/MobileGL/MG_Backend/BackendObjects.h b/MobileGL/MG_Backend/BackendObjects.h index ce170bd2..76e48617 100644 --- a/MobileGL/MG_Backend/BackendObjects.h +++ b/MobileGL/MG_Backend/BackendObjects.h @@ -23,9 +23,9 @@ namespace MobileGL::MG_Backend { // // IT MUST RUN FIRST, BEFORE ANYTHING ELSE IN DestroyImpl. ARCHITECTURE.md §14's order is // publish -> the server drains and acks -> stop the apply thread (Kill, then join) -> close - // the transport -> the client drains the compile pool -> MobileGL::Destroy() -> release the - // sync/query handles; DestroyImpl IS MobileGL::Destroy, so everything before that arrow has - // to happen at its top. Two consequences are load-bearing rather than tidy: + // the transport -> the client drains the compile pool -> MobileGL::Destroy(); DestroyImpl IS + // MobileGL::Destroy, so everything before that last arrow has to happen at its top. Two + // consequences are load-bearing rather than tidy: // // * the apply thread is JOINED before PipeStats::Shutdown() dumps, so nothing is writing // a counter while the final line is produced; @@ -33,11 +33,8 @@ namespace MobileGL::MG_Backend { // BackendObject - which is NOT that global (table 3) - is destroyed on the thread that // owns the context, by ServerLoop::Stop, rather than on the app thread. // - // The sync/query registries stay where they are, drained at MobileGL/Init.cpp:62 and :67 - // BEFORE pActiveBackendObject.reset(). ARCHITECTURE.md §14 originally put them after - // MobileGL::Destroy(); the two are only reconcilable if a split sync handle is - // client-minted and needs no backend call, which is P10's. CONTRACT-P5 4 flags this as a - // KNOWN OPEN ITEM and asks v1 to record which way it went: P5 keeps today's order. + // The sync/query registries stay where they are: DestroyImpl drains them with + // DestroyAllSyncObjects and DestroyAllQueryObjects before pActiveBackendObject.reset(). void ShutdownSplitRoles(); // F1 (P7 wave 2). Init()'s SPLIT ARM, run BEFORE MG_State::Init() rather than after it. diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 0e20c7e3..ee9bfca8 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -1987,13 +1987,9 @@ namespace MobileGL::MG_Backend::DirectGLES { const SharedPtr& stateTextureObject); // ---- P4a's handle arm (D-B3). Three server-owned serials that REPLACE, on their - // own arm, the six frontend-version memos above; the legacy members stay beside - // them under MOBILEGL_PIPE_LEGACY_MEMOS because ARCHITECTURE.md §9.6 keeps the - // pre-handle arm compiled through P3a/P4a, and because clearing the family's bit - // has to run the pre-handle arm rather than a half-migrated one. - // - // Inside the guard, so the PULL build's BackendTextureObject is byte-for-byte the - // pre-P4a object and G1's admitted-resize set stays empty (D-P). + // own arm, the six frontend-version memos above; the legacy members still sit beside + // them, but the MOBILEGL_PIPE_LEGACY_MEMOS switch and the pre-handle arm it compiled + // are retired (ARCHITECTURE.md §9.6): clearing the family's bit is a named startup refusal. // // 0 is never a real serial - the applier's counters start at 1 and only ever // advance, including across a make-current (PipeApply.cpp's three-way argument) - @@ -3042,7 +3038,6 @@ namespace MobileGL::MG_Backend::DirectGLES { Uint32 m_syncedLinkVersion = ~0u; Uint32 m_syncedImageUnitVersion = ~0u; // P4a's replacement for the two above on the handle arm; see GetSyncedShaderCsoSerial. - // Push-only, so the pull build's object is byte-for-byte the pre-P4a one (D-P). Uint64 m_syncedShaderCsoSerial = 0; Uint64 m_syncedBindingsSerial = 0; // Image units addressed by the program's FORMAT-LESS image uniforms, and the digest @@ -3218,15 +3213,13 @@ namespace MobileGL::MG_Backend::DirectGLES { SamplerParameters m_cacheSamplerParameters; Uint16 m_syncedSamplerVersion = 0; // P4a (D-B3): the SamplerCso record's Serial at the last completed sync. It replaces - // m_syncedSamplerVersion, which stays beside it because the pre-handle arm compiles - // under MOBILEGL_PIPE_LEGACY_MEMOS through P3a/P4a (ARCHITECTURE.md §9.6). + // m_syncedSamplerVersion, which is still declared beside it but no longer read: the + // pre-handle arm and its MOBILEGL_PIPE_LEGACY_MEMOS switch are retired (ARCHITECTURE.md §9.6). // // The two are not interchangeable and that is the point: the frontend version is per // OBJECT, while the serial is per CONTENT-ADDRESSED CSO, and two frontend samplers // with identical parameters share one CSO and therefore one serial - so under the // handle arm the second of them costs no driver call at all. - // - // Push-only, so the pull build's object is byte-for-byte the pre-P4a one (D-P). Uint64 m_syncedSamplerSerial = 0; }; @@ -3362,8 +3355,6 @@ namespace MobileGL::MG_Backend::DirectGLES { // on an ALREADY-ATTACHED renderbuffer moved nothing the framebuffer bit could see. // The client now emits resource_respecify straight from those three mutators, the // applier bumps this serial, and one compare here sees it. - // - // Push-only, so the pull build's object is byte-for-byte the pre-P4a one (D-P). Uint64 m_syncedResourceSerial = 0; }; diff --git a/MobileGL/MG_Impl/Pipe/Verb/GpuWriteSet.h b/MobileGL/MG_Impl/Pipe/Verb/GpuWriteSet.h index 56bf9b97..6cc2b3b7 100644 --- a/MobileGL/MG_Impl/Pipe/Verb/GpuWriteSet.h +++ b/MobileGL/MG_Impl/Pipe/Verb/GpuWriteSet.h @@ -12,9 +12,11 @@ // 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::BeginXfbCaptureForDraw), 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 +// - and the six backend sites that rows 0-3 below mirror are all on the server's side of a split +// (DirectGLES.cpp's SyncBufferBindingPointsByRecord, SyncAtomicCounterBuffers and +// MarkWritableImageBufferTexturesGpuWritten; UniformManager.cpp's ResolveStorageTexelBufferDescriptor +// and ResolveStorageBufferDescriptor; WireDraw.inc's BeginWireXfbCaptureForDraw). 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. @@ -73,22 +75,22 @@ namespace MobileGL::MG_Record { // 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, + // DirectGLES.cpp's SyncBufferBindingPointsByRecord (shader-storage class) and + // UniformManager.cpp's 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 + // DirectGLES.cpp's 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 + // DirectGLES.cpp's MarkWritableImageBufferTexturesGpuWritten and + // UniformManager.cpp's 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 (BeginXfbCaptureForDraw): the capture targets, because - // "the capture is a GPU write like any shader's". + // WireDraw.inc's BeginWireXfbCaptureForDraw, which VulkanRenderer::BeginXfbCaptureForDraw + // calls: 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, diff --git a/MobileGL/MG_IntegrationTest/Scenarios/LargeArenaAdoptionScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/LargeArenaAdoptionScenario.cpp index 703f0bf5..4b510973 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/LargeArenaAdoptionScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/LargeArenaAdoptionScenario.cpp @@ -30,7 +30,7 @@ // changes what the three above assert: // // * AnAdoptionCostsExactlyOneMapPersistentRoundtrip counts the acquisition. -// ARCHITECTURE.md §12 prices the adopted store at one round trip per STORAGE +// An adopted store costs one map_persistent round trip per STORAGE // DEFINITION; `map-persistent-roundtrips` counts every map_persistent // emission, mint or decline (D-B2), so one definition plus a frame of draws // must publish exactly one. It reads the library's summary line, so it needs diff --git a/MobileGL/MG_IntegrationTest/Scenarios/ResourceSubsystemControlScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/ResourceSubsystemControlScenario.cpp index 2edd882d..31fa90eb 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/ResourceSubsystemControlScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/ResourceSubsystemControlScenario.cpp @@ -101,8 +101,8 @@ namespace MGITest { // path because the fix was not yet in this branch's history and the workload reproduced // as a hard SIGSEGV in the vertex fetch; that detour is what ID-9 supersedes. // - // The COUNT is unaffected by the change: two storage definitions either way, which is - // what ARCHITECTURE.md §12 prices. + // The COUNT is unaffected by the change: two storage definitions either way, and an + // adopted store costs one map_persistent round trip per storage definition. constexpr int kDefinitionsInTheWindow = 2; constexpr int kDrawsPerDefinition = 3; constexpr int kInset = 2; diff --git a/MobileGL/MG_IntegrationTest/Scenarios/StorageBufferRegrowScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/StorageBufferRegrowScenario.cpp index 6fcffbaf..5c56898e 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/StorageBufferRegrowScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/StorageBufferRegrowScenario.cpp @@ -20,8 +20,8 @@ // The assertion is deliberately on the WHOLE grown range, so a partial write names the byte the // stale extent stopped at. // -// P3a ADDS THE COST OF THAT REGROWTH (gate G10). ARCHITECTURE.md §12 prices a persistently mapped -// store at "one round trip per STORAGE DEFINITION, not one per store" - and, emphatically, not one +// P3a ADDS THE COST OF THAT REGROWTH (gate G10). An adopted store costs one map_persistent round trip +// per STORAGE DEFINITION (BufferObject::TryAdoptLargeStorage runs on each one) - and, emphatically, not one // per draw. `map-persistent-roundtrips` (`mpr=` in the summary line) counts every map_persistent // EMISSION, mint or decline (D-B2), so the claim is directly countable: N definitions of an // adopted store must publish exactly N, whatever the workload does between them. A regression that diff --git a/MobileGL/MG_State/GLState/BufferState/PersistentMapTracker.h b/MobileGL/MG_State/GLState/BufferState/PersistentMapTracker.h index 13ec291c..3ff5634b 100644 --- a/MobileGL/MG_State/GLState/BufferState/PersistentMapTracker.h +++ b/MobileGL/MG_State/GLState/BufferState/PersistentMapTracker.h @@ -37,11 +37,12 @@ // 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 §12 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. +// Only changed blocks ship. At each validate point a member pushes just the blocks written +// since its last push. A map the mprotect write-fault tracker holds (MOBILEGL_PIPE_PERSISTENT_MPROTECT; +// Linux and Android) has its pages protected read-only after each push, and a write fault marks +// its page dirty; any other map compares each block's hash with the one the last push saw +// (MOBILEGL_IPC_PERSISTENT_HASH_SUPPRESS; 0 restores the whole-span push). ARCHITECTURE.md §12 +// describes this dirty-page tracking and hash suppression. // // 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 diff --git a/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp b/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp index 5d716910..9ed37e90 100644 --- a/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp +++ b/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp @@ -140,7 +140,7 @@ namespace { #if MOBILEGL_BUILD_DISAGGREGATED -// Row 0 - DirectGLES.cpp:570 / UniformManager.cpp:1231. +// Row 0 - SyncBufferBindingPointsByRecord (DirectGLES.cpp) / ResolveStorageBufferDescriptor (UniformManager.cpp). TEST_F(SplitBufferSet, Row0EverySsboBindingPointIsMarkedByADraw) { auto ssbo = MakeBuffer(11u, 256); MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, 0).Bind(ssbo); @@ -153,7 +153,7 @@ TEST_F(SplitBufferSet, Row0EverySsboBindingPointIsMarkedByADraw) { "behind the shadow glMapBuffer and glGetBufferSubData read"; } -// Row 1 - DirectGLES.cpp:618. The point of a counter is that the shader increments it. +// Row 1 - SyncAtomicCounterBuffers (DirectGLES.cpp). 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); @@ -246,9 +246,9 @@ TEST_F(SplitBufferSet, Row2TheSweepIsBoundedByTheImageUnitHighWaterMark) { binding = MG_State::GLState::ImageTextureBinding{}; } -// Row 3 - VulkanRenderer::BeginXfbCaptureForDraw() in VulkanRenderer.cpp. 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. +// Row 3 - BeginWireXfbCaptureForDraw() in WireDraw.inc, which VulkanRenderer::BeginXfbCaptureForDraw() +// calls. 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); diff --git a/MobileGL/MG_Test/Util/PipeStatsTest.cpp b/MobileGL/MG_Test/Util/PipeStatsTest.cpp index 2e2deb67..f2b918a6 100644 --- a/MobileGL/MG_Test/Util/PipeStatsTest.cpp +++ b/MobileGL/MG_Test/Util/PipeStatsTest.cpp @@ -385,8 +385,9 @@ namespace { EXPECT_EQ(line.find("wrec/f=47.00"), String::npos) << line; } - // THE STAGED-BLOB DISTRIBUTION, which is the optional deliverable MEASUREMENTS.md §6.3 says - // has never existed. It shares the draw-payload histogram's edges - bucket 0 is "0 bytes", + // THE STAGED-BLOB DISTRIBUTION, the optional deliverable: before this histogram the per-blob + // byte distribution after chunking had never been measured. It shares the draw-payload + // histogram's edges - bucket 0 is "0 bytes", // bucket n>0 is [2^(n-1), 2^n) - so the two are read the same way, and it is a COUNT per // bucket rather than a byte sum: the shape is the deliverable and the bytes have their own // counter in `seg`. diff --git a/MobileGL/MG_Util/Metrics/PipeStats.cpp b/MobileGL/MG_Util/Metrics/PipeStats.cpp index 845e4af0..16dd1314 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.cpp +++ b/MobileGL/MG_Util/Metrics/PipeStats.cpp @@ -777,8 +777,8 @@ namespace MobileGL::MG_Util::PipeStats { // P6 gate 8's two per-frame numbers have their JSON homes where every other ByteClass and // CallClass already does - the `bytes` and `calls` blocks above, under their long names // (stage-segment-bytes, wire-records) - so there is nothing to add for them here. The - // staged-blob DISTRIBUTION does need one: it is the deliverable MEASUREMENTS.md §6.3 says - // has never been measured, and like the draw-payload histogram above it is a run-total + // staged-blob DISTRIBUTION does need one: it is the per-blob distribution after chunking, + // first measured through this dump, and like the draw-payload histogram above it is a run-total // distribution, which is exactly what Tracy cannot plot and this dump exists to carry. // Bucket 0 is "0 bytes" and bucket n>0 is [2^(n-1), 2^n): the same edges PayloadBucketOf // gives both histograms, so the two are read the same way. diff --git a/MobileGL/MG_Util/Metrics/PipeStats.h b/MobileGL/MG_Util/Metrics/PipeStats.h index 9a9eabfb..d9154590 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.h +++ b/MobileGL/MG_Util/Metrics/PipeStats.h @@ -421,9 +421,9 @@ namespace MobileGL::MG_Util::PipeStats { // populations: the one above is the command payload of a draw (plan section 4.5.7, sizing // SEG_CMD), this one is how big the blobs a workload stages into SEG_STAGE actually are. // - // IT EXISTS FOR CONTRACT-P6.md §9 item 8, and MEASUREMENTS.md §6.3 says in as many words why: + // IT EXISTS FOR CONTRACT-P6.md §9 item 8, for this reason: // the chunking work landed (1e7c372e buffer walks, 9469d48e texture slabs) and every - // `maxrec` number the file carries predates it, so "the per-blob byte distribution after + // `maxrec` number then on record predated it, so "the per-blob byte distribution after // chunking" had no measurement at all. A per-frame AVERAGE cannot answer it either: an // average of 8 MiB is the same number whether one blob of 8 MiB was staged or two of 4 MiB // that the budget should have cut to one. The bucket edges are PayloadBucketOf's, shared so