mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-08 20:28:32 +09:00
[Feat] (TraceApp, CI): pass arbitrary env vars through the retrace lane
- PLAN-B.md §8.2 and appendix B add a batch of new runtime switches
(MOBILEGL_PIPE_PUSH / _VERIFY / _STATS / _LEGACY_MEMOS / _TEXEL_RETAIN_MB /
_INDEX_MIRROR_MB, plus MOBILEGL_IPC_* later), and §11 P0 wants them parsed beside
the existing ones. Today every knob that has to reach an Android replay costs an
edit in five files - run_android_retrace_local.py, trace-replay-ci.sh,
TraceReplayActivity's request record, the JNI marshalling, and the setenv block in
trace_replay_core.cpp. That per-knob tax is what this replaces: one extra,
`--es mobilegl_env "K=V;K=V"`, carries all of them.
- Applied last, immediately before dlopen(libMobileGL.so), so it can also override
the dedicated fields above it - MobileGL's config is read during the load, and an
escape hatch that cannot beat the defaults is not one. An entry with no '=' unsets
the variable, which is the only way to clear a default the marshalling sets.
- The existing per-knob flags stay: they carry semantics beyond a setenv (use_angle
also selects a variant, the dump lists are joined, DirectVulkan forces the
R11G11B10F fallback), and rewriting them as env strings would move that logic into
the callers.
- Surface: --env / MOBILEGL_TRACE_ENV in trace-replay-ci.sh, repeatable --env
KEY=VALUE in run_android_retrace_local.py, `mobilegl_env` intent extra,
Request::envOverrides.
- The two-level parse now lives in trace_env_overrides.hpp, beside the semicolon
splitter it shares with the texture and FBO dump lists, and
tools/trace_replay/trace_env_overrides_test.cpp pins it: the empty entries a
trailing ';' leaves behind must not become unsetenv(""), `K=` must stay a Set of
the empty string rather than an Unset (a knob read with getenv() != nullptr sees
those as opposite answers), and only the FIRST '=' may separate, or a value
carrying '=' is truncated without a word of warning. The whole MOBILEGL_PIPE_*
batch rides on this parse, and the only lane that exercised it end to end was an
on-device retrace, which would have reported a splitting bug as "the knob had no
effect".
- The check is built and RUN at build time and mobilegl_trace_replay depends on it,
so `cmake --build ... --target mobilegl_trace_replay` - the exact command of
test.yml's "Build trace replay" job, which never invokes ctest - runs it. It is
assert-free on purpose: that lane configures Release, and <cassert> under NDEBUG
would compile every check into a green run that checked nothing. Negative control:
swapping find('=') for rfind('=') fails 2 checks, and keeping the splitter's empty
entries fails 2 more.
This commit is contained in:
@@ -0,0 +1,74 @@
|
||||
#pragma once
|
||||
|
||||
// The generic environment passthrough of the retrace lane, split out of
|
||||
// trace_replay_jni.cpp and trace_replay_core.cpp so a host-side test can pin it.
|
||||
//
|
||||
// One intent extra (`--es mobilegl_env "K=V;K=V"`) carries every MOBILEGL_* knob that has
|
||||
// no dedicated flag, which is what PLAN-B.md §11 P0 needs when it adds MOBILEGL_PIPE_*.
|
||||
// That makes this hand-rolled two-level parse the single point where the whole batch can
|
||||
// be silently misread, and the only lane that exercises it end to end runs on a device -
|
||||
// hence tools/trace_replay/trace_env_overrides_test.cpp, which every desktop configure
|
||||
// that builds the replay runner runs at build time.
|
||||
|
||||
#include <cstddef>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
namespace mobilegl_trace {
|
||||
|
||||
// Splits `A;B;C` into its entries, dropping empty ones. Also used for the texture and
|
||||
// FBO dump lists, whose entries carry their own ',' and ':' separators. A value that
|
||||
// itself contains ';' therefore cannot be expressed - that is the format's limit, not a
|
||||
// bug to work around here.
|
||||
inline std::vector<std::string> SplitSemicolonList(const std::string& value) {
|
||||
std::vector<std::string> values;
|
||||
std::size_t begin = 0;
|
||||
while (begin < value.size()) {
|
||||
const std::size_t end = value.find(';', begin);
|
||||
const std::string entry = value.substr(begin, end - begin);
|
||||
if (!entry.empty()) {
|
||||
values.push_back(entry);
|
||||
}
|
||||
if (end == std::string::npos) {
|
||||
break;
|
||||
}
|
||||
begin = end + 1;
|
||||
}
|
||||
return values;
|
||||
}
|
||||
|
||||
enum class EnvOverrideAction {
|
||||
// Nothing to do: the entry is empty, or names an empty key.
|
||||
Ignore,
|
||||
// setenv(key, value, 1). `K=` is a Set of the empty string, deliberately distinct
|
||||
// from Unset: a knob read with getenv() != nullptr treats them differently.
|
||||
Set,
|
||||
// unsetenv(key). An entry with no '=' means this, and it is the only way for a
|
||||
// caller to clear a variable the per-knob marshalling above it already set.
|
||||
Unset,
|
||||
};
|
||||
|
||||
// Classifies one `KEY=VALUE` / `KEY` entry. The first '=' separates; later ones belong to
|
||||
// the value, so `KEY=a=b` sets KEY to `a=b`.
|
||||
inline EnvOverrideAction ParseEnvOverride(const std::string& entry,
|
||||
std::string* key,
|
||||
std::string* value) {
|
||||
key->clear();
|
||||
value->clear();
|
||||
const std::size_t separator = entry.find('=');
|
||||
if (separator == std::string::npos) {
|
||||
if (entry.empty()) {
|
||||
return EnvOverrideAction::Ignore;
|
||||
}
|
||||
*key = entry;
|
||||
return EnvOverrideAction::Unset;
|
||||
}
|
||||
if (separator == 0) {
|
||||
return EnvOverrideAction::Ignore;
|
||||
}
|
||||
*key = entry.substr(0, separator);
|
||||
*value = entry.substr(separator + 1);
|
||||
return EnvOverrideAction::Set;
|
||||
}
|
||||
|
||||
} // namespace mobilegl_trace
|
||||
@@ -4,6 +4,7 @@
|
||||
#include "apitrace_exit.hpp"
|
||||
#include "png.h"
|
||||
#include "trace_benchmark.hpp"
|
||||
#include "trace_env_overrides.hpp"
|
||||
|
||||
#include <algorithm>
|
||||
#include <cerrno>
|
||||
@@ -130,6 +131,32 @@ std::string JsonEscape(const std::string& value) {
|
||||
return out.str();
|
||||
}
|
||||
|
||||
// Generic environment passthrough. One intent extra carries `K=V;K=V`, so a new
|
||||
// MOBILEGL_* knob costs nothing in the five files between the CI script and this
|
||||
// setenv - the per-knob plumbing above is what this replaces going forward
|
||||
// (PLAN-B.md §11 P0, which adds a batch of MOBILEGL_PIPE_* switches).
|
||||
//
|
||||
// Applied last, immediately before the library is loaded: it is the escape hatch, so it
|
||||
// has to be able to override the fields marshalled above, and MobileGL's ConfigLoader
|
||||
// reads the environment during dlopen. The decision of what each entry means lives in
|
||||
// trace_env_overrides.hpp so a host-side test can pin it; this is only the setenv.
|
||||
void ApplyEnvOverrides(const std::vector<std::string>& entries) {
|
||||
for (const std::string& entry : entries) {
|
||||
std::string key;
|
||||
std::string value;
|
||||
switch (ParseEnvOverride(entry, &key, &value)) {
|
||||
case EnvOverrideAction::Set:
|
||||
setenv(key.c_str(), value.c_str(), 1);
|
||||
break;
|
||||
case EnvOverrideAction::Unset:
|
||||
unsetenv(key.c_str());
|
||||
break;
|
||||
case EnvOverrideAction::Ignore:
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
bool LoadMobileGL(const Request& request, std::string& error) {
|
||||
setenv("MOBILEGL_BACKEND_TYPE", request.backend.c_str(), 1);
|
||||
setenv("MOBILEGL_TRACE_LIBRARY", request.mobileGlLibrary.c_str(), 1);
|
||||
@@ -207,6 +234,8 @@ bool LoadMobileGL(const Request& request, std::string& error) {
|
||||
setenv("MOBILEGL_TRACE_DUMP_TEXTURE_2D", dumpPoints.c_str(), 1);
|
||||
}
|
||||
|
||||
ApplyEnvOverrides(request.envOverrides);
|
||||
|
||||
void* handle = dlopen(request.mobileGlLibrary.c_str(), RTLD_NOW | RTLD_GLOBAL);
|
||||
if (handle == nullptr) {
|
||||
const char* dlError = dlerror();
|
||||
|
||||
@@ -63,6 +63,11 @@ struct Request {
|
||||
bool deriveNumSubgroups = false;
|
||||
bool iterationRPFixBarrier = false;
|
||||
int holdMs = 0;
|
||||
// Generic environment passthrough, each entry `KEY=VALUE` (an entry with no '='
|
||||
// unsets KEY). Applied last, right before libMobileGL.so is loaded, so a knob that
|
||||
// has no dedicated field above can still be forwarded from the CI script without
|
||||
// touching this struct again.
|
||||
std::vector<std::string> envOverrides;
|
||||
};
|
||||
|
||||
struct Result {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
#include "trace_replay_core.hpp"
|
||||
|
||||
#include "spawn_spike.hpp"
|
||||
#include "trace_env_overrides.hpp"
|
||||
|
||||
#include <android/native_window.h>
|
||||
#include <android/native_window_jni.h>
|
||||
@@ -28,22 +29,7 @@ std::string ToString(JNIEnv* env, jstring value) {
|
||||
return out;
|
||||
}
|
||||
|
||||
std::vector<std::string> SplitSemicolonList(const std::string& value) {
|
||||
std::vector<std::string> values;
|
||||
std::size_t begin = 0;
|
||||
while (begin < value.size()) {
|
||||
const std::size_t end = value.find(';', begin);
|
||||
const std::string entry = value.substr(begin, end - begin);
|
||||
if (!entry.empty()) {
|
||||
values.push_back(entry);
|
||||
}
|
||||
if (end == std::string::npos) {
|
||||
break;
|
||||
}
|
||||
begin = end + 1;
|
||||
}
|
||||
return values;
|
||||
}
|
||||
using mobilegl_trace::SplitSemicolonList;
|
||||
|
||||
jobject MakeResult(JNIEnv* env, const mobilegl_trace::Result& result) {
|
||||
jclass clazz = env->FindClass("top/mobilegl/plugin/trace/TraceReplayActivity$TraceReplayResult");
|
||||
@@ -131,7 +117,8 @@ Java_top_mobilegl_plugin_trace_TraceReplayActivity_nativeRunTraceReplay(JNIEnv*
|
||||
jboolean benchmarkMode,
|
||||
jint benchmarkTailFrames,
|
||||
jboolean benchmarkFinish,
|
||||
jstring benchmarkResultPath) {
|
||||
jstring benchmarkResultPath,
|
||||
jstring envOverrides) {
|
||||
mobilegl_trace::Request request;
|
||||
request.tracePath = ToString(env, tracePath);
|
||||
request.goldenPath = ToString(env, goldenPath);
|
||||
@@ -168,6 +155,7 @@ Java_top_mobilegl_plugin_trace_TraceReplayActivity_nativeRunTraceReplay(JNIEnv*
|
||||
: mobilegl_trace::kDefaultBenchmarkTailFrames;
|
||||
request.benchmarkFinish = benchmarkFinish == JNI_TRUE;
|
||||
request.benchmarkResultPath = ToString(env, benchmarkResultPath);
|
||||
request.envOverrides = SplitSemicolonList(ToString(env, envOverrides));
|
||||
|
||||
ScopedTraceReplayState replayState;
|
||||
mobilegl_trace_set_requested_size(request.width, request.height);
|
||||
|
||||
+12
-4
@@ -131,7 +131,8 @@ public final class TraceReplayActivity extends Activity {
|
||||
request.benchmark,
|
||||
request.benchmarkTailFrames,
|
||||
request.benchmarkFinish,
|
||||
request.benchmarkResultPath
|
||||
request.benchmarkResultPath,
|
||||
request.envOverrides
|
||||
);
|
||||
Log.i(TAG, result.toString());
|
||||
TraceReplayResult finalResult = result;
|
||||
@@ -171,7 +172,8 @@ public final class TraceReplayActivity extends Activity {
|
||||
boolean benchmark,
|
||||
int benchmarkTailFrames,
|
||||
boolean benchmarkFinish,
|
||||
String benchmarkResultPath
|
||||
String benchmarkResultPath,
|
||||
String envOverrides
|
||||
);
|
||||
|
||||
|
||||
@@ -254,6 +256,9 @@ public final class TraceReplayActivity extends Activity {
|
||||
final int benchmarkTailFrames;
|
||||
final boolean benchmarkFinish;
|
||||
final String benchmarkResultPath;
|
||||
// Generic environment passthrough, `K=V;K=V`. A future MOBILEGL_* knob needs no
|
||||
// new intent extra, no new JNI parameter and no new field beside this one.
|
||||
final String envOverrides;
|
||||
|
||||
private TraceReplayRequest(
|
||||
String tracePath,
|
||||
@@ -284,7 +289,8 @@ public final class TraceReplayActivity extends Activity {
|
||||
boolean benchmark,
|
||||
int benchmarkTailFrames,
|
||||
boolean benchmarkFinish,
|
||||
String benchmarkResultPath
|
||||
String benchmarkResultPath,
|
||||
String envOverrides
|
||||
) {
|
||||
this.tracePath = tracePath;
|
||||
this.goldenPath = goldenPath;
|
||||
@@ -315,6 +321,7 @@ public final class TraceReplayActivity extends Activity {
|
||||
this.benchmarkTailFrames = benchmarkTailFrames;
|
||||
this.benchmarkFinish = benchmarkFinish;
|
||||
this.benchmarkResultPath = benchmarkResultPath;
|
||||
this.envOverrides = envOverrides;
|
||||
}
|
||||
|
||||
static TraceReplayRequest from(Intent intent, File filesDir, String defaultBackend) {
|
||||
@@ -351,7 +358,8 @@ public final class TraceReplayActivity extends Activity {
|
||||
intent.getBooleanExtra("benchmark", false),
|
||||
intent.getIntExtra("benchmark_tail_frames", 200),
|
||||
intent.getBooleanExtra("benchmark_finish", true),
|
||||
benchmarkResultPath
|
||||
benchmarkResultPath,
|
||||
readString(intent, "mobilegl_env", "")
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user