Skip to content

perf(ci): make the nix dependency build reusable across revisions - #1722

Closed
daniel-noland wants to merge 23 commits into
restore-codecovfrom
pr/daniel-noland/ci-nix-cache
Closed

perf(ci): make the nix dependency build reusable across revisions#1722
daniel-noland wants to merge 23 commits into
restore-codecovfrom
pr/daniel-noland/ci-nix-cache

Conversation

@daniel-noland

Copy link
Copy Markdown
Collaborator

Stacked on #1714. Read that one first; this branch is based on it, and the
diff shown against main includes its commits until it merges.

#1714 decides what CI runs and when. This decides how much it costs.

What was wrong

Every crane derivation opted out of crane's deps-only split, so each one
compiled the whole dependency graph itself -- 476 of 516 crates in a full
build are third-party, and -Zbuild-std rebuilds the standard library on top
of that. Sharing one dependency build per flag-set fixes that, but only if
the shared artifact actually survives a source edit, and three separate
channels were invalidating it:

  • --remap-path-prefix==${src} put the filtered source store path into
    RUSTFLAGS, which is part of both the derivation and cargo's per-unit
    fingerprint. Any Rust edit gave a new compilation identity to all 516
    crates, including the ~475 that contain none of our source.
  • VERSION came from git describe, so it changed per commit.
  • results/ was inside src.

check was untouched by any of this, because check-clippy and
check-doctest invoked bare cargo against an empty target/ on an
ephemeral runner. Routing them through nix exposed that the docs
derivation had rotted: thirty-one broken intra-doc links across six crates,
and a missing --check-cfg=cfg(emulated) in RUSTDOCFLAGS that would have
failed the build even with every link correct.

Results

Like-for-like, 32 jobs, measured on the lab runners:

scenario before after
pull request that edits Rust 213.5 runner-min 169.4 (-21%)
re-run, docs-only change, or merge-queue recheck 213.5 runner-min 103.0 (-52%)

clippy went from forty-one per-package derivations (~12 min) to one (32.4s),
and now runs --all-targets: test code is as load bearing as the rest and
gets the same static analysis. check/debug holds at ~233s while also
linting test targets and building docs, neither of which it did before.

Much of what remains was traded from compute to network, which is the cheaper
resource on these runners.

Guard

just check-deps-reuse builds the dependency derivation before and after a
synthetic source edit and fails if the hash moves. It catches both
regressions above, and runs in lint and check.

@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c2cf1f92-0423-409c-a2b4-71b3fdbc1dd6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@daniel-noland daniel-noland added the ci:+merge-ready Run all checks which will be run in the merge queue regardless of label status label Aug 16, 2026
@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Optimizes CI/Nix build performance by introducing a reusable, deps-only Cargo artifact build (shared across revisions/configs) and stabilizing inputs that previously invalidated cache reuse (source path remapping, VERSION/tag, and filtered source contents). It also routes more checks through Nix to ensure CI and local runs converge, adds guards to detect cache-splitting regressions, and fixes/avoids rustdoc link/cfg warnings surfaced by running docs in the sandbox.

Changes:

  • Add shared deps-only craneLib.buildDepsOnly artifacts (tests vs non-tests variants) and wire workspace builds/checks/tests/clippy/docs/doctests to reuse them.
  • Stabilize per-revision cache keys (fixed --remap-path-prefix, VERSION handling for deps builds, exclude results/ from src) and add just check-deps-reuse guard + CI step.
  • CI ergonomics/cost controls: dynamic strategy.max-parallel, retry logic for registry pushes, and ensure docs/doctests run via Nix.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
