Reuse graph integration build outputs in CI - #9670
Conversation
|
@dnikolayev is attempting to deploy a commit to the HASH Team on Vercel. A member of the Team first needs to authorize it. |
|
|
||
|
|
||
| def run(args: list[str], root: Path, env: dict[str, str]) -> bytes: | ||
| return subprocess.run(args, cwd=root / "apps/hash-graph", env=env, |
There was a problem hiding this comment.
Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'.
🥳 Fixed in commit 1082f6c 🥳
There was a problem hiding this comment.
Removed the version probes that executed environment-selected tool paths in 1082f6c. Tool overrides remain bound to the cache key by their resolved paths and SHA-256 binary hashes; only fixed Rust/Cargo commands perform version discovery. A regression checks that compiler and wrapper overrides are never executed for identity inspection and that changed wrapper bytes invalidate the identity. All 15 helper checks and 3 timing checks pass locally. Leaving this thread for reviewer resolution.
🌟 What is the purpose of this PR?
Reduce graph build overhead during integration startup by reusing a validated graph executable. This is a draft implementation; performance savings and full integration equivalence have not yet been established.
The starting baseline is
67f60d5446ed3224609161f938e1b36bc9d62f89. Application source, dependencies and feature selection are unchanged. The original graph command remainscargo build --bin hash-graph --all-features.🔗 Related links
🚫 Blocked by
🔍 What does this change?
Caches have no restore prefixes. GitHub's repository/ref cache boundaries remain in force; no privileged execution of fork code or upstream secret sharing is introduced. Environment values are hashed rather than written to receipts.
Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
📜 Does this require a change to the docs?
The helper includes implementation and reproduction documentation for contributors.
🕸️ Does this require a change to the Turbo Graph?
The graph compile task remains uncached; only its supported CI invocation is guarded.
The initial input boundary is conservative: unrelated tracked edits, different pruning or runner identities can miss. Linux build and fresh-runner backend restoration have passed. Metadata, hashing and transfer costs remain part of the repeated measurements. No saving is claimed from the historical run or from focused unit tests.
The fork harness uses identical credential-free settings for both variants: no upstream Turbo/sccache access, local Turbo cache on fresh runners, and
CARGO_INCREMENTAL=0. The primary baseline retains the original Cargo command and environment; the candidate pays its preparation, validation and cache costs. Earlier sealed-environment baseline runs are classified as diagnostic, not net performance evidence. Smoke scope contains only Playwright and backend integration and cannot establish complete Test workflow savings. The generator also supports the complete push-profile Test selection/gates. The first full net baseline passed on fixed sourcef4dc7dd3faa93fd22120ba3ce48ece73592b5324: 32m 09s to final job completion and 440m 37s aggregate job time. Playwright took 30m 22s and was the last substantive job; backend integration took 19m 01s. All 76 executed jobs passed, with one expected publish dry-run skip. This single baseline establishes no performance saving. Every source checkout is pinned so warm repetitions retain identical selection.Diagnostic results on source
12556720a1c8a455ac8922899579f95bd5807459(one successful cold run per variant, sealed-environment baseline):Baseline diagnostic and candidate diagnostic both executed 32 Playwright tests with 8 skips and 110 backend tests plus 48 snapshot tests with 9 skips. These single samples are inconclusive and are excluded from primary net-savings claims. Later graph Cargo invocations in these sealed diagnostics compiled no crates and spent minutes waiting for shared Cargo locks; the original fallback smoke runs had much shorter repeat waits. The sealed wrapper can affect scheduling, so those waits cannot be presented as normal-baseline compilation savings. A later implementation change removes duplicate payload hashing; it still needs repeated measurement.
The complete diagnostic baseline passed all gates: 31m 48s to final job completion (31m 49s GitHub-reported total run time) and 455.45 aggregate job-minutes. It used the sealed baseline and overlapped diagnostic runs, so it is feasibility evidence rather than a primary baseline.
The fresh-runner diagnostic has confirmed an exact Actions cache hit and executable restoration for backend integration, with the same executed counts and a 14m 18s job. Playwright also passed with 32 tests and 8 skips in 31m 37s, but its initial Actions restore missed: only the machine fingerprint changed. The run is classified as primed with 1 of 2 actual Actions hits, not a fully restored warm run. It does not establish a repeated warm comparison.
The initial candidate smoke fell back to ordinary Cargo and passed; it did not exercise reuse. The initial baseline smoke retained a Playwright extension service-worker setup timeout (31 passed, 8 skipped, 1 failed). Both attempts remain retained and excluded from performance summaries.
🐾 Next steps
Complete the fork execution proof and paired measurements; retain failed attempts and cache receipts. Report regressions and inconclusive comparisons alongside improvements. Maintainer-run validation may still be necessary to establish performance with upstream external caches. Frontend reuse is outside this PR.
🛡 What tests cover this?
❓ How to test this?
python3 .github/actions/graph-build-cache/graph_build_cache_test.pyandpython3 .github/scripts/integration_build_timings_test.py.python3 .github/scripts/integration-build-timings.py manifest.jsonto derive comparisons. The script and helper README document the manifest and retrieval commands.📹 Demo
Not applicable; CI behavior is demonstrated by execution receipts and timing results.