RUM-18135: Add a cold-start benchmarking harness and methodology guide - #3749
RUM-18135: Add a cold-start benchmarking harness and methodology guide#3749Valpertui wants to merge 3 commits into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 652dbe8 | Docs | View more details | Give us feedback! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcc1de7649
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dcc1de7 to
5e229ec
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e229ec8a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5e229ec to
597ffb3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 597ffb303b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
597ffb3 to
4dc3290
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dc32909e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Measuring what the SDK costs at app startup is easy to get wrong in ways that
produce a confident number rather than an obviously broken one. Running the same
APK in both arms on a mid-range device, an uncontrolled protocol reported
+12.7 ms, 95% CI [+3.0, +22.5], p = 0.011 — a significant regression between two
identical builds. Every control here removes a failure mode that produced a
wrong number, not one we imagined.
verify_sdk_active.sh step zero: installs, md5-attests the install, launches
via the real launcher intent and proves Datadog actually
initialized. A build that CONTAINS the SDK is not
necessarily one that INITIALIZES it, and an arm that was
inert invalidates everything measured against it. The
oracle is the `datadog-*` thread CoreFeature.initialize()
always creates and R8 cannot rename.
coldstart_bench.sh the A/B. ABBA-counterbalanced blocks, md5 attestation per
install, pre-granted runtime permissions, pre-registered
warm-up discard, and a per-launch assertion that the
launch was COLD and the app owned the foreground. Aborts
rather than recording a sample it cannot vouch for.
ab_stats.py statistics. The primary endpoint is a paired test over
per-block deltas, because launches inside one arm×block
cell share an install, an AOT compilation and a thermal
state; pooling them estimates the SE from within-cell
scatter only. The Welch figures are printed as
[diagnostic] for contrast.
fp_simulation.py reproduces the false-positive table that justifies that
choice, driving ab_stats.py's own interval code so the
published numbers cannot drift from the tool.
lib.sh shared helpers: resolves adb/aapt2 without depending on
PATH, refuses to measure a locked device, and matches
both `mResumedActivity` and the bare `ResumedActivity:`
some vendors print.
Both device-touching scripts snapshot every setting they change and restore it
from an EXIT trap, revoke only the permission grants they made themselves, and
treat Ctrl-C as "stop and restore" rather than "restore and keep going".
Customers ask what the SDK costs at startup. The honest answer depends on their
app, their device mix and their feature set, so the useful thing to publish is
not a number but a method they can run themselves — and the caveats that decide
whether the number it produces means anything.
docs/benchmarking_sdk_cold_start.md walks through it: build release-configured
APKs, use a physical device, prove the SDK initializes, validate the protocol
with an A/A run, then measure. It states what the harness measures (a
process-cold, page-cache-warm start to first frame) and tabulates which controls
bias the result and in which direction — most of them downward, so the figure is
closer to a lower bound than a worst case.
It also documents what the exercise taught us that contradicts the intuition:
* TTID is the wrong endpoint on framework apps. On one React Native app first
frame was under a third of startup, so a TTID-only comparison could not have
seen SDK cost in the other two thirds.
* Session Replay, the SDK's heaviest feature while recording, added ~9 ms at
startup — inside the noise. The cost was in the core SDK.
* The largest block of SDK CPU we measured, ~149 ms of JIT (~390 ms with
Session Replay), turned out not to be a startup cost at all: zero of it ran
before reportFullyDrawn(). Locate work relative to your endpoint before
attributing it.
* Traces tell you what work exists and where. They do not give a second
opinion on magnitude, and tracing itself lengthened the measured window by
~7% in every arm.
Linked from the root README next to the other troubleshooting guides. The
existing sdk_performance.md figures are explicitly marked as predating this
protocol so they are not mistaken for a comparable baseline.
Same protocol as the guide, written for an agent driving the investigation rather than a human reading it end to end: ordered steps, hard gates, and the measured cost of skipping each control. It exists because the failure mode this work exposed is an agent (or a person) producing a plausible number from a broken protocol and reporting it. The skill front-loads the two gates that catch that — prove the SDK initializes, then A/A the protocol — and refuses to treat an A/B as meaningful until both pass. Also carries the attribution rules that are easy to get backwards: what each verify_trace.py exit code licenses you to conclude, why a stopped app still passes a liveness check, and why work has to be timestamped against the measurement endpoint before it can be called a startup cost. Ships in the repository so a clone picks it up, and makes no assumption about whether the app being measured is ours or a customer's.
4dc3290 to
652dbe8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 652dbe8e37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if args.expect_absent: | ||
| if active: | ||
| print(" VERDICT: FAIL — baseline arm, but Datadog IS active in this trace.") | ||
| return 1 | ||
| print(" VERDICT: PASS — baseline arm, no Datadog activity (as expected).") | ||
| return 0 |
There was a problem hiding this comment.
Run foreground validation for baseline traces
When capture_trace.sh verifies a baseline, it passes both --expect-absent and --require-foreground, but this successful early return occurs before the lifecycle query and therefore bypasses the whole-window foreground check. A permission or system activity can take over and return before the shell's final TOP snapshot, and the contaminated baseline trace will still be accepted. The fresh evidence beyond the earlier foreground finding is this baseline-only return path in the current verifier; defer the absent-arm success verdict until after foreground validation.
Useful? React with 👍 / 👎.
| elif fg_verdict == "unknown" and args.require_foreground: | ||
| print(" NOTE: no performResume slice for this process, so foreground ownership" | ||
| " could not be established from the trace.") | ||
| return 0 |
There was a problem hiding this comment.
Fail when foreground ownership cannot be established
When a trace contains bindApplication but no performResume slice—for example because the vendor's lifecycle instrumentation is unavailable—fg_verdict remains unknown; even with --require-foreground, this branch only prints a note and then returns success. This accepts a trace for which the promised whole-window ownership check could not run, so capture_trace.sh may treat an incomparable capture as verified. Return exit 4 for the unknown case as well.
Useful? React with 👍 / 👎.
| if [ "$want" = 1 ]; then | ||
| case "$w" in 1) echo "FATAL: AIRPLANE=1 but Wi-Fi is still on ('svc wifi disable' had no" >&2 | ||
| echo " effect -- it needs root on many devices). Refusing to record a run" >&2 | ||
| echo " stamped airplane=1 that was not offline. Turn Wi-Fi off by hand." >&2 | ||
| return 1 ;; esac |
There was a problem hiding this comment.
Reject indeterminate offline radio readbacks
When settings get global wifi_on or mobile_data returns null, empty output, or another unsupported value, these checks reject only the literal value 1 and then announce that the radios were verified off. Thus a failed svc command on a device without readable settings can still produce a CSV stamped airplane=1 while a radio remains enabled. The fresh evidence beyond the earlier network-state comment is that the current readback gate still accepts every value except 1; require both values to be exactly 0 before proceeding.
Useful? React with 👍 / 👎.
| _MUST_MATCH = ("fp", "emulator", "compile_filter", "animations", "airplane", "abi", | ||
| "launcher") |
There was a problem hiding this comment.
Match arm build identities before pooling CSVs
When CSVs come from successive APK pairs on the same device with the same protocol and launcher, every field in this compatibility list can match even though the baseline or treatment binary changed, because the CSV header records neither arm's APK digest nor version. ab_stats.py then pools block deltas from different experiments into one interval without requiring --allow-mixed. The fresh evidence beyond the earlier metadata-parity findings is the absence of arm build identities from both the current header and _MUST_MATCH; stamp and compare the attested hashes or equivalent build identifiers.
Useful? React with 👍 / 👎.
| # settle: 3 discarded launches so dex/oat caches and any first-run migrations | ||
| # are done, then verify Datadog liveness on the settled state. | ||
| for _ in 1 2 3; do | ||
| "$ADB" shell am force-stop "$PKG"; sleep 3 | ||
| "$ADB" shell am start -W "${START_ARGS[@]}" >/dev/null 2>&1; sleep 8 | ||
| done |
There was a problem hiding this comment.
Match the benchmark's pre-measure launch count
With the default WARMUP=3, each benchmark cell performs one liveness-probe launch and then three warm-ups before its first measured launch, whereas trace capture performs only these three settle launches, making the traced launch the fourth post-install start rather than the benchmark's fifth. This matters especially with the default fresh-install speed-profile condition, where the scripts themselves note that no AOT profile exists and JIT/cache state evolves over early launches. Reproduce the probe-plus-warm-up count, preferably using an explicit setting shared with the benchmark, so the trace represents the state of a measured launch.
Useful? React with 👍 / 👎.
| # The permission/ANR dialog contamination this guards against ACCUMULATES, so | ||
| # sampling it once per arm cannot see it. Record per launch instead. | ||
| case "$(dd_top_activity)" in "$PKG"/*) fg=ok ;; *) fg=OTHER ;; esac |
There was a problem hiding this comment.
Observe foreground ownership throughout each benchmark launch
When another activity temporarily takes the foreground during the six-second post-launch collection window and returns before this single snapshot, the row is recorded with foreground=ok even though the app was paused for part of the measurement scenario. This can silently admit permission, authentication, or system-activity contamination into ttfd and app_trace_ms, unlike the trace path's intended whole-window validation. Poll ownership or inspect lifecycle transitions over the collection window rather than checking only its final state.
Useful? React with 👍 / 👎.
| if [ -n "$APP_TRACE_REGEX" ]; then | ||
| app_tr=$(printf '%s\n' "$lg" | grep -m1 -oE "$APP_TRACE_REGEX" \ | ||
| | grep -oE '[0-9]+' | tail -1) || true |
There was a problem hiding this comment.
Validate the host-app metric regex before starting the run
When APP_TRACE_REGEX is malformed, such as an unclosed bracket expression, grep exits with a regex error on every launch but the surrounding || true converts each failure into an empty metric. The hour-long benchmark therefore completes with app_trace_ms=NA throughout, and only subsequent analysis reveals that the explicitly requested endpoint is unusable. Compile-test a non-empty regex during preflight and abort immediately on grep's error status.
Useful? React with 👍 / 👎.
What does this PR do?
Adds a cold-start benchmarking harness, a customer-facing methodology guide, and an agent skill — the tooling and the written contract for answering "how much does the SDK cost at app startup?" on a given app and device.
Three commits, purely additive (12 new files, no SDK code touched):
RUM-18135: Add the cold-start A/B benchmark harnesstools/coldstart-benchmark/— the scripts and the statisticsRUM-18135: Document how to measure the SDK's cold-start impactdocs/benchmarking_sdk_cold_start.md, linked from the root READMERUM-18135: Add a coldstart-benchmark skill for coding agents.claude/skills/coldstart-benchmark/SKILL.mdThe harness:
verify_sdk_active.sh— step zero. Installs, md5-attests the install against the local file, launches via the real launcher intent, and provesDatadog.initialize()actually ran. The oracle is thedatadog-*threadCoreFeature.initialize()always creates (setupExecutors()then an immediate NTP-sync submit,CoreFeature.kt:265-266) and that R8 cannot rename, because the name is built at runtime from a string template.coldstart_bench.sh— the A/B (or A/A). ABBA-counterbalanced blocks, md5 attestation per install, pre-granted runtime permissions, a pre-registered warm-up discard, and a per-launch assertion that the launch wasCOLDwith the app in the foreground. Aborts rather than recording a sample it cannot vouch for.ab_stats.py— statistics. The primary endpoint is a paired test over per-block deltas.fp_simulation.py— reproduces the false-positive table that justifies that choice.capture_trace.sh/verify_trace.py— optional Perfetto attribution, with a liveness gate and a three-way verdict.lib.sh— shared helpers; resolvesadb/aapt2without depending onPATH.Motivation
Customers ask what the SDK costs at startup. The honest answer depends on their app, their device mix and their feature set, so the useful thing to publish is not a number but a method they can run — plus the caveats that decide whether the number it produces means anything.
The naive protocol does not work. Running the same APK in both arms, so the true difference is zero by construction, on a mid-range device (Helio G95, Android 12):
The first row is a statistically significant regression between two identical builds. Anyone running that protocol would have reported it.
Two design decisions are worth flagging for review:
The primary endpoint is paired on blocks, not pooled over launches. Launches inside one arm×block cell share an install, an AOT compilation and a thermal state, so an unpaired test estimates the standard error from within-cell scatter only. Counterbalancing removes the ordering bias; it does nothing about this. By simulation (true effect zero, within-launch sd 11 ms, per-cell shift sd σ_b), false-positive rate of a nominal-95% interval:
A 4 ms between-block shift is ordinary.
./fp_simulation.pyreproduces this table usingab_stats.py's own interval code, so it cannot drift away from the tool.Most controls bias the measured cost downward, and the guide says so. Forced AOT, discarded warm-ups,
am force-stop(which does not evict the page cache), TTID-only measurement and pre-granted permissions all shrink the number.docs/benchmarking_sdk_cold_start.mdtabulates the direction of each. The metric is a process-cold, page-cache-warm start to first frame, and is closer to a lower bound than a worst case.Additional Notes
Findings the guide documents that contradict the intuition — each measured, not assumed:
com.datadog.*JIT (~390 ms with Session Replay), across ten traces. Timestamped: zero of it began beforereportFullyDrawn(); the first started 406–500 ms after, in all ten traces. It is post-launch background CPU. A Baseline Profile would not have moved TTID or TTFD on that app.On
docs/sdk_performance.md: the guide explicitly marks those figures as predating this protocol (5 launches per arm, fixed order, no A/A, no confidence intervals) so they are not mistaken for a comparable baseline. No numbers there are changed by this PR.Safety.
coldstart_bench.shandcapture_trace.shuninstall/reinstall the app under test and pre-grant its runtime permissions. Both snapshot every device setting they touch and restore it from anEXITtrap;INT/TERMexit into that trap, so Ctrl-C stops the run and restores the device, once. Both revoke only the grants they made themselves — never a device-widepm reset-permissions. A preflight refuses to run whenPKGdoes not match the APKs (every block runsadb uninstall $PKG) or when the two arms declare differentversionCode/versionName..gitignorecoversresults_*.csv,bench_*.logand*.pftrace, since a Perfetto capture is device-wide.No CHANGELOG entry — tooling and docs only; no shipped SDK code changes.
How to validate
Static analysis (from
tools/coldstart-benchmark/):End-to-end, against a device or emulator:
An A/A run must produce a mean block delta near zero with a CI straddling it, and no significant order effect.
Review checklist (to be filled by reviewers)