routing/src/fib/test.rs Reduce workload under cfg(instrumented) to keep coverage runs practical.
nix/profiles.nix Register and (under coverage) enable cfg(instrumented) and related flags.
net/src/ipv6/hop_by_hop.rs Adjust rustdoc markup to avoid/resolve intra-doc link warnings.
net/src/ip_auth/v6.rs Adjust rustdoc markup to avoid/resolve intra-doc link warnings.
net/src/ip_auth/v4.rs Adjust rustdoc markup to avoid/resolve intra-doc link warnings.
net/src/headers/within.rs Fix/adjust rustdoc link target for EmbeddedMatcher.
net/src/headers/view.rs Normalize rustdoc links/markup; align docs with items in scope.
net/src/headers/pat.rs Adjust rustdoc markup (e.g., ArrayVec) to avoid link warnings.
net/src/headers/mod.rs Adjust rustdoc markup (e.g., MAX_VLANS) to avoid link warnings.
net/src/headers/embedded_view.rs Normalize rustdoc links/markup for embedded view APIs.
lifecycle/src/lib.rs Fix rustdoc link path for TOTAL constant reference.
justfile Add stable source-prefix linking, timings toggle, deps-reuse guard, retry push logic, and route clippy/doctest via Nix builds.
default.nix Implement reusable deps-only artifacts, stabilize inputs (src prefix, VERSION), add doctests derivation, timings support, and exclude volatile outputs from substitution.
config/src/external/overlay/acl.rs Adjust rustdoc markup to avoid/resolve intra-doc link warnings.
concurrency/src/thread/mod.rs Adjust rustdoc markup to avoid/resolve intra-doc link warnings.
concurrency/src/stress.rs Adjust rustdoc markup to avoid/resolve intra-doc link warnings.
concurrency/src/slot.rs Fix rustdoc link path for Subscriber::snapshot.
ci.just Refine lab runner budgeting and split test-each budget from other jobs; add docs check recipe.
acl/src/lib.rs Update crate docs around reference feature/module documentation.
acl/src/dpdk/install.rs Adjust doc reference to crate-private function and avoid broken link.
.github/workflows/README.md Document new ci:-vlab label semantics alongside other CI labels.
.github/workflows/dev.yml Add dynamic parallelism for deep runs, add docs check job, add deps-reuse guard step, and implement ci:-vlab gating.
.github/actions/nix-shell/action.yml Support “prewarmed” Nix runner images and refine Cachix push filtering to exclude volatile/image outputs.
.cargo/config.toml Register cfg(instrumented) to prevent unexpected_cfgs warnings outside coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread justfile
Comment on lines +43 to +46
_link-sources:
{{ _just_debuggable_ }}
mkdir -p "$(dirname '{{ src_prefix }}')"
ln -sfn "$(pwd)" '{{ src_prefix }}'
Comment thread default.nix
Comment on lines +463 to +471
orig:
if for-deps || no-bins then
{
postBuild = (orig.postBuild or "") + ''
unset RUSTFLAGS;
'';
}
else
{
daniel-noland and others added 2 commits August 15, 2026 20:29
Debug binaries retained the complete Rust toolchain through their standard-library source paths, adding roughly 2.4 GB to the closure. They also carried a sizable DWARF index that neither packaged debugger consumes.

Point those paths at the much smaller rust-src component, omit unused prebuilt documentation, and remove .debug_names. Source browsing and symbols remain available while the resulting diagnostic images become practical to store and transfer.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
A core collected from the lab is useful only with the exact unstripped binaries and sources that produced it. A general debugging toolbox cannot reconstruct that relationship after the release has moved on.

Provide a purpose-built gdb image alongside each build and teach it Rust's standard-library types without retaining rustc. This keeps post-mortem debugging reproducible while avoiding unrelated live-debugging tools.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
@daniel-noland daniel-noland self-assigned this Aug 16, 2026
daniel-noland and others added 20 commits August 15, 2026 20:41
Post-mortem inspection and live debugging need different tools. The core viewer cannot offer an editor-driven session, while bugstalker understands Rust layouts and can expose the running dataplane through the Debug Adapter Protocol.

Track bugstalker upstream for its current remote DAP support and package it separately with the matching binaries and sources. Keeping the image single-purpose avoids making every diagnostic artifact carry every debugger.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Some failures need a record of the dataplane's kernel interactions rather than an interactive debugger. A small, repeatable tracing environment is easier to deploy and feed into existing log analysis than a general-purpose toolbox.

Package lurk around the matching release binaries and follow the worker threads where the dataplane does its work. Because syscall tracing needs no symbols, this image can stay much smaller than the debugger images.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
The diagnostic images are useful only when they match the build being investigated, but building roughly 850 MB of extra images for every pull request would undermine the runner-load reduction this CI rework is meant to achieve.

Build them automatically for pushes, the merge queue, and manual runs, with an explicit label available for debugging a pull request. Publish all three beside tagged releases so the matching tools remain available when a deployed build needs investigation.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
All three images built green while two of them could not do what the
README documented.  Building proves an image links; it says nothing about
whether its entrypoint runs.

`smoke-container` runs each one the way the README tells a user to:

  - the tracer under a plain `docker run`, with no seccomp relaxation,
    and requires an `execve` in the trace.  Its failure mode is the
    reason this exists: lurk emitted eight well-formed JSON lines
    recording its own child failing to start, then exited 0, which the
    `jq -R 'fromjson? // empty'` filter we document accepts without
    complaint.  Both guards fire against the pre-fix image.
  - the debugger over DAP, requiring a `process` event rather than a
    successful connection.  Connecting proves nothing on its own: in
    remote-DAP mode bugstalker waits for the client to name the program.
    This does not reproduce the entrypoint defect, which was in the
    documentation rather than the runtime; it pins the launch contract so
    the next change to it is visible.
  - the core viewer through its own entrypoint rather than by invoking
    gdb directly, since the `--directory` and `source` flags that
    register the printers live in that entrypoint.  Checks that the
    printer set is registered, not merely that gdb started.

The trace goes to a file rather than a shell variable: at a few megabytes
it overruns the here-string limit, and every grep against it then fails
with E2BIG, which reads exactly like a failed trace.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Workspace Rust outputs include the repository source and change on nearly every revision. Uploading them consumes transfer and storage while yielding few substitutions, crowding out the slower C and C++ dependencies that benefit from a shared cache.

Give source-volatile outputs a common store-path prefix, skip their substitute lookups, and exclude every OCI assembly path that could reintroduce them through its closure. Stable vendored sources, development dependencies, and native libraries remain cacheable.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Every lab job realizes the same dev shell against an empty store.  The
job logs put that at ~45s each: ~22s re-fetching the npins tarballs into
an empty `~/.cache/nix` and ~20s substituting ~340 store paths into an
empty `/nix/store`.  Each job lands on its own ephemeral runner, so both
halves are paid every time -- ~28 minutes across a 32-job pull request
run, a third of all lab runner time on that run.  The lab pool is the
binding constraint on this workflow, so that time comes off the queue.

The fix belongs in the runner image, which can carry the store and the
tarball cache already warm.  `install-nix-action` aborts as soon as it
finds nix on PATH, which makes such an image a drop-in, but that early
exit also skips the nix.conf, NIX_PATH, and TMPDIR setup the action
would otherwise do.  Supply those here so a job is configured the same
way either way.

`access-tokens` is the setting that matters: it is per-run, so an image
cannot bake it in, and without it a pin that misses the image's tarball
cache is fetched anonymously against the rate limit shared by every
runner behind the lab's address.

This assumes nothing about the image.  Where nix is not preinstalled the
new step is a no-op and the install proceeds exactly as before, so it is
safe ahead of the companion githedgehog/gha-runner change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
a5066b6 applied `source-volatile` to the crane workspace builds as well
as to image assembly.  Its premise -- that workspace outputs change on
nearly every revision and so rarely substitute -- holds for a revision
that changes Rust code.  It does not hold for most of the runs we pay
for.

`src` admits only cargo sources plus `.md`, `.json`, `.h`, and
`.justfile`, so `.github/**` and `*.nix` never reach the derivation.  A
revision touching only CI or nix hashes identically to its parent, as
does any re-run of a tree already built: a label-triggered run, the merge
queue, and the push to main after merge are all that.

Those runs used to cost almost nothing.  On 760a9c2 `sanitize/address`
compiled not one crate; it fetched a single 1.4 GiB `all-0.25.2` nextest
archive built by the v0.25.2 tag run on main and spent its 80s running
1235 tests.  After a5066b6 the same job compiles the workspace from
scratch: 1007s in run 31865573803, and 1051s re-running that same
revision on an idle pool, which rules out both runner contention and a
one-time cold cache.  Across a run that is 87 -> 211 minutes of lab time.

Nothing here shares between differently configured jobs.  The asan and
bluefield3-musl builds carry different RUSTFLAGS and sysroots, so they
hash to different store paths, as they must.  The reuse is strictly
between runs that build identical sources.

Keep `source-volatile` on the tar, buildEnv, and container derivations.
Those are built once per revision by a single job and reach the lab
through ghcr, so a copy in the binary cache buys nothing, and their
per-revision image blobs are what motivated the original change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`just build` writes its out-links to `results/`, and while that path is
gitignored, `lib.cleanSource` does not read gitignore.  The directory
therefore landed in `src`, so every developer who had run a build carried
a `src` hash that differed from CI's -- and that changed again whenever
they built a different target, because the symlinks point at store paths.

The practical effect was that local builds stopped matching the binary
cache after the first `just build`, which is the opposite of what this
tree is set up to do.  `target`, `sysroot`, and `devroot` were already
excluded for the same reason.

Verified by instantiation: a working tree containing `results/` now
produces the same derivation as a clean checkout of the same commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every crane derivation passed `cargoArtifacts = null`, which opts out of
the deps-only split crane does by default.  Each one therefore compiled
the whole dependency graph itself: 476 of the 516 crates in a full build
are third-party, and `-Zbuild-std` means each also rebuilds the standard
library from source.  Because those derivations embed `src`, that work
was redone for every revision that touches any Rust file.

Build the dependencies once per flag-set instead.  Crane dummifies the
workspace sources for that build, so it hashes on the manifests and
substitutes across revisions -- the reuse the per-package derivations can
never have.

Two variants are needed.  `mk-needs-unwind` gives tests
`-Zbuild-std=std,panic_unwind` against `panic_abort` for production, and
the test builders run under `profile-tests'`; sharing one artifact would
miss on fingerprints and rebuild anyway.  The test variant builds with
`cargo test --no-run` so dev-dependencies land in the artifacts, which
the nextest archives need.

Scope both with a `--package` per member of `package-list` rather than
letting cargo walk the whole dummified workspace.  The union the
consumers need is strictly smaller, and `package-list` is platform-aware:
for wasm32-wasip1 it honours the `wasm = false` opt-out in
`workspace.metadata.package`, leaving 13 members.  `k8s-intf` is one of
the excluded, and it is what pulls `rustls -> aws-lc-rs -> aws-lc-sys`;
that crate compiles its C sources with the host gcc and no WASI sysroot
(`CC_wasm32_wasip1 = Some(gcc)`, `WASI_SYSROOT = None`) and fails on
`pthread_rwlock_t`.  Inheriting the platform filtering rather than
restating it is what keeps `wasm32-wasip1` and `validator/release`
building.

Measured locally (debug, 8 jobs).  `workspace.dataplane` drops from 476
compiled crates to 52, 63s -> 30s, against 52s for the shared build:
break-even at about 1.6 consumers.  `tests.all` drops from 556 crates to
41, confirming dev-dependencies are present.

The gain is uneven.  A small package's baseline build only compiles its
own slice of the graph, so unpacking the artifacts can cost as much as it
saves: acl 27s -> 28s, args 23s -> 18s, cli 19s -> 15s.  These local
figures also exclude fetch cost entirely -- both arms read a warm local
store -- so they understate the cache on a runner with a fast path to the
CDN.  CI measurement follows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--remap-path-prefix==${src}` put the filtered source store path into
RUSTFLAGS, and RUSTFLAGS is part of both the derivation and cargo's
per-unit fingerprint.  Every revision touching any Rust file therefore
gave a new compilation identity to all 516 crates in a build -- the ~475
third-party ones and the standard library included, none of which contain
our source.

That is what made the dependency split worthless on the runs it was meant
to help.  Re-running an unchanged tree took 73.6 minutes of lab time
against 157.5 for the same jobs before the split; adding a one-line test
took 165.2, because the deps artifact was invalidated by the edit it was
supposed to survive.

The replacement prefix has to be stable *and* present when tests run.
Stable is the point of the change; present is load bearing for bolero,
which was not obvious.  `check!()` records `file!()`, and
`TargetLocation::abs_path` resolves it by canonicalising that path,
falling back to joining `CARGO_MANIFEST_DIR`'s ancestors.  Under nix the
manifest dir is `/build/source/...`, gone by the time tests run, so the
fallback is dead and resolution rests entirely on `file!()` existing.
The old remap satisfied that by accident, the store path being present on
the runner.  A prefix that does not exist fails all nine
`rate::test::derivative_of_arbitrary_*` cases in about ten milliseconds
with "could not resolve target work dir", never running an input.

So remap to `/tmp/dataplane/src`, which any user can create, and put the
tree there for the consumers that need it:

  - `test`, `test-each`, and `coverage-archive` link it at this checkout
    before running.
  - the debug images link it at the sources they ship, which they now
    carry explicitly through `source-tree`.  Until now the source reached
    those closures by accident: the remapped store path was baked into
    the binaries, so nix's reference scanner retained it.
  - `coverage-archive` passes `--path-equivalence` to `llvm-cov show` and
    `report`, which read sources to render them.  `export` does not need
    it; it only emits paths, and the existing rewrite handles those.

Verified: the deps derivation is now identical either side of a source
edit (9xs5b0w5...), the workspace builds, and the production binary's
closure no longer drags the source along -- which also stops image
closures reintroducing source-volatile paths.  Verified for bolero
through the path that failed rather than a bare `cargo test`, which
cannot reproduce it: `just test stats` runs 31 tests green, with
`derivative_of_arbitrary_8` taking 1.013s rather than failing in 0.012s.

Panic messages and backtraces now name `/tmp/dataplane/src/...` rather
than a store path.  Inside the debug images that resolves through the
symlink.

The prefix is spelled in both default.nix and the justfile.  Deriving one
from the other means either threading an `--argstr` through every build
call site or paying a `nix eval` on every `just` invocation; a mismatch
fails loudly and immediately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A queued merge blocks everything behind it and its result is what gates
the merge, so it is worth finishing sooner even though the extra runners
come out of the pool pull requests are waiting in.  Run the merge queue
and pushes to main or a release branch four matrix entries at a time;
pull requests stay at one so they cannot crowd the queue out.

`strategy` cannot read `env` -- only `github`, `inputs`, `needs`, and
`vars` -- so the decision travels as a `plan` output, which also puts it
in the job that already decides what a run does.  Job outputs are
strings, hence `fromJSON`.

Jobs read it through `strategy.max-parallel`, which is also what feeds
`ci::parallel` into `JUST_VARS`, so the per-job core budget follows
without a second switch to keep in step.

`cross` is deliberately left at one.  `ci.just` turns `ci::parallel` into
`share = 1/N` and only `ci::cross` and `ci::cross-test` consume it, so
raising it there divides each job's eight cores rather than adding
throughput -- unless the runner pods really do get isolated core budgets,
which is a question for whoever owns the scale set.  `miri` has a single
matrix entry, so the setting does nothing for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The label still exists and 23 pull requests have carried it, but nothing
has read it since 68dc9e1 made VLAB opt-in.  It is also the only way to
sit out the lab tests while keeping everything else: ci:+merge-ready
turns on every gate, VLAB included, so dropping it to skip an hour of lab
time would also drop miri, the sanitizers, cross, concurrency, test_each,
and wasm.

Skip the whole VLAB matrix, hybrid legs included, when a pull request
carries ci:-vlab.  This mirrors ci:-upgrade, the subtractive label the
workflow already honours.

Documentation for the label is deliberately left for a separate commit:
`src` admits every `*.md` in the tree, so editing the workflow README
rehashes every workspace derivation, and this needs to stay cache-neutral
while the timing experiment is running.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ghcr.io drops a push every so often -- twice in the last five runs, both
times `writing blob: uploading layer chunked: blob upload unknown`, and
403s have been seen too.  A lost push fails a job that has already done
all its work, and we have no visibility into the registry's side, so
retrying is the best available answer.

Two layers, because they cover different failures.  `--retry-times` is
skopeo's own and retries a blob rather than restarting a copy that may
already have moved most of an image; it does not retry `denied`/403 or a
blob-upload error, which are the two modes we actually see.  An outer
loop restarts the whole copy for those.  That is safe: skopeo skips blobs
the registry already has and a partial upload is discarded server side,
so the push is idempotent.  `oras push` for the wasm validator goes
through the same loop.

Retries are announced with `::warning::` so the flake rate stays visible
in the run summary; a silent wrapper would turn "ghcr is degrading" into
"CI got slower".  Errors outside the retryable set fail immediately
rather than sitting through a minute of backoff first.

Collapsing the six identical `skopeo copy` arms into `push_image` is a
side effect of having somewhere to put the retry.

Verified against simulated failures: a call that fails twice then
succeeds recovers and logs two warnings, a persistent 403 exhausts its
attempts and errors, and a non-retryable `manifest invalid` fails in 0s.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shared dependency artifacts were rehashing on every commit, so the
split still bought nothing in CI even after the source prefix was fixed.
Runs 5 and 6 built dataplane-tests-deps at 297xbx0p... and kfrigyc3...
from trees whose only difference was one test function.

`invoke` puts `VERSION = tag` in every crane derivation's environment,
and the justfile computes `tag` from `git describe --tags --dirty
--always`, which moves with every commit.  That reached the dependency
build as surely as it reached the workspace builds.

A dependency build compiles third-party crates and the standard library.
None of them read VERSION, and cargo only fingerprints an env var for
crates that actually reference it, so pinning it to a constant there
leaves the artifacts valid for consumers while decoupling their hash from
the commit.

Both invariants now hold: the dependency derivation is unchanged across
`--argstr tag dev` versus a git-describe style tag, and unchanged across
an edit to a workspace source file.

This is the second such channel.  The first was the source path in
RUSTFLAGS; anything else that varies per commit and lands in the
dependency build's environment will defeat the split the same way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two per-commit inputs have leaked into the shared dependency build: the
source path, through `--remap-path-prefix` in RUSTFLAGS, and the git
version, through `VERSION = tag`.  Each defeated the split completely,
and each took several CI runs to notice, because the symptom is a cache
miss rather than a failure -- the jobs still pass, just slowly, and only
a careful read of a build log shows dependencies compiling that should
have been fetched.

`check-deps-reuse` varies the two things that move per commit -- the tag
and a workspace source file -- and requires the dependency derivation to
hold still.  It runs in `lint`, which already has nix available, and
costs a pair of instantiations.

Verified to fail on both known regressions rather than merely passing on
a healthy tree: reintroducing `VERSION = tag` reports the git version,
and restoring `--remap-path-prefix==${src}` reports the workspace source.

This does not prove there is no third channel.  It does mean a third one
is caught by the lint job on the commit that introduces it, rather than
by reading a coverage log three runs later.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Answers "where did the time actually go" with per-crate numbers rather
than counting `Compiling` lines.  That question came up twice while
sizing the dependency split: 475 of 516 crates are third-party, but a
count of compile units is not a share of wall time, and nothing in the
logs distinguished them.

Off by default.  Enabling it changes every cargo command and so rehashes
every derivation, which is not a cost to impose on ordinary builds --
verified: with `timings=false` the derivation is byte-identical to before
this commit, and with `timings=true` it differs.

    just timings=true build tests.all

leaves the report in `$out/cargo-timings`.  Both `cargo build` and
`cargo nextest archive` support the flag, so every builder is covered.

Note the report embeds a timestamp, so an output built with it is not
reproducible.  That is another reason to leave it off outside of
investigation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ci::check-clippy` and `ci::check-doctest` invoked bare `cargo` against a
`target/` directory that starts empty on every ephemeral runner, so they
recompiled the whole dependency graph every time and no amount of nix or
Cachix work could touch them.  That is why `check` barely moved across
eight runs while the jobs that do build through nix swung widely: run 8's
`check/debug` fetched its dependency artifact and still compiled 427
third-party crates, because only one of its four steps goes through nix.

The nix derivations for all three already existed and nothing called
them, so `docs` rotted: thirty-one broken intra-doc links across six
crates, each hidden behind the last because rustdoc stops at the first
crate that fails.  One of them was `acl`'s link to `crate::reference`,
which resolves only where the non-default `reference` feature happens to
be on; a code span is correct in every configuration, and the "Always
built" claim beside it was simply false.  `docs-builder` also set
RUSTDOCFLAGS without the `--check-cfg=cfg(emulated)` that RUSTFLAGS
carries, so every `cfg_attr(emulated, ...)` site tripped
`unexpected_cfgs` -- it would not have built even with every link
correct.

clippy keeps `--all-targets`.  Test code is as load bearing as the rest
and deserves the same analysis, and dropping it would have quietly
narrowed what is linted.  Linting test targets means compiling them, so
this takes the unwind flavour of `-Zbuild-std`, the test profile, and
`cargo-artifacts-tests`, matching how the tests themselves are built.

Doctests run inside the sandbox.  They cannot be archived and shipped to
the host like the other tests: cargo rejects `--doc --no-run` outright,
so there is no build-without-running step to archive.  Being the first
thing to execute in the sandbox, they turned up two things that never
mattered while everything ran on the host -- `.cargo/config.toml` is part
of `src` while `scripts/test-runner.sh` was not, and that script's
`#!/usr/bin/env bash` has nothing to resolve in the sandbox.  Hence `.sh`
in the source filter and a `patchShebangs` before the build.  Cargo
reports both as "No such file or directory" against the test rather than
the interpreter, which is why they looked identical.

Verified by building each: `clippy.acl`, `doctests.all`, and `docs.all`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`clippy` ran as forty-one per-package derivations, each unpacking the
shared dependency artifacts before linting one crate.  `clippy.args` --
one of the smallest packages -- took 17.8s with those artifacts already
warm, so almost all of it was fixed setup.  Forty-one of those is about
twelve minutes to lint a workspace that takes 32.4s in a single
derivation, measured either side of this change.

The per-package split bought no cache granularity to pay for it: `src`
covers the whole workspace, so editing any file invalidates all
forty-one at once.  `clippy.pkg.<name>` is still there for linting one
crate by hand.

Both `--package` lists come from `package-list` rather than `--workspace`,
because it is platform-aware: on wasm32-wasip1 the excluded members pull
in dependencies that cannot build for that target.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lab cgroup allows ten cores.  `cores=8` left two of them idle on
every job, and nothing inside the container can see the limit to catch
that: `nproc` reports the node's 32 and the cgroup files are not readable
from the pod.  Measuring throughput from inside cannot recover it either,
since concurrent slots that fit within ten cores show no contention.
Treat the ten as given rather than inferred.

One derivation at a time suits the heavy jobs, which each build a single
large derivation: on an otherwise identical run `dataplane/release` went
559s -> 454s and `check/fuzz` 944s -> 709s.

`test_each` is the exception, building one derivation per package, and it
went the other way at 613s -> 887s.  Give it two jobs of five cores so
that a package too small to saturate the budget no longer leaves most of
it idle, while the total request stays inside the cgroup.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two costs that no amount of caching reaches.

`test_concurrency_fibtable` drives 100k packets through six threads.
Under `-Cinstrument-coverage` every counter is instrumented and it runs
47.6s of an 84s suite -- nextest cannot finish faster than its slowest
test, so it is a floor under `check`, `coverage`, `sanitize`, and
`test_each` alike.  Coverage measures which lines execute, not how often,
so give it 2k packets there via an `instrumented` cfg, registered and set
the same way `emulated` already is.  Deliberately not set for the
sanitizers: those runs want the iterations, because that is how they find
races.  Three smaller concurrency tests in the same file cost 5-9s each
and are left alone for now.

`check/fuzz` compiles the workspace optimised to re-verify what
`check/release` has already covered, for 709s in the differential run.
The fuzz build still gets exercised by `coverage/fuzz`, both sanitizers,
and the fuzzing campaign these worker-minutes are being freed up for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ci:-vlab` has been honoured since the label handling landed, but the
workflow README listed `ci:-upgrade` as the sole subtractive label.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@daniel-noland

Copy link
Copy Markdown
Collaborator Author

Superseded by #1725, which is the same branch re-parented onto #1724.

The debug images were split out of #1714 into #1724 on review, and this PR's nix work depends on them (perf(nix): remap sources to a fixed prefix patches their extraCommands and adds source-tree), so it has to sit above them in the stack. GitHub declines to change a stacked PR's base through either the GraphQL or REST API, so this one is closed rather than re-parented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:+merge-ready Run all checks which will be run in the merge queue regardless of label status

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants