diff --git a/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp b/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp index a3a62267..a1370ddb 100644 --- a/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp +++ b/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp @@ -292,14 +292,47 @@ TEST_F(SplitBufferSet, TheLastBlockIsTheRemainderAndNotAWholeBlock) { buffer->ReleaseMemory(false); } -// E3(a)'s NEGATIVE CONTROL: 0 disables the push, it does not mean "one unlimited block". +// 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(); @@ -307,7 +340,13 @@ TEST_F(SplitBufferSet, AZeroBlockSizeTurnsThePushOffRatherThanMakingItUnlimited) 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"; - buffer->ReleaseMemory(false); + + 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. @@ -380,6 +419,7 @@ TEST(SplitBufferSet, TheWholeSetIsInertOnTheMonolithPath) { MGL_SPLIT_ONLY_OR_SK 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(); } diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index ca9ed87b..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 @@ -4655,15 +4660,18 @@ TEST(DirectGLESBufferDrawProbe, ALiveHostMapKeepsTheHandleArmProbeDirtyBetweenTw // 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 - MGPSubData::HasLiveHostWrites, set -// by MGPipeEmitResourceSubData from BufferObject::HasLiveHostWritesForWire - and with the -// producer in place the last frontend read in IsBufferDrawCleanByHandle retires under split, -// because under a spawn there is no frontend object on that side to ask. +// 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. // -// So the same fixture is driven twice with the transport flipped, and the second half is the -// one that would have been impossible before: the record ALONE has to be able to say both -// answers. If it could not, this case reads CLEAN in both arms and the regression the case -// above records comes back through the other door. +// 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; @@ -4676,65 +4684,99 @@ TEST(DirectGLESBufferDrawProbe, UnderSplitTheRecordAloneAnswersTheLiveHostMapQue 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 - auto owner = MakeShared(0u); - owner->Respecify(256, nullptr); - const MG_Pipe::MGPipeHandle res = - MG_Pipe::MGPipeSlots().Acquire(MG_Pipe::MGPipeKind::Buffer, owner->GetLifetimeId()); - ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(res)); - - auto& applier = MG_Pipe::MGPipeApplier(); - if (applier.Resources.size() <= static_cast(res.Slot)) { - applier.Resources.resize(static_cast(res.Slot) + 1); - } - auto& record = applier.Resources[res.Slot]; - record = {}; - record.Gen = res.Gen; - record.Live = true; - record.Desc.Width = 256; - record.Serial = 7; - record.HasLiveHostWrites = false; - - auto& twin = BufferImpl::g_backendBufferResources.GetOrCreate(res); - twin = MakeShared(); - auto* const resource = twin.get(); - resource->id = 1; - resource->contextGeneration = BufferImpl::CurrentBufferContextGeneration(); - resource->storageInitialized = true; - resource->storageSize = 256; - resource->syncedChangeSerial = record.Serial; - + // 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 map is live and the object still says so - but the probe may no longer ask it. - void* const mapped = owner->AcquireMemoryRange( - Range1D{0, 256}, BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); - ASSERT_NE(mapped, nullptr); - ASSERT_TRUE(owner->IsMapped()); - // AcquireMemoryRange's NotePersistentMapStateChanged may have emitted the falling/rising - // edge record; re-stamp the fixture so the only question left is the flag. - record.Serial = 9; - resource->syncedChangeSerial = record.Serial; { - const std::lock_guard lock(resource->pendingMutex); - resource->pendingRanges.clear(); + // 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); } - record.HasLiveHostWrites = false; - EXPECT_TRUE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, owner.get())) - << "the frontend IsMapped() read did NOT retire under split - it is still what answers, " - "and a spawned server has no object to ask"; - - record.HasLiveHostWrites = true; - EXPECT_FALSE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, owner.get())) - << "the record alone cannot say 'a host writer is live', so the question has no answer " - "on the server's side of a split at all"; - + MG_Config::Ipc.PersistentBlockKb = previousBlockKb; MG_Config::Transport = previousTransport; - owner->ReleaseMemory(false); - BufferImpl::g_backendBufferResources.ReleaseByHandle(res); - MG_Pipe::MGPipeSlots().Free(MG_Pipe::MGPipeKind::Buffer, res); - record = {}; + MG_Pipe::MGPipeSetResourceOps(nullptr); + MG_Config::Features.PipePush = previousPush; #endif }