From d83fc6643a359fee4fe93b9487005451ec5f0867 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 21 Sep 2026 18:30:18 -0400 Subject: [PATCH] [CI, android-plugin] (P6): CI starts a second MobileGL process on a device The Android retrace matrix gains a third lane, spawn-acceptance, beside monolith-control and inproc-acceptance. Until now nothing in CI had ever started a second MobileGL process on a device at all. The apk.yml retrace job already carried a `transport` matrix dimension and already branched on it, so the lane itself is three edits: a row in github_transport_matrix, an elif in the retrace step, and a summary to render. What needed thought was the gate. THE SPAWN LANE IS APK-ONLY, and that is a statement about where evidence is missing rather than about what spawn can do. Linux already has test.yml's `retrace-split`, which is {backend, case, transport} over inproc AND spawn with the two negative controls that make it mean anything. A spawn row in the Linux transport matrix would pay for a second full sweep proving nothing - and test.yml's `retrace` job stages no server image, so the lane would not merely be redundant, it would be red. self_test_transport_matrices now asserts the absence in both directions, because an accidental removal of the `if apk` has to be caught in the self-test rather than in CI. --require-spawn REQUIRES TWO SENTENCES, and the second is the one that matters. ConfigLoader's marker proves the VALUE RESOLVED - it exists only in the Spawn arm, which exists only under MOBILEGL_BUILD_DISAGGREGATED - but that is a statement about a parser. `spawn ARMED - the server role runs in pid N` is written only after the launch, the connect and the handshake have all succeeded, and the pid in it is the SERVER's. It is the only line in the log a same-process run cannot produce. Both halves red once, measured: * a monolith run demanded to prove spawn: REFUSED, transport_resolution_marker false, server_pid null - while the retrace itself scored ssim=1.000000, which is exactly the trap. OpenRA matches its golden under monolith too, so the picture has never been the evidence. * the same run's real spawn log with the one ARMED line deleted: REFUSED, transport_resolution_marker TRUE and server_pid null. That is the "resolved the transport and then never left the process" case, isolated. THE PROOF USES `any` WHERE THE inproc GATE USES `all`, and the difference is load-bearing rather than a relaxation. Under spawn two processes append to one MOBILEGL_LOG_FILE_PATH, and the launcher deliberately scrubs every MOBILEGL_IPC_* from the child's environment (anti-recursion catch (b)) - so the server's own `Config: IPC` line reads strict=0 role-split-state=0 BY CONSTRUCTION. Demanding every line match would red every spawn run for the one thing the design requires. --use-pbuffer is not optional on this lane: until P12 a spawned server cannot be handed a window, because an ANativeWindow* is a pointer into the CLIENT's process (Rule H). MOBILEGL_IPC_SERVER_PATH is deliberately NOT passed - nativeLibraryDir carries an install-time hash that nothing off the device can spell, and the Activity resolves it. The local runner now passes the same knobs and the same --require-* flag CI does, so "it works locally" can no longer mean "it ran monolith locally". That change immediately caught its own omission: the first local run with the gate armed failed on client_ipc_line_present=false because the runner had been sending MOBILEGL_TRANSPORT alone. Verified on device 2f7cbe2e: OpenRA.DirectGLES spawn, ssim=1.000000, mismatchPixels=0, "real spawn transport, server role in pid 9885, and zero Fatal records confirmed". --- .github/workflows/apk.yml | 38 ++++++++++++ android-plugin/trace-replay-ci.sh | 58 +++++++++++++++++++ .../trace_replay/run_android_retrace_local.py | 29 +++++++++- tools/trace_replay/trace_cases.py | 46 ++++++++++++--- 4 files changed, 161 insertions(+), 10 deletions(-) diff --git a/.github/workflows/apk.yml b/.github/workflows/apk.yml index b8258833..46e3623c 100644 --- a/.github/workflows/apk.yml +++ b/.github/workflows/apk.yml @@ -469,6 +469,23 @@ jobs: --env "MOBILEGL_TRANSPORT=inproc;MOBILEGL_IPC_ROLE_SPLIT_STATE=1;MOBILEGL_IPC_STRICT_ERRORS=1;MOBILEGL_IPC_RUN_AHEAD=1" --require-inproc ) + elif [ "${MOBILEGL_CI_TRANSPORT}" = "spawn" ]; then + # THE SAME CLIENT KNOBS AS inproc, so the two acceptance lanes differ in exactly one + # thing: which PROCESS the server role runs in. They are scrubbed out of the CHILD's + # environment by the launcher and read by the client, which is where they belong. + # + # MOBILEGL_IPC_SERVER_PATH IS DELIBERATELY ABSENT. nativeLibraryDir carries an + # install-time hash, so nothing off the device can spell it; TraceReplayActivity + # resolves it from getApplicationInfo() when the env asks for spawn, and an explicit + # value would override that. --use-pbuffer is NOT optional here: until P12 a spawned + # server cannot be handed a window at all, because an ANativeWindow* is a pointer + # into the CLIENT's process and SetWindowHandle is refused on the way across with + # Fatal{UnmigratedSurface, "AndroidNativeWindow@P12"} (Rule H). + extra_retrace_args+=( + --env "MOBILEGL_TRANSPORT=spawn;MOBILEGL_IPC_ROLE_SPLIT_STATE=1;MOBILEGL_IPC_STRICT_ERRORS=1;MOBILEGL_IPC_RUN_AHEAD=1" + --use-pbuffer + --require-spawn + ) else extra_retrace_args+=(--env "MOBILEGL_TRANSPORT=monolith") fi @@ -633,6 +650,13 @@ jobs: pattern: MobileGL-android-retrace-result-inproc-acceptance-inproc-* path: retrace-artifacts/inproc-acceptance + - name: Download Android spawn acceptance results + if: env.MOBILEGL_CI_DISAGGREGATED == 'ON' + uses: actions/download-artifact@v8 + with: + pattern: MobileGL-android-retrace-result-spawn-acceptance-spawn-* + path: retrace-artifacts/spawn-acceptance + - name: Render monolith control summary run: | node tools/trace_replay/render_retrace_summary.mjs \ @@ -652,6 +676,11 @@ jobs: --group-label "Android Emulator / inproc acceptance" \ --html mobilegl-android-inproc-acceptance.html + - name: Render spawn acceptance summary + if: env.MOBILEGL_CI_DISAGGREGATED == 'ON' + run: | + node tools/trace_replay/render_retrace_summary.mjs --input retrace-artifacts/spawn-acceptance --output-dir android-retrace-summary/spawn-acceptance --title "MobileGL Android spawn acceptance" --group-label "Android Emulator / spawn acceptance" --html mobilegl-android-spawn-acceptance.html + - name: Upload Android monolith control summary uses: actions/upload-artifact@v7 with: @@ -669,6 +698,15 @@ jobs: archive: false if-no-files-found: error + - name: Upload Android spawn acceptance summary + if: env.MOBILEGL_CI_DISAGGREGATED == 'ON' + uses: actions/upload-artifact@v7 + with: + name: MobileGL-android-spawn-acceptance-summary-${{ github.sha }} + path: android-retrace-summary/spawn-acceptance/mobilegl-android-spawn-acceptance.html + archive: false + if-no-files-found: error + remove-artifact-clutter: name: remove artifact clutter runs-on: ubuntu-latest diff --git a/android-plugin/trace-replay-ci.sh b/android-plugin/trace-replay-ci.sh index b508cbda..75f2480e 100644 --- a/android-plugin/trace-replay-ci.sh +++ b/android-plugin/trace-replay-ci.sh @@ -34,6 +34,7 @@ Usage: [--dump-texture-2d CALL,TEXTURE,LEVEL,DIR] \ [--env "K=V;K=V"] \ [--require-inproc] \ + [--require-spawn] \ [--benchmark] \ [--benchmark-tail-frames N] \ [--benchmark-finish 0|1] \ @@ -67,6 +68,8 @@ variables to the replay process. They are applied last, immediately before libMobileGL.so is loaded, so they override every flag above; an entry with no "=" unsets the variable instead. This is the generic passthrough: a MOBILEGL_* knob that has no flag of its own needs no plumbing to be forwarded. +Pass --require-spawn to require the same proof for transport=spawn, plus the one +sentence a monolith fallback can never write: the pid of the server process. Pass --require-inproc to require the library's real transport-resolution log, strict errors and separate role state, plus zero Fatal records. This checks the APK process after replay; a host environment echo or an SSIM-only pass is not proof. @@ -131,6 +134,7 @@ coherent_as_flush=0 texture_2d_dumps="" env_overrides="${MOBILEGL_TRACE_ENV:-}" require_inproc=0 +require_spawn=0 benchmark=0 benchmark_tail_frames=200 benchmark_finish=1 @@ -178,6 +182,7 @@ while [ "$#" -gt 0 ]; do --dump-texture-2d) texture_2d_dumps="$(next_arg "$@")"; shift 2 ;; --env) env_overrides="$(next_arg "$@")"; shift 2 ;; --require-inproc) require_inproc=1; shift 1 ;; + --require-spawn) require_spawn=1; shift 1 ;; --benchmark) benchmark=1; shift 1 ;; --benchmark-tail-frames) benchmark_tail_frames="$(next_arg "$@")"; shift 2 ;; --benchmark-finish) benchmark_finish="$(next_arg "$@")"; shift 2 ;; @@ -559,6 +564,59 @@ proof_path.write_text(json.dumps(proof, indent=2), encoding="utf-8") if not passed: sys.exit("trace replay failed actual inproc/strict/role/zero-Fatal proof: " + json.dumps(proof)) print("Android retrace: real inproc transport, strict role state and zero Fatal records confirmed") +PY + fi + if [ "${require_spawn}" -eq 1 ]; then + "${PYTHON}" - "${result_dir}/mobilegl.log" "${result_dir}/transport-proof.json" <<'PY' +import json +from pathlib import Path +import re +import sys + +log_path, proof_path = map(Path, sys.argv[1:]) +text = log_path.read_text(encoding="utf-8", errors="replace") if log_path.is_file() else "" + +# TWO SENTENCES, AND THE SECOND IS THE ONE THAT MATTERS. +# +# ConfigLoader's marker proves the VALUE RESOLVED - it exists only in ConfigLoader's Spawn arm, +# which exists only under MOBILEGL_BUILD_DISAGGREGATED, so a pull library cannot write it. That +# is a statement about a parser. +# +# `spawn ARMED - the server role runs in pid N` is written by ClientSession::StartSpawned only +# after the launch, the connect and the handshake have ALL succeeded, and the pid in it is the +# SERVER's. It is the only line in this log a same-process run cannot produce: inproc resolves +# its own marker while running the server role on a thread HERE. Requiring only the first would +# accept a session that resolved spawn and then never reached another process. +marker = "Config: MOBILEGL_TRANSPORT=spawn - the MGPipe record stream" +armed = re.search(r"spawn ARMED - the server role runs in pid (\d+)", text) + +# `any`, NOT the `all` the inproc gate uses, and the difference is load-bearing. +# +# Under spawn TWO PROCESSES append to one MOBILEGL_LOG_FILE_PATH, and the launcher deliberately +# scrubs every MOBILEGL_IPC_* out of the child's environment (anti-recursion catch (b)) - so the +# server's own `Config: IPC` line reads strict=0 role-split-state=0 BY CONSTRUCTION. Demanding +# that every line match would red every spawn run for the one thing the design requires. +configs = re.findall(r"Config: IPC[^\r\n]*", text) +fatals = re.findall(r"^.*Fatal\{.*$", text, re.MULTILINE) +required = {"strict": "1", "role-split-state": "1", "run-ahead": "1"} +def matches(line): + return all(re.search(r"\b" + re.escape(key) + "=" + value + r"\b", line) + for key, value in required.items()) +client_configs = [line for line in configs if matches(line)] +config_ok = bool(client_configs) + +passed = marker in text and armed is not None and config_ok and not fatals +proof = {"required_transport": "spawn", "library_log": str(log_path), + "transport_resolution_marker": marker in text, + "server_pid": int(armed.group(1)) if armed else None, + "ipc_config_lines": configs, "required_ipc_settings": required, + "client_ipc_line_present": config_ok, + "fatal_count": len(fatals), "fatal_lines": fatals, "passed": passed} +proof_path.write_text(json.dumps(proof, indent=2), encoding="utf-8") +if not passed: + sys.exit("trace replay failed actual spawn/second-process/zero-Fatal proof: " + json.dumps(proof)) +print("Android retrace: real spawn transport, server role in pid " + f"{proof['server_pid']}, and zero Fatal records confirmed") PY fi } diff --git a/tools/trace_replay/run_android_retrace_local.py b/tools/trace_replay/run_android_retrace_local.py index f08a188f..81f63d12 100644 --- a/tools/trace_replay/run_android_retrace_local.py +++ b/tools/trace_replay/run_android_retrace_local.py @@ -449,6 +449,28 @@ def parse_args(): return parser.parse_args() +def transport_proof_args(args): + """The same arm-proof flag CI passes, so a local run cannot be greener than the lane. + + trace-replay-ci.sh's --require-inproc / --require-spawn read the LIBRARY's own log and + refuse a run whose transport did not actually resolve - and for spawn, one whose server + role never left the process. Passing them here too is what stops "it works locally" from + meaning "it ran monolith locally". + """ + return {"inproc": ["--require-inproc"], "spawn": ["--require-spawn"]}.get(args.transport, []) + + +# The knobs .github/workflows/apk.yml hands BOTH acceptance lanes. They are kept here verbatim so +# a local run is the same run: the arm proof below reads the library's own `Config: IPC` line and +# requires strict/role-split-state/run-ahead to be 1, so a local invocation that quietly omitted +# them would fail a gate CI passes - or worse, pass a weaker one. +SPLIT_ACCEPTANCE_KNOBS = ( + "MOBILEGL_IPC_ROLE_SPLIT_STATE=1", + "MOBILEGL_IPC_STRICT_ERRORS=1", + "MOBILEGL_IPC_RUN_AHEAD=1", +) + + def transport_env(args): """The transport knob as env overrides, or nothing at all for monolith. @@ -456,7 +478,9 @@ def transport_env(args): environment is byte-identical to what it was before this option existed - the control arm has to stay a control. """ - return [] if args.transport == "monolith" else [f"MOBILEGL_TRANSPORT={args.transport}"] + if args.transport == "monolith": + return [] + return [f"MOBILEGL_TRANSPORT={args.transport}", *SPLIT_ACCEPTANCE_KNOBS] def main(): @@ -482,7 +506,8 @@ def main(): failures += run_benchmark_case(case, backend, args) continue print(f"=== Android retrace: {case['name']} / {backend} ===", flush=True) - rc = run_case(case, backend, env_overrides=transport_env(args) + list(args.env), + rc = run_case(case, backend, extra_args=transport_proof_args(args), + env_overrides=transport_env(args) + list(args.env), use_pbuffer=args.use_pbuffer) try: render_summary() diff --git a/tools/trace_replay/trace_cases.py b/tools/trace_replay/trace_cases.py index 4d1b2db0..ffa80339 100644 --- a/tools/trace_replay/trace_cases.py +++ b/tools/trace_replay/trace_cases.py @@ -246,21 +246,36 @@ def github_apk_matrix(cases): def github_transport_matrix(cases, *, apk=False): - """Run the entire original matrix as both control and inproc acceptance. + """Run the entire original matrix as control and as acceptance, once per transport. - The two transports use the same D/P artifact and each case's existing backend + The transports use the same D/P artifact and each case's existing backend list. The legacy split:true subset does not limit this main acceptance matrix. Nested APK case/backend metadata is deliberately left untouched. + + THE SPAWN LANE IS APK-ONLY, and that is a statement about where the evidence is missing + rather than about what spawn can do. On Linux the spawn arm already has a job of its own - + test.yml's `retrace-split`, which is {backend, case, transport} over inproc AND spawn and + carries the two negative controls that make it mean anything (the pull library must red it, + dropping the draws must red it). Adding spawn to THIS matrix as well would pay for a second + full Linux sweep that proves nothing the first one does not, and test.yml's `retrace` job + has neither the server image staged nor MOBILEGL_IPC_SERVER_PATH set - so the lane would + not merely be redundant, it would be red. + + On Android there is no such job. Until this lane existed, nothing in CI ever started a + second MobileGL process on a device. """ build = github_apk_matrix if apk else github_test_matrix if not cases: raise ValueError("transport matrix needs at least one trace case") + lanes = [ + (cases, "monolith-control", "monolith"), + (cases, "inproc-acceptance", "inproc"), + ] + if apk: + lanes.append((cases, "spawn-acceptance", "spawn")) return {"include": [ {**row, "lane": lane, "transport": transport} - for selected, lane, transport in ( - (cases, "monolith-control", "monolith"), - (cases, "inproc-acceptance", "inproc"), - ) + for selected, lane, transport in lanes for row in build(selected)["include"] ]} @@ -275,10 +290,21 @@ def self_test_transport_matrices(): acceptance = [row for row in mixed if row["transport"] == "inproc"] strip_lane = lambda rows: [{key: value for key, value in row.items() if key not in ("lane", "transport")} for row in rows] + spawned = [row for row in mixed if row["transport"] == "spawn"] assert strip_lane(controls) == original, "control lost original case/backend/parameters" assert strip_lane(acceptance) == original, "acceptance lost original case/backend/parameters" assert all(row["lane"] == "monolith-control" for row in controls) assert all(row["lane"] == "inproc-acceptance" for row in acceptance) + # THE SPAWN LANE IS APK-ONLY AND MUST NOT DRIFT INTO THE LINUX MATRIX. test.yml's + # `retrace` job exports matrix.transport straight into MOBILEGL_TRANSPORT and stages no + # server image, so a spawn row reaching it would be red for a packaging reason and say + # nothing about the transport. Asserted in both directions: present for apk, absent + # otherwise - an accidental `if apk` removal has to be caught here rather than in CI. + if apk: + assert strip_lane(spawned) == original, "spawn lane lost original case/backend/parameters" + assert all(row["lane"] == "spawn-acceptance" for row in spawned) + else: + assert not spawned, "the Linux transport matrix must carry no spawn lane" identities = [(row["case"]["name"] if apk else row["case"], row["backend"]["name"] if apk else row["backend"], row["transport"]) for row in mixed] @@ -286,7 +312,10 @@ def self_test_transport_matrices(): # A restricted case keeps exactly its original backend and golden data. restricted = [{**cases[0], "split": False, "ci_backends": ["DirectGLES"]}] rows = github_transport_matrix(restricted, apk=apk)["include"] - assert len(rows) == 2 and strip_lane(rows[:1]) == strip_lane(rows[1:]) + expected_lanes = 3 if apk else 2 + assert len(rows) == expected_lanes + assert all(strip_lane(rows[:1]) == strip_lane(rows[index:index + 1]) + for index in range(1, expected_lanes)) try: github_transport_matrix([], apk=apk) except ValueError: @@ -294,7 +323,8 @@ def self_test_transport_matrices(): else: raise AssertionError("an empty acceptance lane silently passed") print(f"transport matrix {'APK' if apk else 'Linux'}: {len(controls)} unchanged controls, " - f"{len(acceptance)} full acceptance entries; exact metadata and negative controls OK") + f"{len(acceptance)} inproc acceptance entries, {len(spawned)} spawn acceptance " + f"entries; exact metadata and negative controls OK") def cmake_quote(value):