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

[MG_Util, MG_IntegrationTest, CI] (P6): the readers the role split left behind

Browse Source 
The per-role log split renamed BOTH halves so that an un-updated reader would
fail loudly instead of silently reading half a session. It worked: CI found
four readers this way. This is them.

- MG_Util/Debug/Log.h: the role vocabulary now exists in the PULL build too,
  header-only. It was wholly inside #if MOBILEGL_BUILD_DISAGGREGATED, so
  PipeInputsTest - which builds in the verify flavour - could not name
  ReadRoleLogs and took the whole build-linux-verify job down at compile time.
  Inline rather than out-of-line because of G1: nothing in the library calls
  these, so an inline definition emits code into the readers and NOTHING into
  libMobileGL.so. Verified: pull symbols 30570 -> 30570, 0 added / 0 removed,
  .text still 0xa52203, and zero role-log symbols in the pull library.

- PipeStatsWindow.h: the harness copy of the naming rule is now flavour-aware.
  It derived `.client` in BOTH flavours, so in the verify build - one role,
  MOBILEGL_LOG_FILE_PATH written unchanged - it opened a file that never
  existed. The copy has to stay a copy: MG_Util's RoleLogPath is `t` (local)
  in the shipping .so this module links, so calling it does not link.
  Added LogMark/MarkLaneLog/ReadLaneLogSince: one offset PER ROLE, because a
  single scalar over the concatenation slides by whatever the other role wrote.

- The four arming scenarios: they asserted on lines the BACKEND emits, and
  under inproc the backend runs on the apply thread - the server role - so the
  line lands in the server's log. Reading only the client's half reported the
  emulation unarmed, i.e. accused the product of a defect the reader invented.
  Four copies of the same private helper pair now delegate to the harness.

- scripts/ci/magma_runahead_checks.py: it unlinked and read the base name as
  if it were a file, so the negative arm saw an empty log and could not prove
  the knob took. It now imports split_log_paths' derivation - the authoritative
  one - rather than becoming a third copy.

Local verification (WSL, lk-split + c6-pull):
  - the 4 CI-red entries: 10/10 PASS under inproc, 10/10 under monolith
  - unit role-log readers: 510/510 PASS
  - magma_runahead_checks negative: run_ahead_0 EXPECTED_RED, restored GREEN
  - the 5 affected TUs compile clean with MOBILEGL_BUILD_DISAGGREGATED=0
  - G1: 0/0/0/0, .text 0xa52203

