swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-08 20:28:32 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (Espryt): pick the unit-bindings debounce by the runtime arm, and stop two slot cases sharing one kind

Browse Source 
- UnitBindingsSnapshot was split by #if MOBILEGL_PIPE_PUSH, so a push build ran P2's
  lifetime-id debounce on the MOBILEGL_PIPE_PUSH=0 arm too. That arm has to reproduce P1
  (ConfigLoader.cpp), or the integrator's A/B measures this slice's mechanism on both sides
  and attributes it to neither - the same complaint g_fbSlotCache was already fixed for. The
  snapshot now carries P1's WeakPtr fields beside the lifetime ids whenever the legacy arm
  is compiled, and Capture/Unchanged pick by EsprytSlotTablesEnabled(). The two answers are
  equivalent (OwnerEquals on two empty pointers is true and LifetimeIdOf(nullptr) == 0 == 0;
  a live-versus-expired control block and two distinct lifetime ids both compare unequal),
  so this is A/B fidelity, not a behaviour change, and a build with no legacy arm carries
  neither the fields nor the branch.
- TwoTablesOfTheSameKindShareOneSlotAndKeepTheirOwnTwin deliberately never swept, so it left
  a live MGPipeKind::Query slot behind for good, and ObjectChurnAloneDrivesTheSweep reads
  HighWater/LiveCount of that same process-global kind. Deltas made them pass today, but
  --gtest_shuffle or a third case on kind Query would have made them interact. The
  two-holder case now has a kind to itself and returns its slot at the end.
- Its comment claimed two live tables of one kind "cannot arise outside this case". They
  can and do: ScopedDirectGLESTextureBindings holds a second live table of kind Texture, and
  package D's subsystem 4 re-keys VaoDrawMemo out of the same per-kind allocator. The
  comment now states the real hazard (whichever holder frees first orphans the other's
  entry; safe, because Free is generation-guarded and FindByHandle compares Gen, but not
  free) and flags it for the integrator.
