mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 18:13:23 +09:00
[tools, android-plugin, CI] (P7): the device matrix runner selects DirectVulkan, repeats are archived, and the spawn env parse is entry-aware
wave 0 item 5. Three instruments P7's device window needs, and the CI steps that
can make each of them red.
run_tcp_matrix.py --backend DirectVulkan end to end. --backend already existed and
already filtered by each case's ci_backends; what did not exist is a per-backend
GOLDEN. The two backends shared `golden` plus an optional `alternate_golden`, and
alternate_golden is an OR across backends, not a per-backend key - so a
DirectVulkan run that matched the DirectGLES picture passed, which is the "went
green on the wrong arm" shape the exit gate exists to catch. trace_cases.json
gains `backend_overrides.<backend>` ({golden, alternate_golden, ssim_threshold}
only: restating target_call or a crop would stop the two backends retracing the
same frame through the same window). A per-backend golden REPLACES the shared
pair, matching another backend's declared image is refused BY NAME, and a --case
named explicitly that does not run a requested backend is an error instead of one
plan fewer and no message. The CMake emitter and the APK matrix refuse a case that
declares the key rather than silently emitting one golden for both arms.
DirectGLES is unchanged and that is asserted, not asserted-in-prose: a case with no
override resolves to itself, and the key is dropped from the resolved case even for
the backend that overrides nothing - so giving DirectVulkan its own golden does not
move the DirectGLES identity fingerprint and does not invalidate its checkpoints.
Each results.json row now records the golden, alternate golden and threshold it was
scored against.
run_android_retrace_local.py gets --matrix (the CI split subset, 39 cases, each
running only its own ci_backends - not --all, which adds the rd12 workload ID-P7-4
excludes), --repeat N, and --archive-dir. The archive is the point: the result root
is keyed by case and backend and by NOTHING else, so repeat 2 overwrites repeat 1
and the only artefact that survives a three-repeat run is the last one, which makes
"were the three repeats the same picture?" unanswerable after the fact - in the one
place where re-running to find out is expensive. It also refuses --benchmark with a
non-monolith --transport, which has always produced a monolith run under a spawn
label (run_benchmark_case passes args.env alone; p6/README.md records the pitfall).
compare_actuals.py computes the pair result.json cannot: SSIM between two actual
PNGs, plus a case x repeat x ssim-vs-golden x ssim-vs-first-repeat table over an
archive. It is the retrace gate's OWN SSIM - a transcription of ComputeChannelSsim
/ ComputeRgbSsim (trace_replay_core.cpp:630/:671), the only SSIM in the tree - and
the crop comes from each repeat's own result.json, so the two columns describe the
same rectangle. PNG decode is stdlib zlib, because an unavailable Pillow in the
device window would turn "the repeats diverged" into "the tool did not run".
TraceReplayActivity: resolveSpawnServerPath's two decisions were String.contains()
on the whole joined K=V;K=V list and neither was entry-aware. The one that bit:
`--env MOBILEGL_IPC_SERVER_PATH` with no '=' is the documented UNSET spelling
(trace_env_overrides_test.cpp:92) and the only way to ask for "spawn with no image"
and watch LaunchServer refuse by name - it did not match "MOBILEGL_IPC_SERVER_PATH="
so a resolved path was appended AFTER it and, because ApplyEnvOverrides applies in
order with setenv(...,1), the deliberate unset was silently undone. That is exactly
the "silently replacing it would make that knob untestable" the method's own comment
forbids. Two more: the key inside another entry's VALUE flipped both decisions the
wrong way, and the transport match was case-sensitive while ConfigLoader lower-cases
the value before matching (ConfigLoader.cpp:322). The parse moves to
SpawnServerPath, a class with no android.* import, so test_android_lifecycle.py
compiles and runs it with plain javac - the parse had no host-side coverage at all,
and its on-device failure mode is "the knob had no effect".
Only spawn gets a path, deliberately: it is the only transport that execs an image.
Red-once, all four: a stub case whose result names the DirectGLES golden fails a
--backend DirectVulkan run with "matched a golden another backend declares"; a
synthetic PNG pair with a closed-form SSIM (identical = 1.0 exactly, constant
black vs white = C1/(255^2+C1), inverted halves = (C2-2v)/(2v+C2)); the pre-fix
resolveSpawnServerPath body fails eight of SpawnServerPathTest's checks; and the
DirectVulkan TCP proof battery is the DirectGLES one under the other prefix,
because an empty prefix selection raises nothing and reads as a clean pass.
CI: test_run_tcp_matrix.py and test_compare_actuals.py join the R-16 step. The
matrix driver's stub regressions had NO CI step - they were run by hand and
recorded in a note - and they guard the tool that decides whether a device golden
run passed.
Counts in WSL: test_run_tcp_matrix 12 -> 18, tcp_lane_tools 4 -> 6,
test_compare_actuals 12 new, test_android_lifecycle green with the new Java unit.
This commit is contained in:
@@ -0,0 +1,122 @@
|
||||
package top.mobilegl.plugin.trace;
|
||||
|
||||
import java.io.File;
|
||||
import java.util.Locale;
|
||||
|
||||
/**
|
||||
* Resolves {@code MOBILEGL_IPC_SERVER_PATH} for the split arms, from the one process that can
|
||||
* spell it.
|
||||
*
|
||||
* <p>P6: {@code MOBILEGL_TRANSPORT=spawn} NEEDS AN ABSOLUTE PATH TO AN EXEC-ABLE FILE, and on
|
||||
* Android there is exactly one such place. An APK's {@code lib/<abi>/} is the only directory an
|
||||
* untrusted_app may exec from - W^X has forbidden the app's own data dirs since API 29 - and the
|
||||
* packager only puts a file there if it is named {@code lib*.so}, which is why the server
|
||||
* executable wears that name (P0 spike A proved the arrangement on two devices).
|
||||
*
|
||||
* <p>NOTHING OFF THE DEVICE CAN SPELL THAT PATH. {@code nativeLibraryDir} contains an
|
||||
* install-time hash, so the host-side runner cannot pass {@code MOBILEGL_IPC_SERVER_PATH} the way
|
||||
* it passes every other knob; the path has to be resolved HERE, by the process that is about to
|
||||
* launch the server. Left unresolved, {@code LaunchServer} refuses BY NAME and the arm reds on
|
||||
* "unresolvable image" - honest, but about the wrong thing.
|
||||
*
|
||||
* <p>AN EXPLICIT VALUE WINS. A caller that spoke about {@code MOBILEGL_IPC_SERVER_PATH} itself is
|
||||
* pointing somewhere deliberately - or deliberately pointing NOWHERE - and silently replacing
|
||||
* that would make the knob untestable.
|
||||
*
|
||||
* <p>This lives in a class of its own, with no {@code android.*} import, so that
|
||||
* {@code tools/trace_replay/test_android_lifecycle.py} can compile and run it with plain javac.
|
||||
* It is a hand-rolled parse of the same {@code K=V;K=V} string the native side parses, and the
|
||||
* only lane that used to exercise it end to end was an on-device spawn retrace - which reports a
|
||||
* parse bug as "the knob had no effect", the hardest shape to notice there is.
|
||||
*
|
||||
* <p>THE RULES ARE THE NATIVE SIDE'S, NOT A SECOND SET. They are the ones pinned by
|
||||
* {@code tools/trace_replay/trace_env_overrides_test.cpp} and applied by
|
||||
* {@code trace_replay_core.cpp:143} {@code ApplyEnvOverrides}:
|
||||
* <ul>
|
||||
* <li>the list is split on {@code ';'} and empty entries are dropped;</li>
|
||||
* <li>the FIRST {@code '='} separates, so a value may contain {@code '='};</li>
|
||||
* <li>an entry with NO {@code '='} is an UNSET, not a key with an empty value;</li>
|
||||
* <li>an empty key is ignored;</li>
|
||||
* <li>entries are applied in order with {@code setenv(..., 1)}, so the LAST one wins;</li>
|
||||
* <li>and ConfigLoader lower-cases {@code MOBILEGL_TRANSPORT} before matching it
|
||||
* ({@code ConfigLoader.cpp:322}), so the comparison here is case-insensitive too.</li>
|
||||
* </ul>
|
||||
*/
|
||||
final class SpawnServerPath {
|
||||
static final String TRANSPORT_KEY = "MOBILEGL_TRANSPORT";
|
||||
static final String SERVER_PATH_KEY = "MOBILEGL_IPC_SERVER_PATH";
|
||||
static final String SERVER_LIBRARY = "libMobileGLServer.so";
|
||||
|
||||
private SpawnServerPath() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns {@code envOverrides} with the resolved server path appended, or unchanged.
|
||||
*
|
||||
* <p>ONLY {@code spawn} GETS A PATH, and that is not an oversight: it is the only transport
|
||||
* that execs an image. {@code monolith} runs no server role at all and {@code inproc} runs it
|
||||
* on a thread of this process, so a server path there would be an unused variable that made
|
||||
* the two arms differ in their environment for no reason.
|
||||
*/
|
||||
static String resolve(String envOverrides, String nativeLibraryDir) {
|
||||
String entries = envOverrides == null ? "" : envOverrides;
|
||||
if (nativeLibraryDir == null || nativeLibraryDir.isEmpty()) {
|
||||
return entries;
|
||||
}
|
||||
if (!"spawn".equals(effectiveTransport(entries)) || mentions(entries, SERVER_PATH_KEY)) {
|
||||
return entries;
|
||||
}
|
||||
String entry =
|
||||
SERVER_PATH_KEY + "=" + new File(nativeLibraryDir, SERVER_LIBRARY).getAbsolutePath();
|
||||
return entries.isEmpty() ? entry : entries + ";" + entry;
|
||||
}
|
||||
|
||||
/**
|
||||
* The transport the LIBRARY will see: the last assignment wins, lower-cased, and a trailing
|
||||
* unset of the key means there is none.
|
||||
*
|
||||
* <p>Returns {@code null} when the list says nothing about the transport, which is how
|
||||
* "ConfigLoader falls back to its monolith default" is spelled here.
|
||||
*/
|
||||
static String effectiveTransport(String envOverrides) {
|
||||
String value = null;
|
||||
for (String entry : split(envOverrides)) {
|
||||
int separator = entry.indexOf('=');
|
||||
if (separator < 0) {
|
||||
if (entry.equals(TRANSPORT_KEY)) {
|
||||
value = null; // The documented unset spelling.
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (separator == 0) {
|
||||
continue; // An empty key never reaches setenv.
|
||||
}
|
||||
if (entry.substring(0, separator).equals(TRANSPORT_KEY)) {
|
||||
value = entry.substring(separator + 1);
|
||||
}
|
||||
}
|
||||
return value == null ? null : value.toLowerCase(Locale.ROOT);
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the caller named {@code key} at all - as an assignment OR as a bare unset.
|
||||
*
|
||||
* <p>Both spellings are the caller speaking. The unset one matters most: it is the only way
|
||||
* to ask for "spawn with no image" and see {@code LaunchServer} refuse by name, and appending
|
||||
* a resolved path after it would silently undo it, because the last entry wins.
|
||||
*/
|
||||
static boolean mentions(String envOverrides, String key) {
|
||||
for (String entry : split(envOverrides)) {
|
||||
int separator = entry.indexOf('=');
|
||||
String name = separator < 0 ? entry : entry.substring(0, separator);
|
||||
if (!name.isEmpty() && name.equals(key)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private static String[] split(String envOverrides) {
|
||||
return envOverrides == null ? new String[0] : envOverrides.split(";", -1);
|
||||
}
|
||||
}
|
||||
+12
-25
@@ -357,33 +357,20 @@ public final class TraceReplayActivity extends Activity {
|
||||
this.envOverrides = envOverrides;
|
||||
}
|
||||
|
||||
// P6: MOBILEGL_TRANSPORT=spawn NEEDS AN ABSOLUTE PATH TO AN EXEC-ABLE FILE, and on
|
||||
// Android there is exactly one such place. An APK's lib/<abi>/ is the only directory an
|
||||
// untrusted_app may exec from - W^X has forbidden the app's own data dirs since API 29 -
|
||||
// and the packager only puts a file there if it is named lib*.so, which is why the server
|
||||
// executable wears that name (P0 spike A proved the arrangement on two devices).
|
||||
// P6: MOBILEGL_TRANSPORT=spawn NEEDS AN ABSOLUTE PATH TO AN EXEC-ABLE FILE, and this is
|
||||
// the one process that can spell it. The reasoning, and the K=V;K=V parsing rules it has
|
||||
// to share with ApplyEnvOverrides, live in SpawnServerPath - a class with no android.*
|
||||
// import, so test_android_lifecycle.py compiles and runs it with plain javac.
|
||||
//
|
||||
// NOTHING OFF THE DEVICE CAN SPELL THAT PATH. nativeLibraryDir contains an
|
||||
// install-time hash, so the host-side runner cannot pass MOBILEGL_IPC_SERVER_PATH the way
|
||||
// it passes every other knob; the path has to be resolved HERE, by the process that is
|
||||
// about to launch the server. Left unresolved, LaunchServer refuses BY NAME and the arm
|
||||
// reds on "unresolvable image" - honest, but about the wrong thing.
|
||||
//
|
||||
// AN EXPLICIT VALUE WINS. A caller that set MOBILEGL_IPC_SERVER_PATH itself is pointing
|
||||
// somewhere deliberately, and silently replacing it would make that knob untestable.
|
||||
// P7: THE TWO DECISIONS BELOW USED TO BE String.contains() ON THE WHOLE JOINED LIST, and
|
||||
// neither of them was entry-aware. The one that bit: `--env MOBILEGL_IPC_SERVER_PATH`
|
||||
// (no '='), the documented UNSET spelling and the only way to ask for "spawn with no
|
||||
// image" and watch LaunchServer refuse by name, did not match "MOBILEGL_IPC_SERVER_PATH="
|
||||
// - so the resolved path was appended AFTER it and, because the last entry wins, the
|
||||
// deliberate unset was silently undone. That is precisely the "silently replacing it
|
||||
// would make that knob untestable" SpawnServerPath's own contract forbids.
|
||||
static String resolveSpawnServerPath(String envOverrides, String nativeLibraryDir) {
|
||||
if (nativeLibraryDir == null || nativeLibraryDir.isEmpty()) {
|
||||
return envOverrides;
|
||||
}
|
||||
if (!envOverrides.contains("MOBILEGL_TRANSPORT=spawn")) {
|
||||
return envOverrides;
|
||||
}
|
||||
if (envOverrides.contains("MOBILEGL_IPC_SERVER_PATH=")) {
|
||||
return envOverrides;
|
||||
}
|
||||
String serverPath = new File(nativeLibraryDir, "libMobileGLServer.so").getAbsolutePath();
|
||||
String entry = "MOBILEGL_IPC_SERVER_PATH=" + serverPath;
|
||||
return envOverrides.isEmpty() ? entry : envOverrides + ";" + entry;
|
||||
return SpawnServerPath.resolve(envOverrides, nativeLibraryDir);
|
||||
}
|
||||
|
||||
static TraceReplayRequest from(Intent intent, File filesDir, String defaultBackend,
|
||||
|
||||
Reference in New Issue
Block a user