Red-once is the CI run itself (35677991453), which failed for each of these
reasons before the change.
This commit is contained in:
swung0x48
2026-09-22 00:37:39 -04:00
parent 75dd0cb1ca
commit 94c130d639
7 changed files with 191 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_IntegrationTest/Harness/PipeStatsWindow.h
+74 -9
View File
@@ -29,6 +29,7 @@
#pragma once
#include <cstdint>
#include <cstdlib>
#include <cstring>
#include <fstream>
@@ -41,13 +42,20 @@ namespace MGITest::PipeStatsWindow {
// ROLE - `<stem>.client<ext>` and `<stem>.server<ext>` - because under inproc both roles are
// threads of one process and a single file made every per-side assertion a search.
//
// THE SUFFIX IS DERIVED HERE AND NOWHERE ELSE, matching MG_Util/Debug/Log.cpp's
// RoleLogPathFor. Two copies of a naming rule is one copy too many: the one that is not
// updated opens a file that does not exist and reports "the library never logged", which
// reads as a product failure rather than as a stale path.
// THE RULE IS COPIED HERE AND IT HAS TO BE, which is worth stating because the obvious fix is
// to call the library's own MG_Util::Debug::RoleLogPath and delete this. That does not link:
// this module links the SHIPPING libMobileGL.so, built -fvisibility=hidden, and the role-path
// helpers are `t` (local) in it - only MG_Test, which links the static archive, may call them.
//
// SO THE COPY IS FLAVOUR-AWARE INSTEAD. That is the part the first copy got wrong: it derived
// `.client` in BOTH flavours, while the pull build has one role and writes
// MOBILEGL_LOG_FILE_PATH unchanged. In the verify lane it therefore opened a file that never
// existed and every reader reported "the library never logged" - a product failure that had
// not happened. Keep this `#if` and Log.cpp's RoleLogPath in step.
inline std::string RoleLogPath(const char* roleSuffix) {
const char* base = std::getenv("MOBILEGL_LOG_FILE_PATH");
if (base == nullptr || *base == '\0') return {};
#if MOBILEGL_BUILD_DISAGGREGATED
std::string path(base);
const std::string::size_type slash = path.find_last_of("/\\");
const std::string::size_type dot = path.find_last_of('.');
@@ -55,15 +63,27 @@ namespace MGITest::PipeStatsWindow {
return path + "." + roleSuffix;
}
return path.substr(0, dot) + "." + roleSuffix + path.substr(dot);
#else
(void)roleSuffix;
return std::string(base);
#endif
}
// The lane's private CLIENT log path, or empty when the lane configured none. Every existing
// caller wants this one: they read markers the CLIENT emits - the transport resolution line,
// PipeStats summaries, the arming diagnostics.
// The lane's private CLIENT log path, or empty when the lane configured none. In the pull
// build this IS the whole log.
inline std::string LibraryLogPath() { return RoleLogPath("client"); }
// The server role's half, for a caller that wants a line only the applier writes.
inline std::string ServerLibraryLogPath() { return RoleLogPath("server"); }
// The server role's half, or EMPTY when this build has no separate server half. Empty rather
// than "the same path again": every caller below concatenates the two, and a pull build that
// named one file twice would show each line twice - which reads as a doubled counter or a
// repeated diagnostic rather than as a path mistake.
inline std::string ServerLibraryLogPath() {
#if MOBILEGL_BUILD_DISAGGREGATED
return RoleLogPath("server");
#else
return {};
#endif
}
inline std::string ReadWholeFile(const std::string& path) {
if (path.empty()) return {};
@@ -72,6 +92,51 @@ namespace MGITest::PipeStatsWindow {
return std::string((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>());
}
inline std::string ReadFileSince(const std::string& path, std::uintmax_t offset) {
if (path.empty()) return {};
std::ifstream file(path, std::ios::binary);
if (!file.good()) return {};
file.seekg(static_cast<std::streamoff>(offset));
return std::string((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>());
}
// A "the log is this long right now" snapshot, ONE OFFSET PER ROLE.
//
// The scenarios that assert on arming take a mark before the workload and read what was
// appended after it, so that only bytes this case caused can satisfy - or refute - the claim.
// The roles are separate files, so this cannot be one scalar: a single offset applied to the
// concatenation would slide by however much the OTHER role happened to write, and the read
// would start mid-line in the wrong file.
struct LogMark {
std::uintmax_t client = 0;
std::uintmax_t server = 0;
};
inline std::uintmax_t FileSizeOrZero(const std::string& path) {
if (path.empty()) return 0;
std::ifstream file(path, std::ios::binary | std::ios::ate);
if (!file.good()) return 0;
const std::streamoff size = file.tellg();
return size < 0 ? 0 : static_cast<std::uintmax_t>(size);
}
inline LogMark MarkLaneLog() {
return LogMark{FileSizeOrZero(LibraryLogPath()), FileSizeOrZero(ServerLibraryLogPath())};
}
// BOTH ROLES, and for the arming diagnostics the SERVER's is the one that matters. Those lines
// are emitted by the BACKEND - "demoted to an ordinary varying", the reroute and emulation
// banners - and under inproc the backend runs on the apply thread, which is the server role.
// Reading only the client's half finds nothing and reports "the emulation is not armed",
// which is the most expensive possible way to be wrong: it accuses the product of a defect
// that the reader itself invented.
inline std::string ReadLaneLogSince(const LogMark& mark) {
return ReadFileSince(LibraryLogPath(), mark.client)
+ ReadFileSince(ServerLibraryLogPath(), mark.server);
}
inline std::string ReadLaneLog() { return ReadLaneLogSince(LogMark{}); }
// The last summary line in the log, verbatim. `found` is false when the library never emitted
// one, which is a different failure from "the counter read zero" and has to be reported as
// one: it means the stats channel never reached the process, not that the workload did
MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp
+15 -16
View File
@@ -111,24 +111,23 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); }
return std::filesystem::path(MGITest::PipeStatsWindow::LibraryLogPath());
}
static std::uintmax_t LibraryLogSize() {
std::error_code ec;
const std::filesystem::path path = LibraryLogPath();
if (path.empty()) return 0;
const std::uintmax_t size = std::filesystem::file_size(path, ec);
return ec ? 0 : size;
// ONE MARK PER ROLE. The library writes a log per role, so "how long is the log
// right now" is two numbers; a single scalar applied to the concatenation would slide
// by whatever the other role wrote in between and start the read mid-line.
static MGITest::PipeStatsWindow::LogMark LibraryLogMark() {
return MGITest::PipeStatsWindow::MarkLaneLog();
}
static std::string LibraryLogSince(std::uintmax_t offset) {
const std::filesystem::path path = LibraryLogPath();
if (path.empty()) return {};
std::ifstream file(path, std::ios::binary);
if (!file.good()) return {};
file.seekg(static_cast<std::streamoff>(offset));
return std::string((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>());
// BOTH ROLES. The arming diagnostics this case looks for are emitted by the
// BACKEND, and under inproc the backend runs on the apply thread - the server role -
// so the line lands in the server's log. Reading only the client's found nothing and
// reported the emulation unarmed, which accused the product of a defect the reader
// had invented.
static std::string LibraryLogSince(const MGITest::PipeStatsWindow::LogMark& mark) {
return MGITest::PipeStatsWindow::ReadLaneLogSince(mark);
}
static std::string LibraryLog() { return LibraryLogSince(0); }
static std::string LibraryLog() { return MGITest::PipeStatsWindow::ReadLaneLog(); }
// One frame that crosses several verb boundaries: a clear (kClear), a draw (kDraw) and
// a readback (kReadback). Three of the nine fill classes, so an entry compare that only
@@ -201,7 +200,7 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); }
"backwards; the VerifyCorrupted. lane owns that half";
}
const std::uintmax_t before = LibraryLogSize();
const MGITest::PipeStatsWindow::LogMark before = LibraryLogMark();
ASSERT_NO_FATAL_FAILURE(DrawOneFrame());
EXPECT_EQ(FirstGLError(), 0u);
@@ -252,7 +251,7 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); }
}
const std::string knob = std::getenv("MOBILEGL_PIPE_VERIFY_CORRUPT");
const std::uintmax_t before = LibraryLogSize();
const MGITest::PipeStatsWindow::LogMark before = LibraryLogMark();
ASSERT_NO_FATAL_FAILURE(DrawOneFrame());
const std::string appended = LibraryLogSince(before);
MobileGL/MG_IntegrationTest/Scenarios/PointSizeDemotionScenario.cpp
+13 -15
View File
@@ -311,22 +311,20 @@ void main()
return std::filesystem::path(MGITest::PipeStatsWindow::LibraryLogPath());
}
static std::uintmax_t LibraryLogSize() {
std::error_code ec;
const std::filesystem::path path = LibraryLogPath();
if (path.empty()) return 0;
const std::uintmax_t size = std::filesystem::file_size(path, ec);
return ec ? 0 : size;
// ONE MARK PER ROLE. The library writes a log per role, so "how long is the log
// right now" is two numbers; a single scalar applied to the concatenation would slide
// by whatever the other role wrote in between and start the read mid-line.
static MGITest::PipeStatsWindow::LogMark LibraryLogMark() {
return MGITest::PipeStatsWindow::MarkLaneLog();
}
static std::string LibraryLogSince(std::uintmax_t offset) {
const std::filesystem::path path = LibraryLogPath();
if (path.empty()) return {};
std::ifstream file(path, std::ios::binary);
if (!file.good()) return {};
file.seekg(static_cast<std::streamoff>(offset));
return std::string((std::istreambuf_iterator<char>(file)),
std::istreambuf_iterator<char>());
// BOTH ROLES. The arming diagnostics this case looks for are emitted by the
// BACKEND, and under inproc the backend runs on the apply thread - the server role -
// so the line lands in the server's log. Reading only the client's found nothing and
// reported the emulation unarmed, which accused the product of a defect the reader
// had invented.
static std::string LibraryLogSince(const MGITest::PipeStatsWindow::LogMark& mark) {
return MGITest::PipeStatsWindow::ReadLaneLogSince(mark);
}
std::string m_buildLog;
@@ -494,7 +492,7 @@ void main()
// Taken BEFORE the program is built, so the line this looks for can only be one
// this process wrote.
const std::uintmax_t before = LibraryLogSize();
const MGITest::PipeStatsWindow::LogMark before = LibraryLogMark();
const GLuint program = BuildCaptureProgram({{GL_VERTEX_SHADER, kPointVertexSource},
{GL_GEOMETRY_SHADER, kPointGeometrySource},
MobileGL/MG_IntegrationTest/Scenarios/PrimitivesGeneratedNoXfbScenario.cpp
+13 -15
View File
@@ -256,22 +256,20 @@ void main() {
return std::filesystem::path(MGITest::PipeStatsWindow::LibraryLogPath());
}
static std::uintmax_t LibraryLogSize() {
std::error_code ec;
const std::filesystem::path path = LibraryLogPath();
if (path.empty()) return 0;
const std::uintmax_t size = std::filesystem::file_size(path, ec);
return ec ? 0 : size;
// ONE MARK PER ROLE. The library writes a log per role, so "how long is the log
// right now" is two numbers; a single scalar applied to the concatenation would slide
// by whatever the other role wrote in between and start the read mid-line.
static MGITest::PipeStatsWindow::LogMark LibraryLogMark() {
return MGITest::PipeStatsWindow::MarkLaneLog();
}
static std::string LibraryLogSince(std::uintmax_t offset) {
const std::filesystem::path path = LibraryLogPath();
if (path.empty()) return {};
std::ifstream file(path, std::ios::binary);
if (!file.good()) return {};
file.seekg(static_cast<std::streamoff>(offset));
return std::string((std::istreambuf_iterator<char>(file)),
std::istreambuf_iterator<char>());
// BOTH ROLES. The arming diagnostics this case looks for are emitted by the
// BACKEND, and under inproc the backend runs on the apply thread - the server role -
// so the line lands in the server's log. Reading only the client's found nothing and
// reported the emulation unarmed, which accused the product of a defect the reader
// had invented.
static std::string LibraryLogSince(const MGITest::PipeStatsWindow::LogMark& mark) {
return MGITest::PipeStatsWindow::ReadLaneLogSince(mark);
}
GLuint m_vao = 0;
@@ -536,7 +534,7 @@ void main() {
// Taken BEFORE the draw, so the line this looks for can only be one this
// process wrote for this span. The latch fires on the FIRST rerouted
// draw, which is inside the query below.
const std::uintmax_t before = LibraryLogSize();
const MGITest::PipeStatsWindow::LogMark before = LibraryLogMark();
const GLuint generated = QueryGenerated([]() { glDrawArrays(GL_TRIANGLES, 0, 3); });
EXPECT_EQ(DrainGLErrors(), 0u);
EXPECT_EQ(generated, 1u) << "the pinned-on lane did not even count correctly";
MobileGL/MG_IntegrationTest/Scenarios/UnlocatedIoBlockScenario.cpp
+13 -15
View File
@@ -364,22 +364,20 @@ void main()
// is searched from here forward, because the file is APPENDED to by every process
// in the lane and a line left behind by an earlier one would otherwise satisfy the
// assertion without this process having done anything at all.
static std::uintmax_t LibraryLogSize() {
std::error_code ec;
const std::filesystem::path path = LibraryLogPath();
if (path.empty()) return 0;
const std::uintmax_t size = std::filesystem::file_size(path, ec);
return ec ? 0 : size;
// ONE MARK PER ROLE. The library writes a log per role, so "how long is the log
// right now" is two numbers; a single scalar applied to the concatenation would slide
// by whatever the other role wrote in between and start the read mid-line.
static MGITest::PipeStatsWindow::LogMark LibraryLogMark() {
return MGITest::PipeStatsWindow::MarkLaneLog();
}
static std::string LibraryLogSince(std::uintmax_t offset) {
const std::filesystem::path path = LibraryLogPath();
if (path.empty()) return {};
std::ifstream file(path, std::ios::binary);
if (!file.good()) return {};
file.seekg(static_cast<std::streamoff>(offset));
return std::string((std::istreambuf_iterator<char>(file)),
std::istreambuf_iterator<char>());
// BOTH ROLES. The arming diagnostics this case looks for are emitted by the
// BACKEND, and under inproc the backend runs on the apply thread - the server role -
// so the line lands in the server's log. Reading only the client's found nothing and
// reported the emulation unarmed, which accused the product of a defect the reader
// had invented.
static std::string LibraryLogSince(const MGITest::PipeStatsWindow::LogMark& mark) {
return MGITest::PipeStatsWindow::ReadLaneLogSince(mark);
}
static GLenum FirstGLError() {
@@ -500,7 +498,7 @@ void main()
// Taken BEFORE the program is built, so the line this looks for can only be one
// this process wrote. The latch means it is emitted at the FIRST stage of the
// FIRST affected program, which is inside the build below.
const std::uintmax_t before = LibraryLogSize();
const MGITest::PipeStatsWindow::LogMark before = LibraryLogMark();
const GLuint program = BuildPipeline(kDistinctTessEvalSource, kDistinctGeometrySource);
if (program == 0) {
MobileGL/MG_Util/Debug/Log.h
+44
View File
@@ -122,6 +122,7 @@
{}
#endif
#include <cstdio>
#include <string>
namespace MobileGL {
@@ -173,6 +174,49 @@ namespace MobileGL {
// these readers exist to check.
std::string ReadRoleLogs(const char* basePath);
void TruncateRoleLogs(const char* basePath);
#else
// THE SAME VOCABULARY IN THE PULL BUILD, where there is exactly one log because there
// is exactly one role. A reader that asks for "what this run logged" has to compile
// and mean the right thing in BOTH flavours; the alternative is an `#if` at every
// call site, and the call site that forgets one is the one that silently reads
// nothing. (It was not hypothetical: PipeInputsTest built only in the verify flavour
// and took the whole `build-linux-verify` job down with it.)
//
// HEADER-ONLY ON PURPOSE, FOR G1. The pull library's symbol set and `.text` must stay
// byte-identical to the baseline, so these may not become library symbols. Nothing
// inside the library calls them - only tests and harnesses do - so an inline
// definition emits code into those readers and nothing at all into libMobileGL.so.
enum class LogRole { Client, Server };
// No roles to separate, so nothing to record. Present so that a harness may say which
// role a thread is without asking which build it is in.
inline void SetThreadLogRole(LogRole) {}
// One role, one file: the pull build writes MOBILEGL_LOG_FILE_PATH unchanged.
inline std::string RoleLogPath(const char* basePath, LogRole) {
return basePath == nullptr ? std::string() : std::string(basePath);
}
inline std::string ReadRoleLogs(const char* basePath) {
std::string all;
if (basePath == nullptr) return all;
if (FILE* file = std::fopen(basePath, "rb")) {
char chunk[4096];
std::size_t got = 0;
while ((got = std::fread(chunk, 1, sizeof(chunk), file)) > 0) {
all.append(chunk, got);
}
std::fclose(file);
}
return all;
}
inline void TruncateRoleLogs(const char* basePath) {
if (basePath == nullptr) return;
if (FILE* file = std::fopen(basePath, "w")) {
std::fclose(file);
}
}
#endif
} // namespace Debug
} // namespace MG_Util
scripts/ci/magma_runahead_checks.py
+19 -3
View File
@@ -19,6 +19,15 @@ import sys
import time
import xml.etree.ElementTree as ET
# P6: MOBILEGL_LOG_FILE_PATH IS A BASE NAME and the library writes one file per ROLE, so no file
# of the literal name exists. The derivation is IMPORTED, not copied: a third copy of the rule is
# a third thing that can fall behind, and the one that does reads an empty log and reports "the
# runtime knob did not take" - a product failure that never happened. (It did happen: this check
# went red on the role split for exactly that reason.)
sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "MobileGL" / "MG_IntegrationTest"
/ "Harness"))
from split_log_paths import any_role_file, read_role_logs, role_paths # noqa: E402
CASES = (
"QueuedBufferVersionsAndDeletedNameReuseKeepTheirPixels",
"QueuedProgramRebindsKeepEachUniformSnapshot",
@@ -98,7 +107,10 @@ def run_case(test, build, directory, overrides):
private = (cwd / private).resolve() if not private.is_absolute() else private.resolve()
if not private.is_relative_to(build):
raise RuntimeError(f"refusing to reset a private log outside the selected build: {private}")
private.unlink(missing_ok=True)
# Every role's file, because a stale server half would otherwise survive into this arm and
# its lines would read as evidence about the knob this run set.
for role_path in role_paths(private):
Path(role_path).unlink(missing_ok=True)
xml = directory / "gtest.xml"
command = [arg for arg in command if not arg.startswith("--gtest_output=")]
command.append(f"--gtest_output=xml:{xml}")
@@ -119,10 +131,14 @@ def run_case(test, build, directory, overrides):
except subprocess.TimeoutExpired:
rc = 124
output.write("\nRUNNER_TIMEOUT: process killed after discovered CTest timeout\n")
private_text = private.read_text(encoding="utf-8", errors="replace") if private.is_file() else ""
# BOTH ROLES, concatenated into the one evidence file this run archives. `Config: IPC` is the
# client's line and the queue-lead diagnostics are the applier's, so either half alone would
# answer one of the two questions the negative arm asks and be silent on the other.
private_text = read_role_logs(private)
(directory / "private.log").write_text(private_text, encoding="utf-8")
result = {"ctest_name": test["name"], "returncode": rc,
"seconds": round(time.monotonic()-started, 3), "private_log_present": private.is_file()}
"seconds": round(time.monotonic()-started, 3),
"private_log_present": any_role_file(private)}
write_json(directory / "process.json", result)
return result