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

[Refactor] (Magma): key the vertex-input cache and the VAO draw memo on {slot, gen} instead of a lifetime id and a heap address

Browse Source 
- Track H subsystem 4 (P2 brief D12.4, ARCHITECTURE.md 9.5), behind
  kMGPipeSubsystemMagmaVertexInput.
- VertexInputStateFactory::ComputeHash's buffer identity component becomes the buffer's
  {slot, gen} - "lifetimeId -> gen mixed into every server-side content hash". Both are
  equally ABA-proof (the allocator maps one onto the other and bumps Gen only on slot
  REUSE); what changes is that the hash now carries the identity the SERVER will be handed
  once buffers travel as handles, instead of a number only the client can mint.
- LookupVaoDrawMemo becomes a direct slot index: the slot IS the index, and the whole
  validation is one handle compare. Gone with the re-key are the Fibonacci mix of the VAO's
  address, the two-way probe, the frame-serial eviction choice and the (pointer, lifetime
  id) pair - slots are dense by construction, so consecutive VAOs land in consecutive
  entries and the collision the address hash existed to spread does not arise below the
  table size.
- The table stays FIXED at 2048 entries and the slot index wraps, where the brief calls
  for a grow-on-demand vector. Reason, and it is a tree fact the brief does not carry:
  nothing in P2 frees a VertexElementsCso slot. The frontend death notification is Espryt
  0b's e2 and it covers Espryt's six kinds; buffers are the only kind with an OnDestroy
  hook today. A grow-on-demand table would therefore hold one ~1 KB VaoDrawMemo per VAO
  EVER created, which on a chunk-cycling Minecraft frame is tens of megabytes. Above the
  table size this degrades to a direct-mapped cache validated by the full {slot, gen}:
  never wrong, only colder, and strictly better than the address hash it replaces.
  Revisit when object deletion reaches the client allocator.
- SetupDrawSnapshot's VAO identity collapses to the same handle - one compare instead of
  (address, lifetime id) - so the snapshot and the draw memo cannot disagree about whether
  the VAO moved. The config version stays: it answers a different question.
- Handle acquisition sits behind a one-entry memo in the renderer. Acquiring is a hash
  probe into the allocator's lifetimeId -> slot map and LookupVaoDrawMemo runs per draw, so
  without it the arm would have swapped the address hash it deletes for another probe; a
  run of draws over one VAO now pays a single Uint64 compare. Magma acquires the handles
  itself because the tracker does not emit object-class state in P2 (it emits for dirty
  bits 0-4 only); when it does, these become reads of what the client already sent.
- Negative control C (MOBILEGL_PIPE_HANDLE_ABA_CONTROL, brief D18) is implemented here
  because the two guards it defeats live here: it makes ComputeHash hash the raw
  BufferObject* and makes LookupVaoDrawMemo skip the lifetime-id compare - the exact state
  the table was in before the ABA fix. It applies to the PRE-HANDLE arm, which is what
  HandleRecycleScenario.AbaControl runs (MOBILEGL_PIPE_PUSH=0), and it is what proves that
  scenario's reproducer still reproduces instead of passing for the wrong reason.
- Verification on this tree: ctest -L integration-gpu -R DirectVulkan is 432/432 under the
  default bitmask and 432/432 under MOBILEGL_PIPE_PUSH=0, and -L unit is green. Pull build
  symbol_report --threshold 0: 0 added / 0 removed / 0 renamed, 4 resized, all four the
  contract commit's.
