|
Donner SVG Editor & Engine
SVG-native editor and embeddable SVG2 + CSS3 engine in C++20, with GPU (WebGPU) and compact CPU renderers, built for correctness, security, and performance.
|
|
|
Donner SVG Editor & Engine
SVG-native editor and embeddable SVG2 + CSS3 engine in C++20, with GPU (WebGPU) and compact CPU renderers, built for correctness, security, and performance.
|
Status: Superseded by 0031: CI Hardening 2026-Q2 (2026-04-20) Author: Claude Opus 4.7 Created: 2026-04-20
Note: The milestones in this doc are preserved for history. The active plan (combining this runtime-reduction scope with the escape-prevention scope from doc 0016) lives in 0030-2. New work should reference the milestones there.
Donner's GitHub Actions CI is the slowest part of the merge cycle: cold-cache macOS runs landed in the 20–25 minute range for the Build step alone (pre-Skia-removal). PRs trigger both Linux and macOS jobs, so PR feedback is gated on the macOS critical path. This doc plans a sequence of changes that should bring PR feedback under 10 minutes (warm) / 15 minutes (cold) without losing test coverage and without dropping macOS coverage.
The biggest single drop already landed in PR #546 (Skia removal, merged 2026-04-20). The remaining wins come from cache hygiene, runner sizing, parallelism, and moving non-blocking work off the PR critical path.
These are hard constraints that bound the solution space:
Per-step seconds, taken from gh run view. "Cold" = first run after a dep bump or long quiet period; "warm" = subsequent runs hitting cache-save: refs/heads/main.
| Run | macOS Build | macOS Test | macOS Fuzz | Linux Build | Linux Test |
|---|---|---|---|---|---|
| 24645801857 (re-push, warm) | 89s | 64s | 10s | 134s | 102s |
| 24640769418 (PR #544, cold) | 1379s | 263s | 294s | 159s | 166s |
| 24639459623 (PR #545, cold) | 1492s | 206s | 437s | 352s | 26s |
| 24623255472 (warm) | 291s | 216s | 14s | 355s | 151s |
| 24618420955 (cold) | 1250s | 200s | 289s | 95s | 133s |
| 24613404030 (cold) | 605s | 141s | 170s | 1391s | 173s |
| 24597636830 (warm) | 314s | 173s | 13s | 330s | 129s |
| Run | macOS Build | macOS Test | macOS Fuzz | Linux Build | Linux Test |
|---|---|---|---|---|---|
| 24648369948 (PR #546, cold) | 1130s | 178s | 266s | 1891s | 158s |
Surprise: Skia removal saved less than predicted. macOS Build dropped from the ~1250–1492s cold range to 1130s — about 15%, not the 70–80% I expected based on Skia's source volume. Linux Build was actually the highest cold number we've recorded (1891s vs prior max of 1391s), almost certainly because removing Skia invalidated the entire transitive cache and this run rebuilt everything from scratch.
Implication for the plan: the dominant CI cost is not one big slow dependency — it's cache eviction across configs and runner sizing. M1 (per-config cache slots, accomplished in this PR by moving fuzzers to a separate workflow) and M4 (macos-15-large) become more attractive than they looked when I assumed Skia was the silver bullet.
The next 2–3 main pushes should show whether warm-cache numbers improve now that the cache no longer alternates between default-config and asan-fuzzer toolchains in the same slot.
flowchart TD
PR[PR push] --> Lint[lint job — 60s]
PR --> LinuxFull[linux: build+test+geode — 5min warm / 8min cold]
PR --> MacOS[macos: build+test — 8min warm / 15min cold]
MainPush[Push to main] --> Lint2[lint]
MainPush --> LinuxFull2[linux full]
MainPush --> MacOS2[macos full]
MainPush --> SkiaRef[skia_ref tests — REMOVED in #546]
Nightly[nightly cron] --> Fuzz[fuzzers — moved off main pushes]
Nightly --> ExtraSanity[full --config=text-full + --config=geode matrix]
LinuxFull -.GHA disk cache, per-config slot.-> CacheLinux[(GHA cache: CI-Linux-default)]
MacOS -.GHA disk cache, per-config slot.-> CacheMacos[(GHA cache: CI-macOS-default)]
Fuzz -.dedicated cache slot.-> FuzzCache[(GHA cache: CI-macOS-asan-fuzzer)]
LinuxFull -.our PRs only.-> RemoteCache[(internal bazel-re1 remote cache)]
MacOS -.our PRs only.-> RemoteCache
LinuxFull2 -.write-through.-> RemoteCache
MacOS2 -.write-through.-> RemoteCache
PR-time gates: lint (~60s, parallel) + linux (~5–8 min) + macOS (~8–15 min). macOS remains the critical path on cold cache; the goal is to make it ≤ 15 min by sizing it up (M4) and giving it an exclusive cache slot that isn't evicted by the fuzzer step (M1).
For each milestone:
A small tools/ci_timing_report.py script that ingests gh run view ... --json jobs output and emits a markdown table would make this measurement repeatable. Optional but recommended before starting M1.
Author: Claude Opus 4.8 (CI-runtime agent) Scope: Root-cause the linux-self-hosted and coverage-self-hosted lanes that ran past 60 minutes under queue contention and were cancelled at the job timeout (PRs #808, #809). Timeouts were raised 60 -> 120 as a stopgap on affected branches by another agent; this appendix targets the root cause so the stopgap becomes unnecessary. Aligns with the project's CI build/test speed and stability goals (the P95/P99 targets below) and the shared-backend capacity follow-ups.
End-to-end PR CI wall-clock, measured from PR trigger to the last required check finishing and INCLUDING queue wait:
Primary metric is end-to-end run completion on that definition; a per-lane view is reported alongside it. Speed is the target, never coverage: no test removed, no lane skipped, no required check made optional. Governing infra principle: all build/test executes on a shared remote-execution (RE) backend; runner hosts stay thin dispatchers; solve tail latency by shifting cores to the RE backend, not by multiplying runner hosts. An interactive development host has PRIORITY on shared RE capacity: if CI and interactive development contend, interactive development wins and the cost is paid in CI tail latency, stated here rather than eroding that priority.
| Window | n (head SHAs) | median | p90 | p95 | p99 | max | >15m | >30m |
|---|---|---|---|---|---|---|---|---|
| PRE #799 (06-26..07-06) | 267 | 58.1 | 254 | 688 | 1028 | 1067 | 85% | 72% |
| POST #799 (07-06..07-10) | 43 | 15.3 | 71.0 | 270.6 | 304.0 | 306.6 | 51% | 33% |
All minutes. Gap to goal (post-#799): P95 270.6 vs 15; P99 304 vs 30. The #799 speedup fixed the median (58 -> 15.3); the entire remaining gap is tail.
| Workflow:job | n | exec med | exec p90 | exec p95 | exec max | queue med | queue p90 | queue p95 | queue max |
|---|---|---|---|---|---|---|---|---|---|
| Co:coverage-self-hosted | 31 | 2.5 | 30.4 | 48.1 | 60 | 0.8 | 1 | 239 | 270 |
| CI:linux-self-hosted | 30 | 0.0 | 30.4 | 46.5 | 60 | 0.6 | 24 | 240 | 270 |
| CI:linux (hosted) | 36 | 5.7 | 65.5 | 66.7 | 77 | 0.6 | 1 | 1 | 1 |
| Co:build (hosted) | 26 | 0.0 | 16.4 | 25.6 | 71 | 0.8 | 1 | 1 | 1 |
| CI:macos (hosted) | 36 | 6.1 | 38.8 | 45.2 | 50 | 0.6 | 1 | 1 | 1 |
| CI:macos-self-hosted | 30 | 0.0 | 3.3 | 4.1 | 5 | 0.6 | 1 | 1 | 3 |
| Co:determine-targets | 43 | 0.6 | 0.8 | 0.9 | 1 | 0.1 | 0 | 0 | 0 |
| CI:determine-targets | 43 | 0.3 | 0.8 | 0.9 | 1 | 0.2 | 1 | 1 | 1 |
| CI:gatekeeper | 43 | 0.1 | 0.2 | 0.2 | 0 | 0.1 | 0 | 0 | 1 |
exec max = 60 on both self-hosted lanes is the timeout firing, not a real completion. exec med 0.0 means the lane was gated off (skipped) on that SHA.
The self-hosted lanes are NOT a single flat backend; correcting an earlier draft of this appendix that attributed the Linux lanes to the macOS backend. The relevant structure, stated without private per-host capacity or identity:
Allocation: the shared RE backend and the priority interactive host are provisioned to overcommit the underlying shared build host, so the executor and the interactive host structurally compete for the same cores. That is exactly the interactive-vs-CI RE contention the operator flagged. The shared build host itself ran at low average load during this profiling, so the bottleneck is contention on the shared path, not a saturated host.
The three worst post-#799 PR SHAs decompose as queue 232-270 min + a 30-60 min (timeout) exec on a self-hosted lane, e2e 292-307 min. Step-level on the two 60-min cancellations (runs 29076327250, 29077015684, two different PRs started within one second at 11:24:44-45Z): checkout 11s, toolchain sync 10s, LLVM fetch 32-45s, then Generate coverage runs the full 60 min and is cancelled at the timeout. Setup is ~1 minute; 100% of the wasted hour is one coverage step that never finished.
Uncontended, an incremental coverage-self-hosted run is ~2.5 min (post-#799 median). Under concurrency it blows up ~20x (superlinear) into the timeout. Two contention points compound, neither of them raw executor cores (the executor is not core-starved):
The huge queue wait (self-hosted p95 239-240 min, max 270) is the same pathology from the front: while runner slots are held by jobs crawling for an hour, later jobs wait hours for a slot. Queue and execution are coupled through contention.
The self-hosted lanes are gated on github.actor == jwmcglynn && github.triggering_actor == jwmcglynn, so this is the operator's own concurrent PRs (and interactive development) contending, not third-party load.
A cluster of post-#799 PR SHAs sits at e2e 66-78 min with near-zero queue, exec in the hosted CI:linux (66-77 min) or hosted Co:build (71 min). These are PRs touching build-graph infra (MODULE.bazel, WORKSPACE*, .bazelrc, build_defs/*, .github/workflows/*, .github/actions/*): determine-targets correctly forces a full //..., the self-hosted lanes skip by design (gate requires fallback != 'true'), and the hosted lane runs the whole graph on GitHub's disk cache. About 19% of post-#799 PR SHAs. Separate population from the self-hosted tail; matters for P90, not the P95/P99 gap.
With lever 1 the lanes no longer oversubscribe the shared RE backend, so uncontended exec stays well under 60 min and the raise is unnecessary; the 60 min bound stays as a fast-fail on a genuinely wedged endpoint (paired with remote_local_fallback=false), matching the intended CI end-state.
Author: Claude Opus 4.8 (coverage-lane CI-runtime agent) Scope (narrowed by operator 2026-07-11): the COVERAGE lane only. Queue contention was fixed by the 2026-07-10 serialization work (PR #817 per-lane concurrency groups); the operator's new bar is per-job EXECUTION time. Target tightened 2026-07-11 to per-job execution P99 <= 15 minutes (supersedes the earlier p95<=30). A 15-minute P99 means even the worst-case coverage run must fit, so structural fixes beat shaving. Sibling agents own test-runtime trims, compile/RE throughput, and RE-host infra; this appendix touches only the coverage lane and does not propose infra or non-coverage test-content changes (those are flagged for the owning agents / the operator).
GitHub Actions REST API (gh api), window 2026-07-09..07-11 (48h), job-level started_at/completed_at (execution) and run created_at (queue). Coverage step decomposition from the lane's own diagnostics artifact (ci-diagnostics-coverage-self-hosted-*): tools/coverage.sh writes a Bazel --profile gz, a --build_event_json_file BEP, and a timing.txt with phase marks (start / bazel_coverage_done / filter_done / end). The exemplar is PR #829 (coverage-self-hosted run 29158844327, the 54-minute run the operator flagged).
| lane | n | exec med | exec p95 | exec max | note |
|---|---|---|---|---|---|
| Coverage:build (hosted; main + hosted PR fallback) | 12 | 69.7 | 74.7 | 75.2 | 9/12 over 30 min |
| Coverage:coverage-self-hosted (PR incremental) | 66 | 6.9 | 60.3 | 60.4 | 12 over 30 min; the 60.3 p95 is the 60-min timeout firing, plus one real 54.0-min completion (#829) |
| Coverage:determine-targets | 90 | 0.5 | 0.8 | 0.9 | healthy |
| Coverage:upload-self-hosted-coverage | 49 | 0.1 | 0.2 | 0.3 | healthy (its queue wait just trails coverage-self-hosted) |
Two jobs violate P99<=15: the hosted build (main baseline + hosted PR fallback), pinned at ~70 min, and coverage-self-hosted (PR incremental) whose real completions reach 54 min and whose timeouts cap at 60.
Job steps: setup 2s, checkout 8s, toolchain sync 9s, LLVM fetch 36s, Generate coverage 3170s (52.8 min), uploads 5s. So ~55s of setup and 100% of the rest is one tools/coverage.sh invocation.
Inside coverage.sh (timing.txt): bazel coverage = 3158s (52.6 min); Python LCOV filter + metrics = 2s; genhtml skipped (--no-html). So the entire cost is the Bazel coverage command; the runner-side post-processing is 2 seconds.
This corrects the prior appendix's hypothesis. The 2026-07-10 note assumed the runner-side lcov merge on the the runner host was a dominant coverage cost. Measured: the Python merge is 2s and Bazel's own Coverage report generation action is 14.1s. Neither is a lever. The cost is the instrumented build+test executed on the RE backend inside bazel coverage.
Decomposition of the 52.6-min bazel coverage (from BEP buildMetrics + profile), affected set = 376 targets from 17 changed files (fallback=false, reason=bazel_diff):
| work | measured | notes |
|---|---|---|
| CppCompile actions EXECUTED | 3112 | instrumented; 898 CppLink; the compile phase dominates wall |
| Compiling total CPU | 3321 cpu-min | summed across RE parallelism |
| TestRunner actions | 296 | 1174 cpu-min; runs across variants (base 206, geode 50, text_full 40) |
| cachedRemotely: true | 4 | across the ENTIRE build -> the coverage cache is effectively cold |
| Coverage report merge (runner) | 14.1s | not a lever |
| Python filter+metrics (runner) | 2s | not a lever |
Four compounding causes, all coverage-specific:
Net: the coverage lane does the right SET of work (incremental scoping is live), but does it at full cold cost every time because the RE coverage cache is never warmed and the instrumented surface is both broad (all //donner) and multiplied (variants).
Levers 1 and 2 are the ones that reach P99<=15 on worst-case (#829-class) PRs and both are operator-reserved (infra load / coverage scope); lever 3 is the safe partial that lands immediately.
A parallel read-only infra audit reports that the RE backend has ample slot headroom under organic CI (roughly well under a quarter of execution slots occupied), while the runner tier is CPU-overcommitted under concurrent runner-side phases (each runner microVM is oversubscribed on the shared runner-host cores). Two implications for this lane:
Operator directives 2026-07-11: (1) MAKE PR coverage incremental (affected subset only, full run on main-push) and prove it; (2) audit how good determine-targets actually is.
(1) Incremental RUN is already wired and engaging. PR coverage runs the bazel-diff affected subset, NOT a full //...: over 60 recent PR coverage runs, fallback_reason was bazel_diff 31x (52%), coverage_smoke 17x (28%), no_instrumentable_targets (skipped, correct) 11x (18%), and a full-//... fallback (infra_change) exactly ONCE (2%). Full-fallback is rare, so it is not a material P99 driver today, and the fallback triggers are earning their cost. The main-push baseline runs the full set. So the Packet A affected-targets intent is live; the coverage cost is the SIZE and COLD cost of the affected subset, not a failure to scope.
Instrumentation-scoping is NOT a compile-time lever in this setup (negative finding). A natural next step was to also make INSTRUMENTATION incremental (scope --instrumentation_filter to the changed packages so unchanged deps compile without coverage). Investigated and rejected with evidence: bazel aquery --config=coverage --collect_code_coverage shows the //donner/base:base compile carrying -fcoverage-mapping/-fprofile-instr-generate even under a narrow --instrumentation_filter=-test_utils$,//donner/svg: that excludes base. .bazelrc does not globally force those copts, so the coverage feature is being applied to all //donner compiles regardless of the filter; the filter scopes which files' coverage is COLLECTED/reported, not which are compiled with instrumentation. Conclusion: narrowing --instrumentation_filter would change the report scope (and risk under-reporting) without cutting the instrumented compile cost. It is a dead end for speed here; the compile cost must be attacked via the RE cache seed (lever 1) or variant scope (lever 2). No code shipped for this idea.
(2) Determination quality. Affected-set size vs branch-side change (over-inclusion), from the same 60-run sample (bazel_diff runs):
| branch (sample) | changed files | affected targets | ratio |
|---|---|---|---|
| codex/wasm-sanitizer-followup | 3 | 378 | 126x |
| editor/group-editing (#829) | 17 | 376 | 22x |
| wasm-ios-runtime | 21 | 381 | 18x |
| geode/gpu-residence | 10 | 187 | 18.7x |
| geode-perf-transfer-counters | 21 | 270 | 12.9x |
| editor/structured-source-drag | 14 | 106 | 7.6x |
| editor/sample-picker-debug-overlay | 25 | 117 | 4.7x |
| editor-v08-ux-polish | 82 | 193 | 2.4x |
The tail (300-460 targets) is what blows the coverage budget. Two contributors:
Ownable determination fix (ranked): align the bazel-diff base worktree to the merge-base. Check out the base worktree at git merge-base BASE_SHA HEAD_SHA instead of BASE_SHA, so bazel-diff compares fork-point vs head (matching the git-diff basis already used for gating). This removes base-drift over-inclusion, shrinking the affected set (and thus coverage compile+test) for stale branches. Caching note: the base-hash cache is keyed by BASE_SHA; it must be re-keyed by the merge-base SHA (which is itself a main commit, so the main-push pre-warm can still hit it). This is a determine-targets change in this lane; it is post-merge-measurable only (pull_request runs the base workflow) and composes with, but does not by itself reach, P99<=15. Fallback-trigger review: the infra-file list and the no_instrumentable_targets skip are pulling their weight (18% correctly skipped, 2% infra-fallback); the large_change>75-files threshold routes broad PRs to the hosted lane and fired on editor-v08-ux-polish (82-85 files) as intended. No trigger is mis-tuned enough to change today.
Levers 1 and 2 remain the only ones that reach P99<=15 on worst-case header-fanout PRs, and both are operator-reserved (infra load / coverage scope).
Implemented in both main.yml and coverage.yml determine-targets: bazel-diff now hashes the CURRENT BASE TIP against the PR's MERGE commit (the workspace checkout, refs/pull/N/merge) instead of base tip vs HEAD_SHA. The base-hash cache keying (base tip, main-push pre-warmed) is unchanged. Fail-closed: if the workspace is not a merge commit, it is the PR head and the comparison degrades to exactly the old behavior.
Design history, recorded for provenance: the first draft hashed the base at the MERGE-BASE (fork point) vs HEAD_SHA. Review (PR #836, P1) correctly identified an escape hole in that scheme: a reverse dependency added on main AFTER the fork (a dep edge onto code the PR touches) exists in neither the fork-point graph nor the PR-head graph, so fork-vs-head hashing cannot see it, while the CI jobs build the merge ref where it is live. The tip-vs-merge comparison dominates both schemes: post-fork drift is present on BOTH sides (identical hashes, drops out, killing the inflation), and the merge graph is the one hashed (merge-only reverse dependencies hash differently vs the base tip, so they stay in the affected set).
Validation (controlled reproductions, hub-local bazel-diff v18.1.0, same flags as CI; fork point 3 commits behind main tip):
| test | scheme | result |
|---|---|---|
| A: inflation (one-line change to donner/base/tests/Box_tests.cc) | old: tip vs HEAD_SHA | 130 affected targets |
| A | new: tip vs MERGE | 2 (//donner/base:base_tests, base_tests_lint) |
| B: merge-only rdep escape (drifted base adds //probe_drift:probe_dep depending on //donner/css:css; PR touches donner/css/CSS.cc) | rejected draft: fork vs HEAD_SHA | 422 targets, probe_dep MISSED |
| B | new: tip vs MERGE | 425 targets, probe_dep CAUGHT |
Test A shows the over-inclusion fix (matches the 126x-tail signature in the determination-quality audit above); Test B constructs the reviewer's escape scenario and shows the shipped scheme closes it while the rejected draft did not. Same PR also raises the CI Linux lane's --remote_timeout from 5s to 60s, mirroring the coverage lane's documented 2026-07-02 rationale, after two consecutive spurious DEADLINE_EXCEEDED-after-4.999s failures on organic load (the serialized coverage job passed concurrently under its 60s timeout; the first tip-vs-merge run then passed full-//... in 17m28s on the same busy backend).
Lever 1 (seed the RE coverage cache from main-push) is already live, and its verification CORRECTS this audit's cold-cache claim. The main-push coverage baseline has been routing to the self-hosted RE lane (same job, executor, and flags as PR coverage) since the runner-routing change (#818) took effect; the hosted build job now runs only as the PR fallback path. Measured main-push lane duration: 5-11 min on the RE lane versus the ~70 min hosted median in the 48h profile above (the profile window straddled the cutover; 69.7 median was the before-state). Codecov baseline semantics are intact: the artifact + hosted-upload path is unchanged, verified on live main-push runs.
Correction (integrity note): this audit's "`cachedRemotely: true` = 4, the coverage cache is effectively cold" was WRONG. BEP action events are emitted only for failed actions, so grepping them undercounts cache hits by orders of magnitude. The authoritative buildMetrics.actionSummary.runnerCount shows:
| run | remote cache hit | remote exec | internal | wall |
|---|---|---|---|---|
| PR #829 coverage (the 54-min exemplar) | 3107 | 1519 | 5854 | 54 min |
| main-push 19:32Z | 5526 | 10 | 10610 | 7.5 min |
| main-push 15:30Z | 3748 | 1 | 104 | 5.7 min |
The seed works: main-to-main runs are ~97 percent cache-hit, and even the #829 worst case got 3107 hits. The revised root cause of the 54-minute run: the 1519 genuine cache-miss actions (the real reverse-dependency recompiles of the PR's changed headers - which can never be cache hits on first run) spent 4511 cpu-min in remote-execution QUEUE versus 158 cpu-min executing (96 percent queuing) at CI's floor scheduler priority during a contended window (the PR's own linux lane ran concurrently on the same backend, plus interactive dev priority). The lane's tail is therefore demand x contention on cache-miss actions, not cache coldness. Levers that cut miss-set size (the merge-ref hashing fix #836, and lever 2 below) attack it directly; scheduler priority is operator-reserved and untouched.
Lever 2 (representative-variant PR coverage) as-built. PR PATCH coverage now trims donner_variant_cc_test wrapper targets (_tiny, _text_full, _geode; build_defs/rules.bzl _VARIANT_SPECS) from the bazel-diff affected set when their base sibling is present; the base/default target is the representative. The full multi-variant matrix stays on the main-push baseline. Coverage-semantics change, stated plainly: patch coverage on PRs reflects the default variant only; backend/feature-gated lines and cross-variant regressions surface on main-push (and in CI's own test lanes, which are untouched - this trims only the coverage lane's instrumented re-run). Fail-safe: variant-only targets (no base sibling) are never dropped; tool failure keeps the untrimmed set. Implemented in tools/coverage_variant_trim.py (unit-tested, //tools:coverage_variant_trim_tests), called from coverage.yml determine-targets after the instrumentability classifier (which therefore still sees the full set).
Review hardening (PR #840 P2): trimming is kind-gated on the generated wrapper rule kind (donner_multi_transitioned_test, from the label_kind query the classifier already runs), so handwritten targets that merely share the suffix (e.g. the cc_library named renderer_geode beside renderer) are never dropped; unknown kinds fail safe to keep.
Measured on the #829 exemplar's real affected list with its real label_kind data: 376 -> 281 targets (95 generated wrappers dropped; 4 name-coincident non-wrapper targets correctly kept by the kind gate; 7 wrapper targets kept for lacking a base sibling); on the run's execution profile those wrappers account for 90 of 296 test executions and the transitioned compile closures behind the ~7x per-element instrumented compile multiplier. Projected: the cache-miss compile set on a variant-heavy PR shrinks by roughly the wrapper share; measured before/after cache-hit and wall numbers ride the next organic C/C++ PR (post-merge measurement, pull_request runs the base workflow).
1.15.0