Files
MobileGL/docs/Disaggregated/notes/p5e/TASK-gl.md
T
swung0x48 aba329cc53 [Docs] (Disaggregated): P6's plan and contract draft; the working notes move from a local directory into the repository
Two things land together: the plan and contract draft P6 needs before it starts, and the
working notes that until now existed on exactly one machine.

1. P6's plan and contract draft (neither started; not one line of code)

     docs/Disaggregated/P6-SPAWN-PLAN.md      the package plan
     docs/Disaggregated/P6-CONTRACT-DRAFT.md  the contract draft (moves to
                                              MG_Remote/CONTRACT-P6.md when c6 lands)

   The three findings that mattered, all written into the documents:

   a. P6 is SMALLER than its roadmap cell suggests. The transport primitives are already in
      the tree and already tested: SocketDoorbell including hangup detection and the two
      lost-wakeup fences (Doorbell.cpp:211-360, four cases in FdPassingTest), SCM_RIGHTS fd
      passing, ShmSegment::Adopt, and the SurfaceOp / WindowKind schema already defined in
      protocol.fbs. inproc uses a real second shm mapping rather than an alias, and
      SessionRings.h:22-28 says that was done so today is not the first time it runs. P6 is
      assembly plus a control plane plus a process, not "write a socket transport".

   b. The evidence for "P6 is only a transport swap" has EXPIRED. It was bought by P5c, whose
      audit found 59 direct memory accesses in a tree that already looked clean. P5e changed
      the tree again. So the first package, a6, is a read-only audit that writes no code, and
      every contract row that depends on it is marked PENDING a6 rather than decided in
      advance.

   c. P6 does not get a game running on the phone. ANativeWindow* is meaningless in the child,
      and protocol.fbs:186-196's own comment says Android transfers the window out of band -
      that band is P12. P6 lands pbuffer / surfaceless / offscreen, which is what HeadlessGL,
      the split-equivalent lane and trace replay need, and which is exactly what P6's own exit
      gate measures. A real window reaching the server is a named refusal. This gets its own
      section because "P6 is done" would otherwise read as "the game runs now".

   The contract adds rule G: a role may not name a process-local handle. It extends P5c's
   rule E (a role may not name the other role's memory) from memory to identity - EGLDisplay /
   EGLSurface / EGLContext / ANativeWindow* / fd number / pid / mapped address either cross as
   a token this contract defines or they do not cross, and a descriptor crosses only through
   SCM_RIGHTS, where the kernel renumbers it.

   What P5e left for P6 gets its own section. The latch semantics are already designed
   (ARCHITECTURE.md:504) and are copied unchanged. What is new is that after run-ahead the
   client no longer waits while the peer process can DIE: the client holds records that were
   published and will never be applied, and Android's freezer suspends the child without
   closing anything. So slow and dead must be distinguishable WITHOUT a timeout threshold.
   That is P6's only new design; everything else is carriage. It shares a predicate with E1's
   outstanding re-specification (ID-122), and the two should be designed together.

   Two gates carried over from P5e's lessons: G2 requires integration-spawn's test-name set to
   match integration-split's name for name, because a differing set means the lane is quietly
   running less; and the arm-proof gate requires every entry to record the child pid and
   transport=spawn in its OWN private log - ID-124 established that this kind of false green
   really happens, and a spawn lane that actually ran monolith would pass every other gate.
   S2's shape must be settled before it is written, and the document says why: E1 is the
   cautionary case.

2. The working notes move into the repository (docs/Disaggregated/notes/, 415 .md files)

   ROADMAP / CURRENT_STAGE_PROGRESS / MEASUREMENTS cite ruling numbers, audit line numbers and
   the provenance of gate figures from ~/w7/notes/, a directory that exists on one machine and
   is under no version control. A record cited hundreds of times that lives only locally is not
   a record, so it moves in whole, with its directory structure intact.

   Only .md moves. Scripts, run logs, census and gate result JSON, patches, APKs and nm dumps
   stay where they are: they are evidence DATA rather than documents - large, regenerated per
   run, and mostly carrying absolute paths and device serials that mean something only on the
   machine that produced them. The mapping and this split are written in
   docs/Disaggregated/notes/README.md.

   Two exceptions are recorded there too: P6's BRIEF has been promoted to a first-class
   document and keeps no copy under notes/, so the two cannot drift apart; and recovered/ holds
   material salvaged from failed workflow runs, which may duplicate the like-named phase
   directories and is kept because a few of those were the only complete version at the time.

   CI's citation lint takes docs/Disaggregated/*.md only (non-recursive), and notes/ is
   deliberately outside it: those notes' file:line citations were written against their own
   code heads, and resolving them against today's HEAD would fail wholesale - which is not an
   error, it is their correct state as a historical record. To check one, use the revision it
   declares (--rev).

3. Inconsistencies fixed in passing

   - README.md's status line still said P5e was in progress while the stage table already said
     closed.
   - CURRENT_STAGE_PROGRESS's next-step item 1 still listed the exit gate as pending, though
     (a) and (b) were run in the previous commit; rewritten to closed plus the one item that
     actually remains (E1).
   - ROADMAP's P6 row still had the pre-phase plan in three of its cells; it now carries the
     package order, the scope boundary and the two new gates.

Checked with check_doc_citations.py: 125 citations across docs/Disaggregated/*.md, 0 problems.
2026-09-18 23:31:39 -04:00

12 KiB

TASK-gl — make the strict lane mean something, and make the flip survivable

Read PACKAGE-PREAMBLE-P5E.md and PREAMBLE-ADDENDUM-WAVE3.md first. Your tree is ~/w7/p5e-gl, branch p5e/gl, base 7c6f6886. Your slug for the gate driver is gl.

You are not migrating a field family. You are fixing the mechanism that decides whether P5e is finished, plus two defects that would break other backends the moment the phase's flags flip. Two sibling packages are running concurrently (pa on the program family in ~/w7/p5e-pa, mv on multi-draw in ~/w7/p5e-mv). Stay out of their trees and out of MG_Backend/DirectGLES/DirectGLES.cpp and MultiDraw.cpp except where this brief names a line.

Do the items in the order given. Items 1 and 2 are correctness landmines and must land even if you run out of room for the rest; commit each item separately.


Item 1 — the barriered stamp must ask the server's own capability (ruling ID-111)

This is a latent crash, not a cleanup. MGPipeRunAheadCapBitsFor (MG_Pipe/MGPipeTypes.h:160-166) never publishes kCapRunAheadApply on BackendType::DirectVulkan, because ruling ID-90 keeps Magma in lockstep for all of P5e. But the server-side stamp in PipeApplier::ApplyOne (MG_Remote/Server/PipeApplier.cpp:1180-1182) reads only the build constant:

MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(
    MG_Pipe::kMGPipeP5eClientWaitRuleLanded ? wireSaysBarriered : true);

draw_vbo, blit, clear and launch_grid are all kWaitNone. So the moment kMGPipeP5eClientWaitRuleLanded flips, a Magma server stamps every draw record UNBARRIERED while its client is still lockstep — and CountBarrierPull (MG_Backend/MGPipe/PipeInputs.cpp:95-97) aborts unconditionally on an unbarriered pull, no knob involved. Magma dies on its first draw, and MagmaP7AllocatorDebtScope's exemption is withdrawn with it.

Fix: conjoin the server's own capability. ServerPublishesRunAhead() already exists at PipeApplier.cpp:40-44 and answers false for a session with no CallMask yet, which is the right answer for the bring-up window.

MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(
    (MG_Pipe::kMGPipeP5eClientWaitRuleLanded && ServerPublishesRunAhead()) ? wireSaysBarriered : true);

Keep wireSaysBarriered computed unconditionally — that is ID-103's reason, the predicate must stay exercised for the whole phase rather than first run on the day it starts deciding.

Red-once: a RemoteClientTest case that drives a DrawVbo through ApplyOne on a session whose CallMask lacks bit 10 and asserts MGPipeApplierCurrentRecordIsBarriered() is true. Removing the new conjunct makes it false. This is the server-side mirror of the client latch and it is what makes ID-90 true in code rather than in prose.

Item 2 — a compile-time trip wire on the residual fill (ruling ID-112)

EmittedCallSuppliesTheWholeField (MG_Impl/Pipe/PipeFill.cpp:2499-2519) is Magma's only source for the seven pointer-backed fields, and Magma dereferences them immediately. Two of the seven rows, GetBoundVertexArray and GetBufferBindingPoint, have no compile-time protection at all: a package that "tidies up" by removing one would not break the build, it would SIGSEGV Magma on its first draw on a device.

Fix: widen NoP4aFamilyFieldIsWhollySupplied (PipeFill.cpp:2667-2682) to cover all seven rows, or add a second static_assert naming the two unprotected ones. About ten lines. This converts the most likely mistake of every remaining package from a device crash into a build break, which is why it is item 2 and not item 8.

Red-once: delete the GetBoundVertexArray row from the table and show the build fails with your named assertion.

Item 3 — the admission predicate, GENERATED (ruling ID-116)

BRIEF §3.3's "§7 allowlist" exists, in MobileGL/MG_Remote/CONTRACT-P5E.md:475-486. Package ra also landed a hand-kept 13-row copy in .github/workflows/test.yml:1207-1292. That copy is wrong in both directions and must be deleted, not corrected.

The rule, which is ID-84 expressed in tables that already exist: a <field>@<verb> pair is admitted iff the field's row is BARRIER_PULLED and the verb's wire op is statically barriered (MGPipeWaitClassFor(op) != kWaitNone), intersected with kMGPipeClassFieldMask[kMGPipeVerbClass[verb]].

Fix: extend scripts/gen_pipe_field_ownership.py — it already joins FieldOwnership.def, already emits MG_Pipe/generated/PipeFieldOwnership.inc, and already has --check / --self-test wired into the gens gate step — to emit

inline constexpr MGPipeFieldMask kMGPipeAdmittedPullMask[kMGPipeVerbCount] = { … };
constexpr Bool MGPipeBarrierPullAdmitted(MGPipeInputField f, MGPipeVerb v) {
    return MGPipeFieldMaskHas(kMGPipeAdmittedPullMask[SizeT(v)], f);
}

from kMGPipeFieldOwnership, MGP_VERB_OP_LIST (FieldOwnership.def:255) and kMGPipeWaitClasses (generated/PipeWire.inc:376). Add --print-admitted so the shell reads the same table the C++ does. Extend --self-test with negative controls in the style already there.

By this rule GetFramebufferBindingSlot@ReadPixels (21 entries, the largest survivor) is admitted, and GetTextureObject@CopyImageSubData is not — see item 5.

Item 4 — the strict knob becomes admission-aware (ruling ID-117)

Today CountBarrierPull aborts on every barrier-pulled read under MOBILEGL_IPC_STRICT_ERRORS, admitted or not, so the CI's allowlist comparison is unreachable code and "hard green" is impossible with the knob as written. Give it a third state, leaving the unbarriered arm exactly as it is:

if (!MGPipeApplierCurrentRecordIsBarriered())
    StrictBarrierPullFatal(field, verb, "UNBARRIERED, the client did not fill it");   // unchanged
++g_residualPulls; …
if (MG_Config::Ipc.StrictErrors) {
    if (!MGPipeBarrierPullAdmitted(field, verb))
        StrictBarrierPullFatal(field, verb, "MOBILEGL_IPC_STRICT_ERRORS=1");          // unchanged
    AdmittedBarrierPullOnce(field, verb);   // MGLOG_W, deduped per (field, verb)
}

The admitted line keeps the marker grammar and changes only the leading tag:

MGPipe: Admitted{UnmigratedPipeInput, "<F>@<V>"} [BARRIER-PULLED, ADMITTED, retires in <phase>]

so every filter written since P5c that matches Fatal{UnmigratedPipeInput keeps meaning exactly "red". The dedupe is required: an 852-draw frame must not write 852 lines.

Check, do not assume, that FieldOwnershipTest.StrictErrorsTurnsABarrierPulledReadIntoANamedAbort and StrictErrorsAlsoPromotesTheStickyForwards still go red — both stamp MGPipeVerb::DrawArrays, whose op is kWaitNone, so they should.

Item 5 — ResourceCopyRegion becomes barriered (ruling ID-118)

GetTextureObject retires in P7, not P5e, but ResourceCopyRegion is kWaitNone (generated/PipeWire.inc:412). After the flip that is a genuinely unbarriered record reading client memory: an unconditional Fatal, no knob. The ruling is to move the op to a barriered wait class rather than special-case the field into the allowlist — then item 3's derivation admits it with no exception, and P7 can retire the field on its own schedule. CopyImageSubData is not on the per-frame path (zero per frame in the Minecraft scene), so the wait costs nothing measurable. Say in your report which wait class you chose and why.

Item 6 — the lane reads every log, and the rsp pin is reworded (rulings ID-119, ID-115)

Three defects, all measured:

  1. The CI step greps MobileGL/MG_IntegrationTest/split-logs/, which holds 90 of the lane's 116 entries. The 26 it misses are exactly the F1. readback block, both NamedBlit pairs, the four Ct. entries and PersistentMapArm — i.e. precisely the readback population the allowlist is about. Drive the log set from ctest instead of from a directory: split_log_paths.py:12-33 already parses ctest --show-only=json-v1 into {entry: absolute MOBILEGL_LOG_FILE_PATH}. Add a markers mode that walks that map and classifies every Fatal{ and Admitted{ line, and make the lane a two-sided ratchet: an unadmitted marker fails, and an admitted pair that no longer appears must be removed from the expected set (so the lane cannot rot green).
  2. The rsp check greps for a counter no split lane emits. And the pin itself is vacuous: the unbarriered arm of CountBarrierPull is [[noreturn]] and runs before ++g_residualPulls, so rsp counts only barriered pulls and "rsp = 0 on unbarriered records" is true whatever the code does. Reword it to what is checkable: rsp counts only admitted barriered pulls and must equal the admitted-marker tally. Assert rsp against draws rather than against 0 — on the device rsp equalled the draw count under inproc and 0 under monolith, so the draw path's retirement is visible as rsp ceasing to scale with draws. MG_IntegrationTest/Harness/PipeStatsWindow.h already gives you CounterOrAbsent(window, "rsp") and "draws"; six scenarios already use it. You need one counting entry in the split lane with MOBILEGL_PIPE_STATS=1, MOBILEGL_PIPE_STATS_PERIOD=1 and its own private log path — not a whole-lane env flip, which races under -j (PipeStatsWindow.h:19-24 argues this).
  3. The lane has no positive control (ruling ID-115). Of the 7 entries that pass strict today, three are PushMonolithArm (where MGPipeServerStampVerbBoundary never runs, so CountBarrierPull is structurally unreachable), two self-skip, one is a death test and one is a Python check. Not one is a record-carrying split GL scenario. So today's "green" is indistinguishable from "strict never armed". Add an assertion that at least one drawing inproc entry completed with a server-stamped verb boundary. Without it, 116/116 means nothing.

Item 7 — partition the lane by backend, or admit Magma's row

Both DirectVulkan.Split.NamedBlit.* entries carry LABELS "integration-split" (MG_IntegrationTest/CMakeLists.txt:2270-2280) and abort on GetFramebufferBindingSlot@Clear (VulkanRenderer.cpp:7648), which P5e does not unbarrier — GetFramebufferBindingSlot retires in P7 on Magma. CONTRACT-P5E §7 already says in prose that Magma keeps the expected-red step. Give those entries an integration-split-magma label with their own expected-red step, so the Espryt allowlist stays a statement about Espryt. If you find a reason to prefer admitting the row instead, argue it.

Item 8 — mark contract amendment 8 UNLANDED (ruling ID-120)

CONTRACT-P5E.md:503-506 says GetBufferBindingPointCount's sticky forward becomes FATAL under split. FieldOwnership.def:147 and :179 both still say BARRIER_PULLED, and the generator refuses a disagreeing field/forward pair (pinned by FieldOwnershipTest.TheSevenStickyForwardsAgreeWithTheirFieldRows). The amendment is unlanded and landing it as written would change 10 pairs of the derived set. Do not land it. Annotate it in the contract as UNLANDED with the phase that will land it (P7/P13), so the divergence is stated rather than discovered.


Gate before you report

build rc=0 · unit 2250/2250 (expect this number to move — say by how much and why) · isplit 116/116 · gpu 1294/1294 · gens all rc=0, including your new generator arm and its self-test · and the strict lane, which after your change should complete every entry. Report the full marker table split into Fatal{ (must be only what pa and mv still owe) and Admitted{.

Expect the marker table to shift, not vanish. The census is a first-fault histogram: entries that abort today on GetProgramForDraw@DrawArrays will, once that row lands, advance to their next pull. Do not read a changed table as a regression without checking which fault moved.

Report

~/w7/notes/p5e/reports/gl-v1.md: per item, what you did, the red-once verbatim, and for any item you did not reach, exactly where you stopped and why. Also state plainly whether, with your changes, "the strict lane is hard green" is now a statement a script can evaluate — and if not, what is still missing.