swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch p5e/flip into feat/disaggregated (P5e: run-ahead armed)

Browse Source
This commit is contained in:
swung0x48
2026-09-18 14:14:11 -04:00
parent 72bc4268ff efe58eac14
commit 25fba0d5ef
11 changed files with 461 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/test.yml
+60
View File
@@ -1380,6 +1380,66 @@ jobs:
fi
echo "integration-magma-split is red by design on ${marker}"
# ---- P5e (ra2), ID-134 / ID-82: THE CLIENT-ARRAY LANE, EXPECTED RED UNDER RUN-AHEAD ----
#
# The two DirectGLES.Split.ClientVertexArrayScenario entries draw from a vertex array in
# the application's OWN memory. The server uploads those bytes by dereferencing a raw
# client pointer per draw, which is exactly the read rule F forbids once the client stops
# waiting - so ID-82 makes the draw a NAMED REFUSAL under run-ahead and gives the staging
# form to P8. Aborting is therefore the DESIGNED behaviour, and a lane that demanded these
# entries green would be demanding that the refusal not fire.
#
# THE LABEL, AND WHY IT IS NOT `integration-split-clientarrays` (ID-131): `ctest -L` is a
# REGEX. That spelling is matched by `-L integration-split` as a substring, so the entries
# would still be in the lane they were moved out of while every listing showed them
# renamed. Counted, not assumed: `-L integration-split` went 181 -> 179 and
# `-L integration-gpu` 1359 -> 1357 when this label landed.
#
# IT BRANCHES ON WHETHER RUN-AHEAD IS ACTUALLY ARMED, and that is not hedging - it is the
# only formulation that is a real assertion on BOTH heads. kMGPipeP5eRunAheadReady is a
# BUILD constant (MG_Backend/Init.cpp): on a head where it is false no environment can arm
# run-ahead, the refusal is unreachable, and these entries are green - so a step that
# simply demanded red would fail for the one reason that is not a defect. The client says
# which world it is in, in its own log line, and the step reads that and then demands the
# matching outcome. When the flip lands for good, delete the green branch.
- name: Integration-clientarrays-split - expected red under run-ahead (ID-82 / ID-134)
working-directory: build-split
env:
MOBILEGL_ITEST_REQUIRE_GPU: "1"
MOBILEGL_TRANSPORT: "inproc"
run: |
ulimit -c unlimited
logs=MobileGL/MG_IntegrationTest/split-logs
rm -f "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null || true
rc=0
ctest -L integration-clientarrays-split --no-tests=error -j 1 \
> "${RUNNER_TEMP}/clientarrays.txt" 2>&1 || rc=$?
tail -5 "${RUNNER_TEMP}/clientarrays.txt"
if ! grep -rqF "run-ahead ARMED" "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null; then
# kMGPipeP5eRunAheadReady is false on this head: the client is lockstep, vi's arm is
# alive and these entries are the positive control they were registered as.
if [ "${rc}" -ne 0 ]; then
echo "::error::integration-clientarrays-split is red WITHOUT run-ahead armed. Under lockstep the client-memory array path is legal and vi keeps it working, so this is a real regression in that arm and not ID-82's refusal."
exit 1
fi
echo "integration-clientarrays-split is green: run-ahead is not armed on this build, so ID-82's refusal is unreachable"
exit 0
fi
if [ "${rc}" -eq 0 ]; then
echo "::error::integration-clientarrays-split went GREEN with run-ahead ARMED. ID-82 refuses a client-memory vertex array under run-ahead by name, so a green run means the refusal did not fire and the server dereferenced a client pointer from an unbarriered record. If P8 has landed the staged form, delete this step and fold the entries back into integration-split."
exit 1
fi
marker='Fatal{UnmigratedVerb, "'
if ! grep -rhoE 'Fatal\{UnmigratedVerb, "(Multi)?DrawArrays\+CLIENT_ARRAYS"\}' \
"${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null | sort -u | grep -q CLIENT_ARRAYS; then
echo "::error::integration-clientarrays-split is red but no private log carries ${marker}...+CLIENT_ARRAYS\"} - the lane failed for some OTHER reason, which is a defect rather than the expected-red control"
grep -rhoE '(Fatal|Admitted)\{[A-Za-z]*, "[^"]*"\}' "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null | sort | uniq -c | sort -rn | head
exit 1
fi
echo "integration-clientarrays-split is red by design on:"
grep -rhoE 'Fatal\{UnmigratedVerb, "(Multi)?DrawArrays\+CLIENT_ARRAYS"\}' \
"${logs}"/*ClientVertexArrayScenario*.log | sort -u
- name: Upload strict lane logs
if: always()
uses: actions/upload-artifact@v7
MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp
+71 -7
View File
@@ -84,13 +84,8 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
constexpr SizeT kMaxComputeWorkGroups = 65535;
constexpr SizeT kMaxComputeFlattenedIndices = kMaxComputeWorkGroups * kComputeWorkGroupSize;
Uint BoundDrawIndirectBufferId() {
const auto& indirect =
MGB_CTX->GetBufferBindingSlot(BufferTarget::DrawIndirect).GetBoundObject();
if (!indirect) return 0;
const auto* resource = BufferImpl::EnsureBufferResource(indirect);
return resource ? resource->id : 0;
}
// BoundDrawIndirectBufferId MOVED (P5e ra2, ID-136) to sit beside BoundIndexBufferId,
// which is the same question about the other target and now has the same two arms.
// ---------------------------------------------------------------------------
// The bound index buffer, and WHICH SIDE ANSWERS FOR IT
@@ -150,6 +145,24 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
std::abort();
}
// ---- P5e (ra2), ID-136: THE SAME REFUSAL FOR THE INDIRECT TARGET -------------------
//
// A sibling of mv's rather than a second idiom, because it is the same sentence about
// the other buffer target: a missing record on the handle arm aborts by name and never
// falls back to the frontend binding slot, which answers for whatever the CLIENT has
// bound NOW - a later verb than the one being applied.
[[noreturn]] void RefuseMissingIndirectBufferRecord(const char* entry,
MG_Pipe::MGPipeHandle res) {
MGLOG_F("MGPipe: Fatal{RoleViolation, \"multidraw-indirect-buffer-arm\"} - %s found no "
"backend resource for the indirect buffer {%u, %u} this verb's record named. "
"The indirect buffer of this family is MGPipeApplier().VerbIndirectBuffer "
"(CONTRACT-P5E §2.1) and the arm is selected by Transport != Monolith "
"(ID-81); falling back to the frontend GL_DRAW_INDIRECT_BUFFER slot here "
"would restore whatever the CLIENT has bound now",
entry, res.Slot, res.Gen);
std::abort();
}
// Does the applier's record say the application SUPPLIED this store's content? The M-3
// rule ScopedRestartIndexSubstitution spells: for a supplied store a coverage gap in the
// server shadow is a missing record and Fatal by name; for one the application ORPHANED
@@ -247,6 +260,57 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
return ResolveBoundIndexBuffer(IndexBufferQuestion::DriverName, "BoundIndexBufferId").Id;
}
// The GL name on GL_DRAW_INDIRECT_BUFFER, i.e. what a tier that swaps in its own scratch
// COMMAND buffer has to put back - the exact twin of BoundIndexBufferId above, and now
// with the same two arms.
//
// ---- P5e (ra2), ID-136: THE SEAT THIS FILE'S OWN RULE HAD MISSED -------------------
//
// This was the LAST unguarded frontend read on the multi-draw apply path, and it sat
// twenty lines above the function that retired its neighbour. It asked
// `MGB_CTX->GetBufferBindingSlot(BufferTarget::DrawIndirect)` and then
// `EnsureBufferResource(<frontend object>)` - a registry lookup keyed by the client's
// identity, which is §4.4's rule and not only the allocator's. Under run-ahead that is
// `Fatal{UnmigratedPipeInput, "GetBufferBindingSlot@DrawArrays"}` on every batch the
// indirect tiers execute: 18 lane entries, and the only thing standing between the flip
// and a green lane once the fill race was fixed.
//
// WHAT IT IS NOT: a data dependency. The tier does not want the client's indirect
// buffer - it never reads a byte of it. It binds its OWN scratch command buffer
// (g_indirectCommands) and wants to put back the name that was there. So the answer is
// not "migrate the value" (which is what P8 owes for the ordinary indirect draw path);
// it is "ask the side that did the binding". ID-133's escalation (iii) legalised the
// pull instead, at the price of a rendezvous on every plain glMultiDraw* on the DEFAULT
// tier - a real cost on the shipping arm, paid to make a lane green. ID-136 withdraws
// that escalation and retires the read, in the commit that adds this arm.
//
// WHY MGPipeApplier().VerbIndirectBuffer IS THE WHOLE ANSWER ON THIS ARM: with a
// transport, the ONLY writer of this process's GL_DRAW_INDIRECT_BUFFER outside this
// function is DirectGLES.cpp's DrawSyncBit::IndirectBuffer arm, which binds from exactly
// that handle and nothing else. So "what was bound" IS "what the verb's record named",
// and a null handle is "this verb bound none" - the same 0 the monolith arm returns for
// an empty slot, and not the arm test (ID-110: the arm was decided by the transport
// above, never inferred from a null).
Uint BoundDrawIndirectBufferId() {
#if MOBILEGL_BUILD_DISAGGREGATED
if (MG_Config::Transport != MG_Config::TransportMode::Monolith) {
const MG_Pipe::MGPipeHandle res = MG_Pipe::MGPipeApplier().VerbIndirectBuffer;
if (MG_Pipe::MGPipeHandleIsNull(res)) return 0;
auto* resource = BufferImpl::EnsureBufferResourceForHandle(nullptr, res);
if (resource == nullptr) {
RefuseMissingIndirectBufferRecord("BoundDrawIndirectBufferId", res);
}
return resource->id;
}
#endif
// MONOLITH GLUE from here down, token for token what this function did before.
const auto& indirect =
MGB_CTX->GetBufferBindingSlot(BufferTarget::DrawIndirect).GetBoundObject();
if (!indirect) return 0;
const auto* resource = BufferImpl::EnsureBufferResource(indirect);
return resource ? resource->id : 0;
}
// ---------------------------------------------------------------------------
// Scratch GL objects
//
MobileGL/MG_Backend/Init.cpp
+1 -1
View File
@@ -95,7 +95,7 @@ namespace MobileGL::MG_Backend {
// server into a run-ahead one, because the failure mode is not a slow frame - it is
// the apply thread reading client memory that has already moved, which renders wrong
// rather than aborting.
constexpr Bool kMGPipeP5eRunAheadReady = false;
constexpr Bool kMGPipeP5eRunAheadReady = true;
// P5e (sb, ID-106). DirectGLES GAINS BIT 13 - the indexed buffer binding points - and
// Magma deliberately does not. c0e landed the phase constant and the dirty-bit map with
MobileGL/MG_Impl/Pipe/PipeFill.cpp
+109 -1
View File
@@ -394,7 +394,48 @@ namespace MobileGL::MG_Pipe {
// transform-feedback span, so clause 2 is false for it - and the verb is a no-op below
// either way.
Bool ClientVerbIsBarriered(MGPipeVerb verb, GLContext* ctx) {
const MGPWireOp op = WireOpForVerb(verb);
MGPWireOp op = WireOpForVerb(verb);
// ---- P5e (ra2): THE JOIN IS MANY-TO-ONE ON THE DRAW FAMILY, SO THE INVERSE IS NOT
// A FUNCTION, AND THE DEFAULT BELOW WAS ANSWERING FOR NINETEEN VERBS -----------------
//
// MGP_VERB_OP_LIST carries ONE row for the whole draw family - `DrawVbo ->
// DrawArrays` - because that is the direction the SERVER needs: a draw_vbo record
// stamps a verb boundary and DrawArrays is the name it prints. Inverting it
// verb-first therefore answers kOpCount for DrawElements, DrawElementsBaseVertex,
// DrawElementsIndirect, MultiDrawElementsBaseVertex and every other indexed /
// instanced / multi / indirect verb - all of which emit exactly that same draw_vbo.
//
// The conservative default below reads "a verb the join does not know answers
// BARRIERED: fill it, wait for it". THE FILL HAPPENS AND THE WAIT DOES NOT. The wait
// is not decided here - it is decided per RECORD, by MGPipeBarriered(op, payload,
// st) at the publish (§2.1) - and the record is a draw_vbo, whose wait class is
// kWaitNone. So every indexed draw filled gPipeInputs while telling
// RefusePipeInputsTouchWhileApplierOwnsIt, through isBarrieredFill, that this thread
// was about to park behind it, and then ran on without parking. The apply thread was
// measured inside a record at that instant, and the abort it produced named the
// CLIENT's verb - a verb the applier cannot stamp, which is what identifies the
// writer (report §2).
//
// So the family's one row is applied to the family. Every other unknown verb keeps
// the conservative answer, which is now honest rather than hoped for: the fill site
// establishes quiescence before it writes (MGPipeValidateForVerb below), so a
// barriered fill no longer rests on a park that may never come.
static_assert(MGPipeVerbForWireOp(MGPWireOp::DrawVbo) == MGPipeVerb::DrawArrays,
"the draw family's representative row moved; the fallback below names "
"draw_vbo because that is the record every kDraw verb emits");
// P5e (ra2): THE CLIENT'S HALF OF ESCALATION (iii) WAS HERE AND WENT WITH IT
// (ID-133, withdrawn by ID-136). It named MultiDrawArrays / MultiDrawElements /
// MultiDrawElementsBaseVertex so the client would FILL for the records the server
// was escalating - a fill the server then read as a barriered pull. Retiring that
// pull outright (MultiDraw.cpp's BoundDrawIndirectBufferId takes a handle arm) left
// nothing for the fill to serve, so a plain multi-draw is an ordinary kDraw verb
// again and takes the draw_vbo answer below. The pair is the phase's own lesson
// written twice: a wait added to make a lane green is paid by an arm, and here the
// arm was the default tier the phone ships.
if (op == MGPWireOp::kOpCount &&
kMGPipeVerbClass[static_cast<SizeT>(verb)] == MGPipeVerbClass::kDraw) {
op = MGPWireOp::DrawVbo;
}
if (op == MGPWireOp::kOpCount) return true;
if (MGPipeWaitClassFor(op) != kWaitNone) return true;
if (MGPipeCallClassFor(op) == kCtxVerb && ctx != nullptr &&
@@ -404,6 +445,40 @@ namespace MobileGL::MG_Pipe {
return false;
}
// ---- P5e (ra2), CONTRACT-P5E §3.5 AMENDED: A BARRIERED FILL MAKES ITSELF QUIESCENT ----
//
// §3.5 exempted the residual fill of a BARRIERED record from the single-writer rule on
// the ground that "this thread is about to park behind it". That is a claim about the
// FUTURE, and the write is in the PRESENT: the order at the validate point is fill,
// then emit, then park, and between the fill and the park the apply thread is still
// draining the UNBARRIERED records the client ran ahead of. The claim was therefore
// never an argument about this instant, and the guard that took it - the
// `if (isBarrieredFill) return;` arm of RefusePipeInputsTouchWhileApplierOwnsIt - could
// not fire however wrong the fill was.
//
// This makes the claim TRUE instead of asserting it. §2.5's forced wait is exactly
// "publish nothing, wait for the applier to reach LastPublishedSeq, drain the reverse
// channel", which is the definition of the window the fill needs, and it already exists
// for glFinish and for BackendObject_Remote's forwarders.
//
// IT IS CALLED AT EVERY GL-THREAD WRITE INTO THE BLOCK, not once per verb, because the
// validate point writes the block in TWO phases that straddle record publication: the
// serial bump / stamp withdrawal / verb rename run BEFORE the emitters, and the 63-field
// walk of step 4 runs AFTER them - and the records the emitters published are records
// whose apply READS the block. One wait cannot cover both halves.
//
// THE ARM IS STATED, NOT INFERRED (ID-81): WaitForApplyToCatchUp's own first test is
// `m_runAheadArmed && m_started`, and m_runAheadArmed is the latch of
// `Transport != Monolith && Ipc.RunAhead && kMGPipeP5eRunAheadReady && the caps bit`. So
// on the monolith arm, on the pull build and under MOBILEGL_IPC_RUN_AHEAD=0 this is a
// call that returns, and the lockstep client's behaviour is unchanged byte for byte
// (G1) - under lockstep appliedSeq is already at LastPublishedSeq by construction.
void QuiesceApplierBeforeFill(const char* surface) {
MG_Remote::Client::ClientSession* session = MG_Remote::Client::ClientSession::Active();
if (session == nullptr) return; // no session: this process has no applier to outrun
session->WaitForApplyToCatchUp(surface);
}
// Whether the LAST fill actually happened, i.e. whether the rows in the block describe
// the verb in flight. Read by MGPipeNoteFrontendMutation, which refreshes one field of
// that fill: with no fill behind it there is nothing to refresh and the write would be
@@ -637,6 +712,14 @@ namespace MobileGL::MG_Pipe {
// P5c (gt, layer 2): the single-field refresh is a client write into gPipeInputs too -
// same gate as the fill, and it makes the same claim the fill made: the rows it is
// touching belong to a record this thread is parked behind (or will park behind).
//
// P5e (ra2): SO IT TAKES THE SAME WAIT. This one runs at a frontend mutation point, not
// at a verb, so "will park behind" is even weaker here than it is at the fill - the
// mutation can land anywhere between two verbs, with the whole run-ahead backlog in
// flight. The wait is a no-op unless run-ahead is armed, and at a mutation point after a
// barriered verb the applier is usually already caught up, so this is a watermark test
// rather than a park in the common case.
QuiesceApplierBeforeFill("MGPipeNoteFrontendMutation");
MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt(
"MGPipeNoteFrontendMutation", /*isBarrieredFill=*/true);
#endif
@@ -3315,6 +3398,17 @@ namespace MobileGL::MG_Pipe {
// thread is about to park behind it. The second argument is which of the two
// claims the caller is making, and a fill that made the second one falsely is the
// named abort.
//
// P5e (ra2): AND IT IS MADE TRUE FIRST. Phase 1 of the block write is three lines
// below - the serial bump, MGPipeServerClearVerbBoundary() and SetVerb - and every
// one of them is a scalar the apply thread reads at each field access inside the
// record it is currently applying. Withdrawing the server's own stamp from under it
// is what produced `Fatal{UnmigratedPipeInput, "<RECORD-SUPPLIED field>@<this
// thread's verb>"}` on the apply thread (report §2). THE RED-ONCE IS THIS LINE:
// delete it and the guard below aborts with Fatal{RoleViolation, "gPipeInputs"}
// naming MGPipeValidateForVerb, because the guard now tests the fact rather than
// taking the claim.
QuiesceApplierBeforeFill("MGPipeValidateForVerb");
MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt(
"MGPipeValidateForVerb", /*isBarrieredFill=*/barriered);
}
@@ -3630,6 +3724,20 @@ namespace MobileGL::MG_Pipe {
// own boundary and §3.3's detector turns any field it still needs from here into
// Fatal{UnmigratedPipeInput, "<field>@<verb>"} by name, which is what makes the strict
// lane a gate rather than a count.
#if MOBILEGL_BUILD_DISAGGREGATED
// P5e (ra2): PHASE 2 OF THE BLOCK WRITE, AND IT NEEDS ITS OWN WAIT. Everything between
// the phase-1 wait and here PUBLISHED RECORDS - the fourteen emitters above - and the
// apply thread reads gPipeInputs while it applies them. So the window the fill opened at
// the top of this function was closed again by this function's own emissions, and the
// 63-field walk below would run straight into it. Same call, same arm, same no-op
// everywhere run-ahead is not armed; the guard beside it is what turns a missing one
// into a named abort rather than a torn read.
if (fillOwed) {
QuiesceApplierBeforeFill("MGPipeValidateForVerb/residual");
MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt(
"MGPipeValidateForVerb/residual", /*isBarrieredFill=*/true);
}
#endif
const Bool applierDerives = ApplierDerivesRenderStateFields();
// BY VALUE, NOT BY REFERENCE: the memo's storage is rebuilt in place when the key moves,
// and the walk below holds this across 63 iterations.
MobileGL/MG_IntegrationTest/CMakeLists.txt
+22 -1
View File
@@ -2192,13 +2192,34 @@ if (MOBILEGL_BUILD_DISAGGREGATED)
# them by dereferencing a client pointer per draw - and it is therefore both the positive
# control for vi's decision to KEEP that arm alive under lockstep and the only entry that
# can reach the run-ahead refusal when ra publishes the caps bit.
#
# ---- P5e (ra2), ID-134: AND WITH THE CAPS BIT PUBLISHED IT IS A LANE OF ITS OWN ----------
#
# "When ra publishes the caps bit" is now, and what these two entries reach is
# `Fatal{UnmigratedVerb, "DrawArrays+CLIENT_ARRAYS"}` / `"MultiDrawArrays+CLIENT_ARRAYS"` -
# ID-82's NAMED REFUSAL, working exactly as ruled. They cannot be green under run-ahead and
# they should not be: a lane that demanded them green would be demanding that the refusal
# not fire, which is the opposite of what vi registered them for. Staging the bytes is P8's.
#
# THE LABEL IS `integration-clientarrays-split` AND THE SPELLING IS THE POINT (ID-131, and
# gl's `integration-magma-split` above is the worked example): `ctest -L` takes a REGEX, so a
# label spelled `integration-split-clientarrays` is still MATCHED by `-L integration-split`
# and the entries would never have left the lane they were moved out of - a split that reads
# as done in every listing and is not. Counted rather than assumed: `ctest -N -L
# integration-split` is 179 after this change and was 181 before, and `-L integration-gpu`
# is 1357 against 1359. `integration-gpu` goes with it for the same reason `integration-
# magma-split` dropped it - a lane that must be green cannot contain an entry that must not.
#
# The two entries keep their private, distinct log paths (SplitLogPaths.cmake.in lists
# MGL_SPLIT_VI_CLIENT_ARRAY_TESTS by name), which is what lets the expected-red CI step
# assert the MARKER rather than merely tolerate a failure.
gtest_discover_tests(MobileGLIntegrationTest
TEST_PREFIX "DirectGLES.Split."
TEST_LIST MGL_SPLIT_VI_CLIENT_ARRAY_TESTS
TEST_FILTER "ClientVertexArrayScenario.*"
DISCOVERY_TIMEOUT 30
PROPERTIES
LABELS "integration-gpu\;integration-split"
LABELS "integration-clientarrays-split"
TIMEOUT ${MGL_ITEST_TIMEOUT}
ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}"
)
MobileGL/MG_Pipe/PipeApply.cpp
+21
View File
@@ -3015,6 +3015,27 @@ namespace MobileGL::MG_Pipe {
if (op == MGPWireOp::DrawVbo && payload != nullptr) {
const auto& draw = *static_cast<const MGPDrawInfo*>(payload);
if ((draw.Flags & static_cast<Uint8>(kDrawClientArrays)) != 0) return true;
// ---- ESCALATION (iii) WAS HERE AND IS WITHDRAWN (P5e ra2, ID-133 then ID-136) ---
//
// ID-133 escalated a PLAIN multi-draw (`NumDraws > 1 && !kDrawIsIndirect`) so that
// MultiDrawImpl::RunIndirect's read of the client's GL_DRAW_INDIRECT_BUFFER binding
// became a legal barriered pull instead of an unbarriered Fatal. It worked, and it
// cost a rendezvous on EVERY plain glMultiDraw* on the DEFAULT tier: there is one
// draw opcode - all twenty entry points collapse onto draw_vbo - and the tier is
// chosen on the server per batch (MultiDraw.cpp's ResolveTierForBatch), so no
// predicate both roles can compute names the arm that actually needed it.
//
// THE CHECK ON AN ESCALATION IS "WHICH ARM PAYS FOR IT", NOT "WHICH LANE GOES
// GREEN", and that is the rule this pair of rulings exists to record. The read was
// a save/restore of a GL binding NAME around the tier's own scratch buffer, not a
// data dependency, so the answer was to ask the side that did the binding:
// BoundDrawIndirectBufferId now takes the handle arm its neighbour
// ResolveBoundIndexBuffer already had, and the pull is GONE rather than legalised.
//
// Both halves landed in ONE commit on purpose: the pull retired without this clause
// withdrawn is a cost with no reason, and this clause withdrawn without the pull
// retired puts 18 lane entries back on the unbarriered arm. Neither is a state to
// gate or to measure, so neither was ever a head.
}
return false;
}
MobileGL/MG_Pipe/PipeApply.h
+9 -1
View File
@@ -921,6 +921,14 @@ namespace MobileGL::MG_Pipe {
// contract, not a condition somebody adds at a call site - because a fourth clause that only
// one side computes is the same silent failure one level down.
//
// THERE WAS AN ESCALATION (iii) AND IT WAS WITHDRAWN, which is worth a line here rather than
// only in the history: ID-133 barriered a plain multi-draw to legalise a pull in Espryt's
// indirect multi-draw tier, ID-136 retired the pull instead (MultiDraw.cpp's
// BoundDrawIndirectBufferId) and took the clause back out. The rule it leaves behind is the
// one to apply to the next candidate: ask WHICH ARM PAYS for an escalation, not which lane
// it turns green - (iii) was keyed on the only wire fact available and therefore charged the
// default tier for a read only two opt-in tiers made.
//
// Escalation (ii) is a REFUSAL under run-ahead (§5.1), so on a run-ahead server it never
// reaches the sink at all; it is here so the predicate is TOTAL and so the lockstep arm,
// where such a draw is legal, gets the barrier it needs.
@@ -992,7 +1000,7 @@ namespace MobileGL::MG_Pipe {
// Computing the predicate anyway (PipeApplier::ApplyOne) is deliberate: it keeps the
// function exercised on every record for the whole phase rather than first run on the day
// it starts deciding.
inline constexpr Bool kMGPipeP5eClientWaitRuleLanded = false;
inline constexpr Bool kMGPipeP5eClientWaitRuleLanded = true;
// P5c (rv): the two serials the PipeInputs texture-shutter accessors answer with under a
// server-stamped verb (FieldOwnership.def, APPLIER_DERIVED). Free functions rather than
MobileGL/MG_Remote/CONTRACT-P5E.md
+59
View File
@@ -103,6 +103,32 @@ must not be told a draw is unbarriered on that account. Escalation (ii) is a ref
(§5.1), so on a run-ahead server it never reaches the sink; it is listed so the predicate is total.
**No other runtime escalation exists**; adding one is an integrator ruling and a row here.
**There was an escalation (iii) and it was WITHDRAWN — ID-133 added it, ID-136 took it back out,
and the reason is the most reusable thing P5e learned about escalations.** ID-133 barriered a
plain multi-draw (`NumDraws > 1 && !kDrawIsIndirect`) so that `MultiDrawImpl::RunIndirect`'s read
of the client's `GL_DRAW_INDIRECT_BUFFER` binding became a legal barriered pull instead of
`Fatal{UnmigratedPipeInput, "GetBufferBindingSlot@DrawArrays"}` on 18 lane entries. It worked, and
it was the wrong instrument:
- There is exactly **one draw opcode** — all twenty draw entry points collapse onto `draw_vbo` —
and the multi-draw tier is resolved on the SERVER, per batch (`MultiDraw.cpp`'s
`ResolveTierForBatch`), from driver caps the client does not hold. So "escalate the indirect
multi-draw op" has no op to name and no predicate both roles can compute, and the narrowest
available key charged **every plain `glMultiDraw*` on every tier**, including the `auto` → `ext`
default the phone ships — a per-batch rendezvous on the shipping arm, added to satisfy a lane.
- And the read was never a data dependency: `BoundDrawIndirectBufferId` SAVES AND RESTORES a GL
binding name around the tier's own scratch command buffer. Giving it the handle arm its
neighbour `ResolveBoundIndexBuffer` already had retires the pull outright.
**The rule this leaves for the next candidate escalation: ask WHICH ARM PAYS FOR IT, not which
lane it turns green.** A wait added to make a lane green is a real cost on a real path; if the
predicate cannot name the arm that needs it, the escalation is charging arms that do not.
The two halves — retiring the pull and withdrawing the clause — landed in one commit (ID-136
overrode ID-113's package boundary for it): the pull retired without the clause withdrawn is a
cost with no reason, and the clause withdrawn without the pull retired puts the 18 entries back
on the unbarriered arm.
### 2.2 The static column, row by row
| class | rows | why |
@@ -212,6 +238,39 @@ UNBARRIERED apply is a finding: `MGLOG_E_ONCE` with the kind, Fatal under strict
outside a barriered fill is `Fatal{RoleViolation, "gPipeInputs"}` by name.
`Fatal{BarrierViolation}` (`:840`) stays for `BatchWaits==0 && !RunAhead`.
**AMENDED by ra2 (ID-132, and it is the sentence this section got wrong).** "Outside a
barriered fill" was the whole test, and a barriered fill was exempted on the strength of what
the caller SAID about itself: *"this touch is the residual fill of a record this thread is
about to park behind"*. That is a claim about the **future**, and the write is in the
**present** — the order at the validate point is fill, then emit, then park, so at the instant
of the write the apply thread is still draining the unbarriered records the client ran ahead
of. **A guard whose exemption cannot be false on the class it exists for is not a guard.** It
never fired on any of the 70 red lane entries; what fired instead was the apply thread, on
`Fatal{UnmigratedPipeInput, "<field>@<the CLIENT's verb>"}`, after the GL thread had bumped
`CurrentVerbSerial`, withdrawn `m_serverStampedVerb` and renamed `m_currentVerb` underneath a
record it was inside.
**The rule as landed:** under `RunAheadArmed` a GL-thread write to the block is legal only
while `ApplyThreadIsInsideApplier()` is false, barriered fill or not; and the three fill sites
(`MGPipeValidateForVerb` phase 1, its step-4 residual walk, `MGPipeNoteFrontendMutation`)
establish that by taking §2.5's forced wait first (`QuiesceApplierBeforeFill`,
`PipeFill.cpp`). **Two waits per filling verb, not one**, because the block is written in two
phases that straddle publication — the serial bump / stamp withdrawal / verb rename before the
emitters, the 63-field walk after them — and the records those emitters published are records
whose apply reads the block. The red-once is deleting a wait; the guard then aborts by name on
the first filling verb behind a run-ahead backlog. Pinned by
`RemoteGuards.ClientBarrieredFillWhileTheApplierIsInsideUnderRunAheadIsFatalByName` with its
green control beside it; both run in the `unit` lane, because the strict lane runs under
lockstep and cannot see this class by construction.
The same correction applies to **§3.2 above, which is UNLANDED and was never marked as such**:
"the server's stamp is server-private … never into the shared block" describes storage the
applier owns, and `MGPipeServerStampVerbBoundary` still writes `m_filled` / `m_currentVerb` /
`m_serverStampedVerb` into `gPipeInputs` itself. That gap is exactly what made the race
possible, and splitting the stamp is the P11 item — but note that per-role stamps **without**
versioning the BARRIER_PULLED values would be worse than today, because it turns a loud
`Fatal{UnmigratedPipeInput}` into a silent stale read. Both, or the values retired first.
---
## §4 id — handle-keyed twins
MobileGL/MG_Remote/Client/ClientSession.cpp
+32 -6
View File
@@ -1316,14 +1316,40 @@ namespace MobileGL::MG_Remote::Client {
//
// This is where a fill left standing for an unbarriered record lands, by name.
if (ClientSessionInstance().RunAheadArmed()) {
if (isBarrieredFill) return;
// ---- P5e (ra2): THE EXEMPTION IS A FACT NOW, NOT A CLAIM ------------------------
//
// This arm used to read `if (isBarrieredFill) return;`, i.e. the caller's own
// sentence "this touch is the residual fill of a record this thread is about to park
// behind" was accepted as the whole argument. It is an argument about the FUTURE.
// The order at the validate point is fill, then emit, then park, so at the instant
// of the write the apply thread is still draining the unbarriered records the client
// ran ahead of - and the measured consequence was the GL thread bumping
// CurrentVerbSerial, withdrawing MGPipeServerClearVerbBoundary's flag and renaming
// m_currentVerb underneath a record the applier was inside, which surfaced as
// Fatal{UnmigratedPipeInput, "<field>@<the client's verb>"} on the apply thread.
// Because the exemption was unconditional, this guard could not fire on any of it:
// that is the answer to "does RefusePipeInputsTouchWhileApplierOwnsIt fire, and if
// not, why not".
//
// So the sentence still exempts the fill, but only once it is TRUE of this instant.
// The fill sites establish it by taking §2.5's forced wait first
// (QuiesceApplierBeforeFill, PipeFill.cpp); removing that wait is the red-once and
// lands here, by name, on the first barriered verb behind a run-ahead backlog.
//
// ApplyThreadIsInsideApplier() is an acquire load of the flag PipeApplier raises for
// the whole of ApplyOne - stamp, decode and LeaveApplier included - so "not inside"
// is exactly "no record is being applied" and not merely "not decoding".
if (isBarrieredFill && !ApplyThreadIsInsideApplier()) return;
if (Server::ServerLoop::OnApplyThread()) return; // the applier owns the block
MGLOG_F("MGPipe: Fatal{RoleViolation, \"gPipeInputs\"} - the GL thread touched "
"gPipeInputs (%s) on a RUN-AHEAD session outside a barriered fill. With "
"the client running ahead the block is the server's to read for as long "
"as an unbarriered record is in flight (CONTRACT-P5E §3), and this thread "
"has no wait that would make the touch quiescent",
surface);
"gPipeInputs (%s) on a RUN-AHEAD session %s. With the client running ahead "
"the block is the server's to read for as long as ANY record is in flight "
"(CONTRACT-P5E §3), so the only legal GL-thread write is a barriered fill "
"that has first taken §2.5's forced wait (QuiesceApplierBeforeFill)",
surface,
isBarrieredFill ? "in a barriered fill that did not first wait for the "
"applier to catch up"
: "outside a barriered fill");
std::abort();
}
// MOBILEGL_IPC_BATCH_WAITS=1 makes "the fill runs while the apply thread applies an
MobileGL/MG_Remote/Client/ClientSession.h
+10 -3
View File
@@ -208,11 +208,18 @@ namespace MobileGL::MG_Remote::Client {
// P5e (ra, CONTRACT-P5E §3.5) RE-DERIVES IT UNDER RUN-AHEAD, and `isBarrieredFill` is
// what the caller says about ITSELF: "this touch is the residual fill of a record this
// thread is about to park behind". With run-ahead armed the block is server-role
// memory for every other touch, so the rule stops being "not while the applier is
// inside" - which is unanswerable when the client never waits - and becomes "not at
// all, unless you are that fill". The MOBILEGL_IPC_BATCH_WAITS early return goes with
// memory for every other touch. The MOBILEGL_IPC_BATCH_WAITS early return goes with
// it: batching is about how many round trips a lockstep client pays, and it says
// nothing about a client that pays none.
//
// P5e (ra2) KEEPS THE ARGUMENT AND STOPS TAKING IT ON TRUST. "About to park behind it"
// is a claim about the future; the write is now. Between the fill and the park the
// apply thread is still draining the unbarriered records the client ran ahead of, so an
// unconditional exemption made this guard unfireable on exactly the class it was
// written for - measured as the apply thread aborting on a field whose stamp the GL
// thread had just withdrawn. So the rule is "not while the applier is inside" again,
// for barriered fills too; what changed is that the fill sites MAKE it true first, with
// §2.5's forced wait (PipeFill.cpp's QuiesceApplierBeforeFill), instead of asserting it.
static void RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface,
Bool isBarrieredFill = false);
MobileGL/MG_Test/Wire/RemoteClientTest.cpp
+67
View File
@@ -2297,6 +2297,10 @@ TEST(RemoteGuards, ClientPipeInputsTouchOutsideABarrieredFillUnderRunAheadIsFata
// ... and the control: the barriered fill says so and is let through. Without this the case
// above would pass just as well against "abort unconditionally".
//
// P5e (ra2): THIS CONTROL IS NOW ALSO HALF OF A PAIR, and the half it does NOT state is the
// one that cost the flip its lane. "The applier is not inside" is true here because nothing in
// this child ever entered it; the case below is the same call with that one fact reversed.
TEST(RemoteGuards, ClientPipeInputsTouchInsideABarrieredFillUnderRunAheadIsAllowed) {
const auto child = RunInChild([] {
StartRunAheadSession();
@@ -2307,6 +2311,69 @@ TEST(RemoteGuards, ClientPipeInputsTouchInsideABarrieredFillUnderRunAheadIsAllow
ExpectChildSuccess(child);
}
// ---- P5e (ra2): THE FLIP'S OWN RED-ONCE, AND THE CLASS THE STRICT LANE CANNOT SEE ----------
//
// The strict lane runs under LOCKSTEP - ApplyOne stamps every record barriered - so nothing
// that happens only once the client stops waiting is visible to it, by construction (ID-132).
// This pair is what measures it instead, and it runs in the ordinary unit lane on any head,
// flip thrown or not, because StartRunAheadSession arms run-ahead itself.
//
// WHAT IT PINS: `isBarrieredFill` is the caller's sentence "this touch is the residual fill of
// a record this thread is about to park behind". That is a claim about the FUTURE. The order at
// the validate point is fill, then emit, then park, so at the instant of the write the apply
// thread is still draining the UNBARRIERED records the client ran ahead of - and the exemption
// that took the claim on trust made this guard unfireable on precisely the class it exists for.
// Measured on the flipped head: the GL thread bumped CurrentVerbSerial, withdrew the server's
// stamp and renamed m_currentVerb underneath a record the applier was inside, and the applier
// aborted with `Fatal{UnmigratedPipeInput, "<field>@<the CLIENT's verb>"}` - a verb no applier
// stamp can produce, which is what identifies the writer.
//
// THE PROBE IS THE GUARD CALL ITSELF (ID-102): the flag is raised by the same raw entry point
// PipeApplier's ScopedApplierEntry uses, on the client thread, BEFORE the call under test - so
// what aborts is this probe and not some object built on the wrong thread.
//
// THE RED: in ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt put the run-ahead arm back
// to `if (isBarrieredFill) return;` and this case stops aborting, while the control above keeps
// passing - which is the difference between a rule and an exemption.
TEST(RemoteGuards, ClientBarrieredFillWhileTheApplierIsInsideUnderRunAheadIsFatalByName) {
const auto child = RunInChild([] {
StartRunAheadSession();
ClientSession::NoteApplyThreadEnteredApplier();
ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt("ra-test-surface",
/*isBarrieredFill=*/true);
ClientSession::NoteApplyThreadLeftApplier();
ClientSessionInstance().Stop();
});
ExpectNamedAbort(child, "Fatal{RoleViolation, \"gPipeInputs\"}");
}
// ---- P5e (ra2): AND THE OTHER HALF - AN INDEXED DRAW IS AN UNBARRIERED VERB ----------------
//
// MGP_VERB_OP_LIST joins the whole draw family through ONE row, `DrawVbo -> DrawArrays`, so
// inverting it verb-first answers kOpCount for DrawElements and for every other indexed /
// instanced / multi / indirect verb. The "unknown verb answers barriered" default then made the
// client FILL for each of them while never parking, because the wait is decided per record and
// the record is a draw_vbo (kWaitNone). On the flipped lane that was 21 of the 69 red entries.
//
// The case states it where it is decidable without a GL context: the guard runs before the
// validate point's null-context return, so a verb that answers UNBARRIERED never reaches the
// guard at all and the child exits 0 even with the applier flag up.
//
// THE RED: delete the kDraw fallback in ClientVerbIsBarriered (PipeFill.cpp) and DrawElements
// answers barriered again - the fill runs, the guard above it sees the raised flag, and this
// case aborts with Fatal{RoleViolation, "gPipeInputs"} instead of exiting 0.
TEST(RemoteRunAhead, AnIndexedDrawVerbIsUnbarrieredAndTouchesPipeInputsNotAtAll) {
const auto child = RunInChild([] {
StartRunAheadSession();
ClientSession::NoteApplyThreadEnteredApplier();
MGPipeValidateForVerb(MGPipeVerb::DrawElements);
ClientSession::NoteApplyThreadLeftApplier();
MGPipeLeaveVerb();
ClientSessionInstance().Stop();
});
ExpectChildSuccess(child);
}
// §3.3's RED-ONCE, and the one that turns the strict lane into a gate. The probe is the
// sticky forward itself, run on the apply thread inside a server-stamped verb with the
// current record marked UNBARRIERED - which is the state the sink puts that thread in for