mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-17 00:28:31 +09:00
[Fix, Test] (MG_IntegrationTest, MG_Test): make the persistent-map membership expectation a function of the arm the map landed in, and pin the arm-independent half on IsLivePersistentMap where both answers can be produced
This commit is contained in:
@@ -43,6 +43,16 @@
|
||||
// address really is valid in this process, which is precisely why E3(d) is worth gating at all -
|
||||
// and a direct count of declines would need a counter from packages b1/v1.
|
||||
//
|
||||
// AND MEMBERSHIP FOLLOWS THE ARM (ID-42). The client's live-persistent-map set is the set of maps
|
||||
// the client still has to PUSH, so an ADOPTED store is deliberately not in it -
|
||||
// PersistentMapTracker::IsLivePersistentMap's second row is IsBackendPersistentMapped(), and an
|
||||
// adopted store's bytes are already in coherent GPU memory. AssertMembership therefore expects
|
||||
// `live == (arm == emulated)`, from the same peek AssertOrRecordArm reads. The arm-INDEPENDENT
|
||||
// claim - "the predicate reads the chain, not the adopt tier" - belongs to a unit case that can
|
||||
// drive both answers on demand (SplitBufferSet.
|
||||
// TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo) and not to an integration entry, which
|
||||
// only ever sees whichever arm its driver and transport happen to give it.
|
||||
//
|
||||
// `mpr` (map-persistent-roundtrips) is counted per ACQUISITION ATTEMPT, mint or decline
|
||||
// (ARCHITECTURE.md:492), so it is the SAME NUMBER on both arms and in both transports: that is
|
||||
// what makes exit gate E3(c)'s "mpr equal to the monolith arm's" checkable by one process. Both
|
||||
@@ -243,6 +253,11 @@ void main() { oColor = vec4(vColor, 1.0); }
|
||||
ASSERT_TRUE(PeekBufferIsAdoptedPersistentMap(vbo, &adopted))
|
||||
<< "no frontend BufferObject behind GL buffer " << vbo << " " << when;
|
||||
const char* arm = adopted ? "adopted" : "emulated";
|
||||
// REMEMBERED, because AssertMembership's expectation is a function of it (ID-42)
|
||||
// and the two have to be talking about ONE observation of ONE buffer. A second
|
||||
// peek would be a second question, and a scenario that asked it twice could
|
||||
// straddle a change and then compare two different moments.
|
||||
m_observedArm = adopted ? ObservedArm::Adopted : ObservedArm::Emulated;
|
||||
RecordProperty("persistent_map_arm", arm);
|
||||
if (declared.empty()) return;
|
||||
ASSERT_EQ(declared, std::string(arm))
|
||||
@@ -259,21 +274,63 @@ void main() { oColor = vec4(vColor, 1.0); }
|
||||
// b1-v1.md 4.1 item 2: the client's live-persistent-map set is meant to be exactly
|
||||
// SyncPersistentMappedRange's early-out chain. A drift between the two stops the push
|
||||
// silently; asking here makes it a named failure instead.
|
||||
//
|
||||
// MEMBERSHIP IS A PROPERTY OF THE ARM, NOT OF THE FLAGS - INTEGRATOR DECISION ID-42,
|
||||
// and the first cut of this function got it wrong in a way no lane could see. It
|
||||
// asserted `live` UNCONDITIONALLY, on the grounds that "a PERSISTENT|WRITE|COHERENT map
|
||||
// that is not FLUSH_EXPLICIT is a member by construction". That sentence is true only
|
||||
// on the EMULATED arm. PersistentMapTracker::IsLivePersistentMap's second row is
|
||||
// `if (buffer.IsBackendPersistentMapped()) return false;` - deliberately, because the
|
||||
// predicate must read the CHAIN and not the tier: an adopted store's bytes are already
|
||||
// in coherent GPU memory and there is nothing for the push to ship, so it is not a
|
||||
// member and must not be one. On the adopted arm `live` is false BY DESIGN.
|
||||
//
|
||||
// It survived the first wave because the two builds that ran it could not contradict
|
||||
// it: the push build compiles no MG_Remote, so MGITEST_PERSISTENT_MAP_TRACKER is
|
||||
// undefined and this function returned at the line above before asserting anything;
|
||||
// the split build's monolith lane, where the tracker IS compiled, adopts
|
||||
// (MGPipeApplyMapPersistent's R-6 decline is ANDed with `Transport != Monolith`,
|
||||
// PipeApply.cpp:2005), and seven entries went red the first time the assertion ran at
|
||||
// all.
|
||||
//
|
||||
// So the expectation is `live == (arm == emulated)`, taken from the SAME peek
|
||||
// AssertOrRecordArm read. The arm-independent half - "the predicate reads the chain,
|
||||
// not the tier" - is not an integration claim and is pinned where it can be driven
|
||||
// directly, by MG_Test/Buffer/SplitBufferTest.cpp's
|
||||
// SplitBufferSet.TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo.
|
||||
void AssertMembership(unsigned int vbo, const char* when) {
|
||||
if (!PersistentMapTrackerAvailable()) {
|
||||
RecordProperty("persistent_map_membership", "unavailable");
|
||||
return;
|
||||
}
|
||||
if (m_observedArm == ObservedArm::NotLookedAt) {
|
||||
// AssertOrRecordArm could not look, so there is no arm to condition on and
|
||||
// "could not look" is not "it was emulated" (PersistentMapPeek.h). A lane that
|
||||
// DECLARES an arm has already been skipped by AssertOrRecordArm in this case.
|
||||
RecordProperty("persistent_map_membership", "arm unknown");
|
||||
return;
|
||||
}
|
||||
const bool emulated = (m_observedArm == ObservedArm::Emulated);
|
||||
bool live = false;
|
||||
ASSERT_TRUE(PeekBufferIsLivePersistentMap(vbo, &live))
|
||||
<< "the tracker is compiled in but could not answer for GL buffer " << vbo << " " << when;
|
||||
EXPECT_TRUE(live)
|
||||
<< "a PERSISTENT|WRITE|COHERENT map that is not FLUSH_EXPLICIT is a member of "
|
||||
"the client's live-persistent-map set by construction, and it is not one "
|
||||
<< when
|
||||
RecordProperty("persistent_map_membership", live ? "member" : "not a member");
|
||||
EXPECT_EQ(live, emulated)
|
||||
<< "the client's live-persistent-map set is the set of maps the client still has "
|
||||
"to PUSH, so membership follows the arm: on the emulated arm this "
|
||||
"PERSISTENT|WRITE|COHERENT non-FLUSH_EXPLICIT map must be a member, and on "
|
||||
"the adopted arm it must not be - IsLivePersistentMap's "
|
||||
"IsBackendPersistentMapped() row takes it out, because an adopted store's "
|
||||
"bytes are already in coherent GPU memory and there is nothing to ship. This "
|
||||
"map landed in the "
|
||||
<< (emulated ? "emulated" : "adopted") << " arm " << when << " and the predicate "
|
||||
<< (live ? "made it a member" : "did not make it a member")
|
||||
<< ". The set is supposed to BE SyncPersistentMappedRange's early-out chain; if "
|
||||
"they have drifted, the push stops shipping this buffer's blocks and nothing "
|
||||
"else says so.";
|
||||
"they have drifted, the push either stops shipping this buffer's blocks or "
|
||||
"starts shipping an adopted store's, and nothing else says so. (ID-42. The "
|
||||
"arm-independent statement - that the predicate reads the chain and not the "
|
||||
"adopt tier - is pinned by SplitBufferSet."
|
||||
"TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo, not here.)";
|
||||
}
|
||||
|
||||
void TearDown() override {
|
||||
@@ -320,6 +377,13 @@ void main() { oColor = vec4(vColor, 1.0); }
|
||||
return RegionIsMostly(image, 8, image.Width() - 9, 8, image.Height() - 9, color, 0.0, when);
|
||||
}
|
||||
|
||||
// The arm this scenario OBSERVED, as opposed to the one its lane declared. A lane may
|
||||
// declare none (the monolith counting lane does not), and the peek may be unable to
|
||||
// look at all, so the third state is not "assume emulated" - it is "there is no arm to
|
||||
// condition on", and AssertMembership records rather than asserts under it.
|
||||
enum class ObservedArm { NotLookedAt, Adopted, Emulated };
|
||||
ObservedArm m_observedArm = ObservedArm::NotLookedAt;
|
||||
|
||||
unsigned int m_program = 0;
|
||||
unsigned int m_vao = 0;
|
||||
unsigned int m_vbo = 0;
|
||||
|
||||
@@ -41,6 +41,21 @@ namespace {
|
||||
using MG_Remote::Client::GpuWriteProducer;
|
||||
using MG_Remote::Client::PersistentMapTracker;
|
||||
|
||||
// A backend that MINTS a persistent mapping, for the one case that needs the adopted arm
|
||||
// (TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo). Only AcquirePersistentMap is
|
||||
// filled in: the frontend null-checks every op individually, and a table with one live
|
||||
// member is the smallest thing that makes AcquireMemoryRange's legacy adoption arm fire.
|
||||
// The storage is the CASE's, not this table's - AdoptPersistentMap keeps the pointer and
|
||||
// never owns it - so the base travels through a file-scope variable the case sets and
|
||||
// clears around the one acquisition it wants minted.
|
||||
void* g_mintedPersistentBase = nullptr;
|
||||
void* MintPersistentMap(BufferObject&) { return g_mintedPersistentBase; }
|
||||
const MG_State::GLState::BufferBackendOps g_mintingBufferOps = [] {
|
||||
MG_State::GLState::BufferBackendOps ops{};
|
||||
ops.AcquirePersistentMap = &MintPersistentMap;
|
||||
return ops;
|
||||
}();
|
||||
|
||||
// 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
|
||||
@@ -64,6 +79,11 @@ namespace {
|
||||
MG_Remote::Client::ResetProducerMarkCountsForTest();
|
||||
}
|
||||
void TearDown() override {
|
||||
// Belt and braces for the one case that installs a minting backend: a table left
|
||||
// behind would make the NEXT case's acquisition land in the adopted arm, and every
|
||||
// membership assertion in this file would then be about a different code path.
|
||||
MG_State::GLState::SetBufferBackendOps(nullptr);
|
||||
g_mintedPersistentBase = nullptr;
|
||||
PersistentMapTracker::Instance().ClearForTest();
|
||||
MG_State::pGLContext = Move(m_context);
|
||||
MG_Config::Transport = m_transport;
|
||||
@@ -258,6 +278,92 @@ TEST_F(SplitBufferSet, MembershipIsSyncPersistentMappedRangesOwnEarlyOutChain) {
|
||||
buffer->ReleaseMemory(false);
|
||||
}
|
||||
|
||||
// THE ADOPTED ARM IS NOT A MEMBER, AND IT IS THE CHAIN THAT SAYS SO - NOT THE TIER (ID-42).
|
||||
//
|
||||
// IsLivePersistentMap's second row is `if (buffer.IsBackendPersistentMapped()) return false;`,
|
||||
// and its comment promises that the row answers for ITSELF: at tier T2 the arm is unreachable
|
||||
// because MapPersistent declines, but a build that reaches T0/T1 later must get the same answer
|
||||
// out of the same row. Nothing drove that promise. The case above only ever sees the declined
|
||||
// arm, and PersistentCoherentMapScenario's membership assertion cannot pin it either - an
|
||||
// integration entry sees whichever arm its driver and transport hand it, which is exactly how
|
||||
// that assertion came to state the emulated arm's property as if it were universal and go red on
|
||||
// seven split-monolith entries the first time it ran.
|
||||
//
|
||||
// So the statement is pinned HERE, where both answers can be produced on demand. The adoption is
|
||||
// made by the PRODUCTION path - AcquireMemoryRange dispatching to the backend's
|
||||
// AcquirePersistentMap and calling PipeResource::AdoptPersistentMap - and not by the case
|
||||
// reaching into the buffer, because a hand-set flag would still be "true" with the production
|
||||
// adoption deleted (R-16). The transport is Monolith for exactly that call, because R-6's decline
|
||||
// is ANDed with `Transport != Monolith` and there is no other way to reach a mint in this build;
|
||||
// the PREDICATE is then asked with the tier back at T2/InProcess, which is the whole point: the
|
||||
// tier says "emulated, always" and the chain still says "not a member".
|
||||
TEST_F(SplitBufferSet, TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo) {
|
||||
constexpr SizeT kSize = 4096;
|
||||
// The storage the backend "mints". Declared first so it outlives the buffer: AdoptPersistentMap
|
||||
// stores the pointer and releases the shadow, and it never owns what it was handed.
|
||||
Vector<Uint8> minted(kSize, static_cast<Uint8>(0));
|
||||
g_mintedPersistentBase = minted.data();
|
||||
|
||||
auto adopted = MakeBuffer(27u, kSize);
|
||||
auto declined = MakeBuffer(28u, kSize);
|
||||
|
||||
// The declined twin first, with no backend ops at all: same flags, same size, same call.
|
||||
declined->AcquireMemoryRange(Range1D{0, kSize},
|
||||
BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent);
|
||||
ASSERT_FALSE(declined->IsBackendPersistentMapped());
|
||||
ASSERT_TRUE(PersistentMapTracker::IsLivePersistentMap(*declined))
|
||||
<< "the emulated arm IS a member - if this fails the two halves are not comparable and the "
|
||||
"assertion below proves nothing";
|
||||
|
||||
{
|
||||
MG_State::GLState::SetBufferBackendOps(&g_mintingBufferOps);
|
||||
MG_Config::Transport = MG_Config::TransportMode::Monolith;
|
||||
adopted->AcquireMemoryRange(Range1D{0, kSize},
|
||||
BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent);
|
||||
MG_Config::Transport = MG_Config::TransportMode::InProcess;
|
||||
MG_State::GLState::SetBufferBackendOps(nullptr);
|
||||
}
|
||||
ASSERT_TRUE(adopted->IsBackendPersistentMapped())
|
||||
<< "the backend declined the mint, so there is no adopted arm here to ask about";
|
||||
|
||||
// THE TIER SAYS EMULATED. The chain must still say "not a member".
|
||||
ASSERT_TRUE(MG_Remote::Client::AdoptTierIsEmulate());
|
||||
ASSERT_NE(MG_Config::Transport, MG_Config::TransportMode::Monolith);
|
||||
EXPECT_FALSE(PersistentMapTracker::IsLivePersistentMap(*adopted))
|
||||
<< "an adopted store's bytes are already in host-visible coherent GPU memory and there is "
|
||||
"nothing for the push to ship, so IsBackendPersistentMapped() takes it out of the set. "
|
||||
"This answer must come from that ROW and not from the adopt tier: the tier is T2 and the "
|
||||
"transport is InProcess right now, which is the configuration in which R-6 says every "
|
||||
"acquisition declines - and the store in front of the predicate is adopted anyway, "
|
||||
"because a later phase's T0/T1 will mint one. A predicate that read the tier would call "
|
||||
"it a member and the push would read an adopted store's Bytes() as if it were the "
|
||||
"shadow.";
|
||||
|
||||
// The SET agrees with the predicate, asked through the production entry point rather than by
|
||||
// reading a member: NoteMapStateChanged is what every one of the five maintenance events
|
||||
// calls, and it must refuse to enrol an adopted store.
|
||||
PersistentMapTracker::Instance().NoteMapStateChanged(*adopted);
|
||||
EXPECT_EQ(PersistentMapTracker::Instance().MemberCount(), 1u)
|
||||
<< "only the declined twin: enrolling an adopted store would make the push read its "
|
||||
"Bytes() - which is the GPU map, not the shadow - as if it were bytes to ship";
|
||||
|
||||
// ...and the consequence, which is the one that would actually corrupt something.
|
||||
const Uint64 before = MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush);
|
||||
MG_Remote::Client::PushPersistentMapsBeforeVerb();
|
||||
EXPECT_EQ(MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush) - before,
|
||||
static_cast<Uint64>(kSize))
|
||||
<< "the declined twin's 4096 bytes and nothing else: the adopted buffer must contribute no "
|
||||
"pushed bytes at all";
|
||||
|
||||
declined->ReleaseMemory(false);
|
||||
// The adopted one is NOT released through ReleaseMemory: ReleasePersistentMap is for a store
|
||||
// being redefined, and a persistent map the application holds outlives every unmap by
|
||||
// definition (PipeResource.h:121-127). It is dropped here with the mapping still adopted,
|
||||
// which is also the shape ~BufferObject has to survive.
|
||||
adopted.reset();
|
||||
g_mintedPersistentBase = nullptr;
|
||||
}
|
||||
|
||||
// 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
|
||||
@@ -402,7 +508,7 @@ TEST_F(SplitBufferSet, OnlyAdoptTierTwoIsImplemented) {
|
||||
|
||||
#else
|
||||
|
||||
// THE SAME FIFTEEN NAMES, SO THE ctest NAME SET DOES NOT MOVE BETWEEN LANES. G2 compares the
|
||||
// THE SAME SIXTEEN 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.
|
||||
@@ -417,6 +523,7 @@ TEST(SplitBufferSet, Row4AReadPixelsIntoAPackPboMarksThePbo) { MGL_SPLIT_ONLY_OR
|
||||
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, TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo) { 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(); }
|
||||
|
||||
Reference in New Issue
Block a user