This commit is contained in:
swung0x48
2026-09-07 23:18:09 -04:00
parent c74c4819fb
commit 43f8b47088
4 changed files with 153 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaPipeArms.h
+18 -2
View File
@@ -11,8 +11,9 @@
#include <Config.h>
#if MOBILEGL_PIPE_PUSH
// kMGPipeSubsystem* - the runtime bitmask's named bits. Push-only, so the pull build's
// include graph is unchanged.
// kMGPipeSubsystem* - the runtime bitmask's named bits - and the client slot allocator that
// mints every MGPipeHandle. Push-only, so the pull build's include graph is unchanged.
#include <MG_Impl/Pipe/SlotAllocator.h>
#include <MG_Pipe/MGPipe.h>
#endif
@@ -59,5 +60,20 @@ namespace MobileGL::MG_Backend::DirectVulkan {
#endif
std::abort();
}
// The {slot, gen} of a frontend object, minted on first sight and stable for that
// object's whole life (ARCHITECTURE.md 4.2). `lifetimeId` is the client's own identity
// for the object - never a GL name, never a heap address - so a deleted-and-recreated
// object at the same address cannot reproduce a handle, which is precisely the ABA
// HandleRecycleScenario reproduces.
//
// A VAO is kind VertexElementsCso: that is the gallium-shaped CSO a vertex array
// resolves to, and it is the only kind in MGPipeKind that names vertex-input state.
// Magma acquires the handle itself in P2 because the tracker does not emit object-class
// state yet (P2 emits for dirty bits 0-4 only); when it does, this becomes a read of what
// the client already sent.
inline MG_Pipe::MGPipeHandle MagmaPipeHandleOf(MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) {
return MG_Pipe::MGPipeSlots().Acquire(kind, lifetimeId);
}
#endif // MOBILEGL_PIPE_PUSH
} // namespace MobileGL::MG_Backend::DirectVulkan
MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp
+26 -1
View File
@@ -7,6 +7,7 @@
// End of Source File Header
#include "VertexInputStateFactory.h"
#include "MagmaPipeArms.h"
#include "MG_Util/Converters/MGToStr/DataTypeConverter.h"
#include <MG_Backend/BackendObjects.h>
#include <utility>
@@ -45,7 +46,31 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// capture came back holding a dead VAO's vertex data (0,0,0,1 - the previous
// test's positions) instead of its own.
// Zero for client memory (no buffer), which is a distinct identity of its own.
const Uint64 bufferKey = attr.Buffer ? attr.Buffer->GetLifetimeId() : 0;
//
// P2 D12.4 / ARCHITECTURE.md 9.5: under the handle arm the identity is the
// buffer's {slot, gen} rather than its lifetime id - "lifetimeId -> gen mixed
// into every server-side content hash". The two are equally ABA-proof (the
// allocator maps one onto the other and bumps Gen only on slot REUSE); what
// changes is that the key is now the identity the SERVER will be handed once
// buffers travel as handles, instead of a number only the client can mint.
Uint64 bufferKey = attr.Buffer ? attr.Buffer->GetLifetimeId() : 0;
#if MOBILEGL_PIPE_PUSH
if (attr.Buffer) {
if (MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput)) {
const MG_Pipe::MGPipeHandle handle =
MagmaPipeHandleOf(MG_Pipe::MGPipeKind::Buffer, attr.Buffer->GetLifetimeId());
bufferKey = static_cast<Uint64>(handle.Slot) | (static_cast<Uint64>(handle.Gen) << 32);
} else if (MG_Config::Features.PipeHandleAbaControl) {
// Negative control C (P2 brief D18), and it applies to the PRE-HANDLE arm
// on purpose: hash the raw BufferObject* the way this did before the
// lifetime-id fix, so HandleRecycleScenario.AbaControl can reproduce the
// ABA and assert the WRONG pixels. That arm is what proves the reproducer
// still reproduces; if the allocator stops handing the address back, it
// fails instead of passing for the wrong reason.
bufferKey = static_cast<Uint64>(reinterpret_cast<SizeT>(attr.Buffer.get()));
}
}
#endif
XXHASH_VERIFY(XXH64_update(m_hashState, &bufferKey, sizeof(bufferKey)));
}
MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp
+69 -2
View File
@@ -3625,6 +3625,43 @@ void main() {
if (m_vaoDrawMemoTable.empty()) {
m_vaoDrawMemoTable.resize(kVaoDrawMemoSlotCount);
}
#if MOBILEGL_PIPE_PUSH
// ---- P2 D12.4, the handle arm ----
//
// The slot IS the index. No Fibonacci mix of an address, no two-way probe, no
// frame-serial recycling choice: slots are dense by construction (the allocator has a
// free list plus a high-water mark), so consecutive VAOs land in consecutive entries
// and the collision the address hash existed to spread does not arise below the table
// size. The whole validation is one handle compare, and a handle cannot alias - Gen
// moves on slot REUSE, so a deleted VAO's successor never matches its predecessor's
// entry even at the same address and with a byte-identical configuration.
if (MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput)) {
const MG_Pipe::MGPipeHandle handle = ResolveVaoHandle(*vao);
// Fixed table, so the index wraps rather than growing: nothing in P2 frees a
// VertexElementsCso slot yet (the frontend death notification is Espryt 0b's e2,
// and buffers are the only kind with one today), so a grow-on-demand vector would
// hold one ~1 KB VaoDrawMemo per VAO EVER created. Above the table size this
// degrades to a direct-mapped cache validated by the full {slot, gen}, which is
// strictly better than the address hash it replaces - never wrong, only colder.
const Uint32 index = handle.Slot & (kVaoDrawMemoSlotCount - 1);
VaoDrawMemo& entry = m_vaoDrawMemoTable[index];
if (entry.vaoHandle == handle) {
return &entry;
}
entry.vaoHandle = handle;
entry.vaoKey = vao;
entry.vaoLifetimeId = vao->GetLifetimeId();
entry.contentHash = 0;
entry.layoutFactsValid = false;
// Unmatchable until a resolve completes (same rule as the legacy arm: a bailed-out
// resolve must never leave stale contents matchable).
entry.bindings.frameSerial = 0;
entry.bindings.indexFrameSerial = 0;
entry.bindings.indexBuffer = nullptr;
return &entry;
}
MagmaPipeRequireLegacyArm("LookupVaoDrawMemo");
#endif
// Multiplicative mix of the (16-byte-aligned) address; take high bits, they
// carry the most entropy of a multiply.
const Uint64 mixed = static_cast<Uint64>(reinterpret_cast<SizeT>(vao) >> 4) * 0x9E3779B97F4A7C15ull;
@@ -3634,12 +3671,22 @@ void main() {
// its own is recycled, and a slot matched on a recycled address hands the new VAO
// the dead one's resolved bindings.
const Uint64 lifetimeId = vao->GetLifetimeId();
#if MOBILEGL_PIPE_PUSH
// Negative control C (P2 brief D18): with MOBILEGL_PIPE_HANDLE_ABA_CONTROL=1 the
// lifetime-id half of the compare is defeated, leaving the recycled address as the
// whole key - exactly the state this table was in before the ABA fix. That is what
// lets HandleRecycleScenario.AbaControl assert the WRONG pixels and so prove that its
// reproducer still reproduces.
const Bool compareLifetimeId = !MG_Config::Features.PipeHandleAbaControl;
#else
constexpr Bool compareLifetimeId = true;
#endif
VaoDrawMemo& first = m_vaoDrawMemoTable[index];
if (first.vaoKey == vao && first.vaoLifetimeId == lifetimeId) {
if (first.vaoKey == vao && (!compareLifetimeId || first.vaoLifetimeId == lifetimeId)) {
return &first;
}
VaoDrawMemo& second = m_vaoDrawMemoTable[index ^ 1u];
if (second.vaoKey == vao && second.vaoLifetimeId == lifetimeId) {
if (second.vaoKey == vao && (!compareLifetimeId || second.vaoLifetimeId == lifetimeId)) {
return &second;
}
// Miss: recycle a slot. Prefer an empty one; otherwise evict the entry whose
@@ -6234,9 +6281,23 @@ void main() {
// draw of a VAO-cycling stream (Minecraft chunk rendering) through the full
// path, re-resolving descriptors and texture layouts nothing invalidated.
const auto& vao = *MGB_CTX->GetBoundVertexArray();
#if MOBILEGL_PIPE_PUSH
// P2 D12.4: the handle replaces the (address, lifetime id) pair here too - one
// compare instead of two, and the same identity the VAO draw memo is keyed on, so
// the two cannot disagree about whether "the VAO moved". The config version stays:
// it answers a different question (did this same object's layout change).
const Bool vaoMoved =
MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput)
? (!(ResolveVaoHandle(vao) == snap.vaoHandle) ||
vao.GetConfigVersion() != snap.vaoConfigVersion)
: (static_cast<const void*>(&vao) != snap.vao ||
vao.GetLifetimeId() != snap.vaoLifetimeId ||
vao.GetConfigVersion() != snap.vaoConfigVersion);
#else
const Bool vaoMoved =
static_cast<const void*>(&vao) != snap.vao || vao.GetLifetimeId() != snap.vaoLifetimeId ||
vao.GetConfigVersion() != snap.vaoConfigVersion;
#endif
const auto& drawFbo =
MGB_CTX->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject();
if (static_cast<const void*>(drawFbo.get()) != snap.drawFbo ||
@@ -6538,6 +6599,9 @@ void main() {
snap.bindGeneration = bindGeneration;
snap.vao = static_cast<const void*>(&vao);
snap.vaoLifetimeId = vao.GetLifetimeId();
#if MOBILEGL_PIPE_PUSH
snap.vaoHandle = ResolveVaoHandle(vao);
#endif
snap.vaoConfigVersion = vao.GetConfigVersion();
snap.vaoLayoutHash = vaoLayoutHash;
snap.pipeline = pipeline;
@@ -7119,6 +7183,9 @@ void main() {
snap.programVersion = program.GetBackendStateVersion();
snap.vao = &vao;
snap.vaoLifetimeId = vao.GetLifetimeId();
#if MOBILEGL_PIPE_PUSH
snap.vaoHandle = ResolveVaoHandle(vao);
#endif
snap.vaoConfigVersion = vao.GetConfigVersion();
snap.drawFbo = drawFbo.get();
snap.drawFboLifetimeId = drawFbo->GetLifetimeId();
MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h
+40
View File
@@ -1045,6 +1045,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// common shape), and "the VAO did not move" would then skip the layout
// re-resolve for a different VAO.
Uint64 vaoLifetimeId = 0;
#if MOBILEGL_PIPE_PUSH
// P2 D12.4: the handle arm's answer to the same question, and one compare rather
// than the pair above. Kept BESIDE them rather than replacing them because the
// pre-handle arm is still compiled (MOBILEGL_PIPE_LEGACY_MEMOS) and this snapshot
// is a value struct, not a wire type.
MG_Pipe::MGPipeHandle vaoHandle = MG_Pipe::kMGPipeNullHandle;
#endif
Uint32 vaoConfigVersion = 0;
const void* drawFbo = nullptr;
// Never-reused lifetime id beside the raw pointer + Uint16 version: a
@@ -1319,6 +1326,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// - bindings revalidates per draw exactly as before (frame serial, content
// hash, per-binding live buffer pointers and slice epochs).
struct alignas(64) VaoDrawMemo {
#if MOBILEGL_PIPE_PUSH
// P2 D12.4: the handle arm's key, and the ONLY key it needs. {slot, gen} is an
// identity, so the pointer-plus-lifetime-id pair below stops being a key here;
// the slot also picks the table entry, so the address hash and the two-way probe
// go with it. Null in an entry that has never been claimed.
MG_Pipe::MGPipeHandle vaoHandle = MG_Pipe::kMGPipeNullHandle;
#endif
const MG_State::GLState::VertexArrayObject* vaoKey = nullptr;
// The VAO's never-reused lifetime id, checked alongside vaoKey. The pointer
// ALONE is not an identity: a deleted VAO's heap address is handed straight
@@ -1347,6 +1361,32 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// (m_currentDrawResolvedEntry) relies on.
static constexpr Uint32 kVaoDrawMemoSlotCount = 2048; // power of two
Vector<VaoDrawMemo> m_vaoDrawMemoTable;
#if MOBILEGL_PIPE_PUSH
// One-entry memo in front of the slot allocator's lifetimeId -> handle map (P2
// D12.4). Acquiring a handle is a hash probe, and LookupVaoDrawMemo runs per draw, so
// the arm would otherwise have swapped one probe (the address hash it deletes) for
// another. A run of draws over one VAO - the common intra-batch shape - pays a single
// Uint64 compare instead.
//
// A lifetime id is never reused, so a hit can only ever be this same object; the
// valid flag exists rather than a zero sentinel because nothing promises the frontend
// counter starts above zero.
Uint64 m_lastVaoHandleLifetimeId = 0;
MG_Pipe::MGPipeHandle m_lastVaoHandle = MG_Pipe::kMGPipeNullHandle;
Bool m_lastVaoHandleValid = false;
MG_Pipe::MGPipeHandle ResolveVaoHandle(const MG_State::GLState::VertexArrayObject& vao) {
const Uint64 lifetimeId = vao.GetLifetimeId();
if (m_lastVaoHandleValid && m_lastVaoHandleLifetimeId == lifetimeId) {
return m_lastVaoHandle;
}
const MG_Pipe::MGPipeHandle handle =
MagmaPipeHandleOf(MG_Pipe::MGPipeKind::VertexElementsCso, lifetimeId);
m_lastVaoHandleLifetimeId = lifetimeId;
m_lastVaoHandle = handle;
m_lastVaoHandleValid = true;
return handle;
}
#endif
// Finds the slot holding `vao`, or recycles the older of its two candidate
// slots into an empty memo keyed on `vao`. Never returns null.
VaoDrawMemo* LookupVaoDrawMemo(const MG_State::GLState::VertexArrayObject* vao);