This commit is contained in:
swung0x48
2026-09-07 23:18:09 -04:00
parent eb81705130
commit caa0a7221b
2 changed files with 92 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp
+57 -4
View File
@@ -1451,9 +1451,25 @@ namespace MobileGL::MG_Backend::DirectGLES {
// start at 1). This is not the twin table's {slot, gen}: a bound texture that has never
// been synced has no twin and therefore no handle, so a handle-keyed snapshot would read
// two never-synced textures as equal. The identity has to exist before the twin does.
//
// Split by the RUNTIME arm, not by the build, for exactly the reason g_fbSlotCache is:
// MOBILEGL_PIPE_PUSH=0 has to reproduce P1's behaviour (ConfigLoader.cpp), and a
// legacy-arm run that debounced on lifetime ids would be running P2's mechanism while
// the A/B attributed the result to P1. The two answers are equivalent - OwnerEquals on
// two empty pointers is true and LifetimeIdOf(nullptr) == 0 == 0; a live-versus-expired
// control block and two distinct lifetime ids both compare unequal - so keeping the
// legacy fields costs that arm nothing but the words, and gives the control back its
// fidelity. A build with no legacy arm compiled carries neither the fields nor the
// branch.
struct UnitBindingsSnapshot {
Array<Uint64, (SizeT)TextureTarget::TextureTargetCount> slotObjects{};
Uint64 samplerObject = 0;
#if MOBILEGL_PIPE_LEGACY_MEMOS
// P1's identity, kept verbatim for the legacy arm only.
Array<WeakPtr<MG_State::GLState::ITextureObject>, (SizeT)TextureTarget::TextureTargetCount>
legacySlotObjects{};
WeakPtr<MG_State::GLState::SamplerObject> legacySamplerObject{};
#endif
};
static Uint64 LifetimeIdOf(const SharedPtr<MG_State::GLState::ITextureObject>& object) {
@@ -1464,32 +1480,69 @@ namespace MobileGL::MG_Backend::DirectGLES {
return object ? object->GetLifetimeId() : 0;
}
#if MOBILEGL_PIPE_LEGACY_MEMOS
#define MGB_UNIT_BINDINGS_HANDLE_ARM (EsprytSlotTablesEnabled())
#else
#define MGB_UNIT_BINDINGS_HANDLE_ARM (true)
#endif
static void CaptureUnitBindings(Int maxTouchedUnit, Vector<UnitBindingsSnapshot>& out) {
const Bool handleArm = MGB_UNIT_BINDINGS_HANDLE_ARM;
out.resize(static_cast<SizeT>(maxTouchedUnit + 1));
for (Int unit = 0; unit <= maxTouchedUnit; ++unit) {
auto& textureUnit = MGB_CTX->GetTextureUnitObject(unit);
auto& snapshot = out[static_cast<SizeT>(unit)];
const auto& slots = textureUnit.GetAllBindingSlots();
for (SizeT i = 0; i < slots.size(); ++i) {
snapshot.slotObjects[i] = LifetimeIdOf(slots[i].GetBoundObject());
if (handleArm) {
snapshot.slotObjects[i] = LifetimeIdOf(slots[i].GetBoundObject());
}
#if MOBILEGL_PIPE_LEGACY_MEMOS
else {
snapshot.legacySlotObjects[i] = slots[i].GetBoundObject();
}
#endif
}
snapshot.samplerObject = LifetimeIdOf(textureUnit.GetSamplerObject());
if (handleArm) {
snapshot.samplerObject = LifetimeIdOf(textureUnit.GetSamplerObject());
}
#if MOBILEGL_PIPE_LEGACY_MEMOS
else {
snapshot.legacySamplerObject = textureUnit.GetSamplerObject();
}
#endif
}
}
static Bool UnitBindingsUnchanged(Int maxTouchedUnit, const Vector<UnitBindingsSnapshot>& snapshots) {
if (snapshots.size() != static_cast<SizeT>(maxTouchedUnit + 1)) return false;
const Bool handleArm = MGB_UNIT_BINDINGS_HANDLE_ARM;
for (Int unit = 0; unit <= maxTouchedUnit; ++unit) {
auto& textureUnit = MGB_CTX->GetTextureUnitObject(unit);
const auto& snapshot = snapshots[static_cast<SizeT>(unit)];
const auto& slots = textureUnit.GetAllBindingSlots();
for (SizeT i = 0; i < slots.size(); ++i) {
if (snapshot.slotObjects[i] != LifetimeIdOf(slots[i].GetBoundObject())) return false;
if (handleArm) {
if (snapshot.slotObjects[i] != LifetimeIdOf(slots[i].GetBoundObject())) return false;
}
#if MOBILEGL_PIPE_LEGACY_MEMOS
else if (!OwnerEquals(snapshot.legacySlotObjects[i], slots[i].GetBoundObject())) {
return false;
}
#endif
}
if (snapshot.samplerObject != LifetimeIdOf(textureUnit.GetSamplerObject())) return false;
if (handleArm) {
if (snapshot.samplerObject != LifetimeIdOf(textureUnit.GetSamplerObject())) return false;
}
#if MOBILEGL_PIPE_LEGACY_MEMOS
else if (!OwnerEquals(snapshot.legacySamplerObject, textureUnit.GetSamplerObject())) {
return false;
}
#endif
}
return true;
}
#undef MGB_UNIT_BINDINGS_HANDLE_ARM
#else
struct UnitBindingsSnapshot {
Array<WeakPtr<MG_State::GLState::ITextureObject>, (SizeT)TextureTarget::TextureTargetCount>
MobileGL/MG_Test/SanityTest.cpp
+35 -9
View File
@@ -3199,10 +3199,18 @@ namespace {
int marker = 0;
};
// Kind Query is unused by every shipping path, so these cases cannot disturb the slot space
// any real twin table allocates out of.
// Kinds Query and Fence are unused by every shipping path, so these cases cannot disturb
// the slot space any real twin table allocates out of.
//
// TWO of them, because MGPipeSlots() is a process-global singleton and three of the cases
// below read its per-kind LiveCount / HighWater. The two-holder case is the one that can
// perturb another, so it gets a kind of its own rather than a promise about gtest's
// registration order: --gtest_shuffle, --gtest_filter and a future case are all free to
// reorder them, and a shared kind would make that a flake.
using FakeSlotTable = MobileGL::MG_Backend::DirectGLES::
BackendSlotTable<FakeStateObject, FakeBackendObject, MobileGL::MG_Pipe::MGPipeKind::Query>;
using FakeSharedKindSlotTable = MobileGL::MG_Backend::DirectGLES::
BackendSlotTable<FakeStateObject, FakeBackendObject, MobileGL::MG_Pipe::MGPipeKind::Fence>;
} // namespace
// The property the whole slice exists for. The pre-P2 registry keyed twins on the frontend heap
@@ -3320,18 +3328,19 @@ TEST(DirectGLESSlotTable, AWholeTableSavesResetsAndRestores) {
TEST(DirectGLESSlotTable, TwoTablesOfTheSameKindShareOneSlotAndKeepTheirOwnTwin) {
using namespace MobileGL;
constexpr MG_Pipe::MGPipeKind kKind = MG_Pipe::MGPipeKind::Fence;
auto& slots = MG_Pipe::MGPipeSlots();
const Uint32 liveBefore = slots.LiveCount(MG_Pipe::MGPipeKind::Query);
const Uint32 liveBefore = slots.LiveCount(kKind);
FakeSlotTable a;
FakeSlotTable b;
FakeSharedKindSlotTable a;
FakeSharedKindSlotTable b;
auto object = MakeShared<FakeStateObject>(0xE1u);
a.GetOrCreate(object) = MakeShared<FakeBackendObject>();
(*a.Find(object.get()))->marker = 1;
b.GetOrCreate(object) = MakeShared<FakeBackendObject>();
(*b.Find(object.get()))->marker = 2;
EXPECT_EQ(slots.LiveCount(MG_Pipe::MGPipeKind::Query), liveBefore + 1u)
EXPECT_EQ(slots.LiveCount(kKind), liveBefore + 1u)
<< "the two tables minted a slot each; Magma's table would then resolve a different "
"handle for the same object than Espryt's";
@@ -3343,9 +3352,26 @@ TEST(DirectGLESSlotTable, TwoTablesOfTheSameKindShareOneSlotAndKeepTheirOwnTwin)
ASSERT_NE(b.FindByHandle(handle), nullptr);
EXPECT_EQ((*a.FindByHandle(handle))->marker, 1);
EXPECT_EQ((*b.FindByHandle(handle))->marker, 2);
// Deliberately no sweep: two tables of ONE kind both hold this slot, and whichever swept
// first would return it to the allocator while the other still names it. The six shipping
// registries are one table per kind, so that cannot arise outside this case.
// TWO HOLDERS OF ONE SLOT is a real configuration, not a test artefact, and this is where
// the sharp edge is: whichever holder sweeps (or is told of the death) first returns the
// slot to the allocator while the other still names it. The allocator makes that SAFE -
// Free is generation-guarded and idempotent, and FindByHandle's Gen compare turns the
// other holder's now-stale handle into a miss - but not free: if the object is still alive
// the next resolution re-Acquires it onto a NEW slot, orphaning the first table's entry
// until its own sweep. Two such configurations exist or are landing: the
// ScopedDirectGLESTextureBindings fixture above holds a second live table of kind Texture
// for the length of a test, and package D's subsystem 4 re-keys VaoDrawMemo out of this
// same per-kind allocator. Flagged for the integrator rather than defended here, because
// the fix (a refcounted slot-ownership token) belongs with whoever owns both holders.
//
// The cleanup below is what keeps that out of the OTHER cases: object first, then both
// tables, so the slot is back on the free list and this kind's LiveCount is where it was.
object.reset();
a.CollectGarbageNow();
b.CollectGarbageNow();
EXPECT_EQ(slots.LiveCount(kKind), liveBefore)
<< "the shared slot outlived both holders and the object";
}
// The sweep has TWO drivers and the table must carry both. Nothing below calls