diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index efc8aaf3..70961568 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -551,12 +551,15 @@ jobs: # The push-only unit tests, on the verify runtime. # - # WHY HERE AND NOT IN `test`. The `test` job builds the PULL library, and G6's chunk-table - # walk and G10's residual assertions live in MG_Test/Pipe, compiled only under - # MOBILEGL_PIPE_PUSH (MGPipeRenderStateSpans.cpp and PipeApply.cpp are appended to - # SOURCE_FILES inside the `if (MOBILEGL_PIPE_PUSH)` block, which is exactly how the pull - # build stays symbol-identical). So before P2 those tests ran in no CI job at all: they - # existed, they were green locally, and CI never executed one of them. + # WHY HERE AND NOT IN `test`. The entries themselves are registered in EVERY build - they + # have to be, or `ctest -N` would stop matching name-for-name between the pull and the push + # build (gate G2). What is push-only is what they assert about: MGPipeRenderStateSpans.cpp + # and PipeApply.cpp are appended to SOURCE_FILES inside the `if (MOBILEGL_PIPE_PUSH)` block, + # which is exactly how the pull build stays symbol-identical, so in a pull build each case + # opens with `#if !MOBILEGL_PIPE_PUSH GTEST_SKIP() << "push not compiled in"`. The `test` + # job therefore runs G6's chunk-table walk and G10's residual assertions as a column of + # skips: CI executes the NAMES and never one of the assertions. This job unpacks a build + # that compiled them, so it is the first place in CI where they actually run. # # This artifact already carries them - the packaging step above tars # ${BUILD_DIR}/MobileGL/MG_Test whole - so the whole cost is the run, which is ~14 s for diff --git a/android-plugin/app/src/trace/cpp/trace_benchmark.cpp b/android-plugin/app/src/trace/cpp/trace_benchmark.cpp index fb67d0aa..973496a1 100644 --- a/android-plugin/app/src/trace/cpp/trace_benchmark.cpp +++ b/android-plugin/app/src/trace/cpp/trace_benchmark.cpp @@ -8,8 +8,14 @@ // CLOCK_THREAD_CPUTIME_ID is POSIX and present on Linux and on every Android API this replays // on; the guard exists so the desktop CLI still builds where it is not, and so that "no CPU // series" is a compile-time fact rather than a silently-zero column. +// +// , not : clock_gettime, CLOCK_THREAD_CPUTIME_ID and struct timespec are POSIX +// names, and only is required to put them at global scope - guarantees the C++ +// subset in namespace std and leaves the rest to the implementation. glibc and bionic both happen +// to provide them either way; this file is built for both by two different toolchains, so it asks +// for the header that actually promises what it uses. #if defined(__unix__) || defined(__linux__) || defined(__ANDROID__) || defined(__APPLE__) -#include +#include #define MOBILEGL_TRACE_HAVE_THREAD_CPU_CLOCK 1 #else #define MOBILEGL_TRACE_HAVE_THREAD_CPU_CLOCK 0 @@ -78,12 +84,17 @@ void Begin(bool finishEachFrame) { gFrameMs.reserve(kFrameReserve); gFrameCpuMs.clear(); gFrameCpuMs.reserve(kFrameReserve); - gLastBoundaryCpuMs = ThreadCpuMs(); gFinishEachFrame = finishEachFrame; gResolvedGlFinish = false; gGlFinish = nullptr; + // Wall baseline FIRST, CPU baseline second - the same order OnFrameBoundary reads them in, + // and for the same reason. Frame 0's CPU interval then sits strictly inside its wall interval, + // so whatever this function costs between the two readings lands in the wall number where it + // can be seen, instead of inflating the CPU number where it cannot. Taken the other way round + // (as this was), frame 0 alone reported a CPU delta biased upward against its own wall delta. gStart = Clock::now(); gLastBoundary = gStart; + gLastBoundaryCpuMs = ThreadCpuMs(); gEnabled = true; } diff --git a/tools/device_bench/README.md b/tools/device_bench/README.md index a83fcaa7..dfa92e3a 100644 --- a/tools/device_bench/README.md +++ b/tools/device_bench/README.md @@ -37,6 +37,10 @@ frequency-pin integrity. `--allow-unverified-profile` is passed, which labels the run unpinned in the warning. **A profile that omits the key entirely is refused the same way** - the guard defaults to unverified, so copying a verified profile and editing the serial cannot inherit its verdict. + Only the file can answer: both scripts reset `PROFILE_VERIFIED=0` immediately before sourcing + it, so `PROFILE_VERIFIED=1` exported in your shell does not re-open the hole. Nor does a + profile path that cannot be read get mistaken for an unverified one - it is reported as + unreadable, and a path relative to the directory you ran the script from is resolved. (`profile.sh` pins nothing - it records a simpleperf profile - so it carries no such guard.) That refusal exists because the pin path is silent when it is wrong: the harness writes diff --git a/tools/device_bench/bench.sh b/tools/device_bench/bench.sh old mode 100755 new mode 100644 index 6410ce3f..09bfbb02 --- a/tools/device_bench/bench.sh +++ b/tools/device_bench/bench.sh @@ -22,6 +22,11 @@ # Screenshots (pre/post measurement) land in results/-