ci: filter Pixi checkouts and trim unnecessary fetches - #3008
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
I asked codex: I'm intrigued by the But also: a few months back there was some back-and-forth with the fetch-depths. Could you please inspect the git history for extra context? To be sure we're not repeating some of the historical mistakes? Yes—this was available to us months ago. I found no evidence that PR #3008 repeats the earlier checkout regressions. Git formalized My inference about why we missed it: checkout’s usual recipe presents depth zero as “full history,” making it easy to overlook the separate question of which branches get fetched. The relevant historical safeguards are:
There was also release back-and-forth over checking out current tooling versus release-tag contents. That concerns which source tree is checked out; our changes preserve those selections. #2161, #2446. We now have live evidence too: PR #3008’s Pixi source-build smoke and freshness check passed. Both checkout steps took about four seconds. That strengthens confidence in the approach; the remaining packaging and GPU paths still need live validation. |
|
/ok to test |
|
I was just working on a similar solution (for the new reno-based release notes setup) and my agent came up with this solution (to run after a shallow checkout): This is very similar, but doesn't rely on a magical constant. Ultimately it probably doesn't matter too much which way we go, but thought I would share. |
|
|
At the moment I believe we don't need it (still working on it), but I was curious: Observation:
codex: The exact sentence entered Git’s documentation on January 11, 2013, in commit There’s a relevant connection to Mike’s suggestion: the same commit introduced So the constant has documented semantics dating back over thirteen years—and its readability was already a concern when that documentation was written. |
|
Logging here for easy reference JIC: CI @ f1d1ec8 https://github.com/NVIDIA/cuda-python/actions/runs/37053620205?pr=3008 (two common flakes, but complete otherwise) |
|
I asked codex: How much do we lose by avoiding the numeric constant, roughly? codex: Roughly two seconds and 47 MiB per checkout, when both approaches use I compared fresh repositories at the same commit, with all tags, twice per strategy:
Depth zero retrieves extra branch history and trees, although historical file contents remain filtered out. Both retained the same source ancestry and release tags. There’s an important distinction for Pixi: our original numeric-constant implementation didn’t use blob filtering. That combination measured 4.9s and 52.6 MiB, so the simplified Pixi settings performed about as well in this comparison. These are small local measurements, not guaranteed Actions timings. We retain essentially all of the observed four-minutes-to-seconds improvement; the narrower filtered fetch would save a little more time and substantially more metadata storage. Measurements{"strategy":"all-filtered","path":"/tmp/cuda-python-fetch-comparison.AEdsZq/all-filtered.DFJ0Yr/","fetch_seconds":2.97,"checkout_seconds":1.29,"total_seconds":4.26,"git_bytes":57515764,"branches":70,"tags":127,"reachable_commits":8599,"source_commits":2424}
{"strategy":"all-filtered","path":"/tmp/cuda-python-fetch-comparison.AEdsZq/all-filtered.LBDySj/","fetch_seconds":3.38,"checkout_seconds":1.16,"total_seconds":4.54,"git_bytes":57519466,"branches":70,"tags":127,"reachable_commits":8599,"source_commits":2424}
{"strategy":"narrow-filtered","path":"/tmp/cuda-python-fetch-comparison.AEdsZq/narrow-filtered.Fmc02h/","fetch_seconds":1.39,"checkout_seconds":1.20,"total_seconds":2.59,"git_bytes":7945308,"branches":1,"tags":127,"reachable_commits":2787,"source_commits":2424}
{"strategy":"narrow-filtered","path":"/tmp/cuda-python-fetch-comparison.AEdsZq/narrow-filtered.er2cAa/","fetch_seconds":0.87,"checkout_seconds":1.13,"total_seconds":2,"git_bytes":7946518,"branches":1,"tags":127,"reachable_commits":2787,"source_commits":2424}
{"strategy":"narrow-unfiltered","path":"/tmp/cuda-python-fetch-comparison.AEdsZq/narrow-unfiltered.LxgjnD/","fetch_seconds":4.84,"checkout_seconds":0.14,"total_seconds":4.9799999999999995,"git_bytes":55127184,"branches":1,"tags":127,"reachable_commits":2787,"source_commits":2424}
{"strategy":"narrow-unfiltered","path":"/tmp/cuda-python-fetch-comparison.AEdsZq/narrow-unfiltered.ssr9uL/","fetch_seconds":4.58,"checkout_seconds":0.14,"total_seconds":4.72,"git_bytes":55127184,"branches":1,"tags":127,"reachable_commits":2787,"source_commits":2424}compare-cuda-python-checkout-fetches.sh#!/usr/bin/env bash
set -euo pipefail
export GIT_CONFIG_GLOBAL=/dev/null
export GIT_CONFIG_NOSYSTEM=1
export GIT_TERMINAL_PROMPT=0
export GIT_ASKPASS=/bin/false
benchmark_root=$(mktemp -d /tmp/cuda-python-fetch-comparison.XXXXXX)
target_sha=f1d1ec8a9bf8b4140ee74db11e17edd3cbd35ff8
printf 'Benchmark directory: %s\n' "$benchmark_root"
for sample in narrow-filtered all-filtered narrow-unfiltered all-filtered narrow-filtered narrow-unfiltered; do
sample_dir=$(mktemp -d "$benchmark_root/$sample.XXXXXX")
git init --quiet "$sample_dir"
git -C "$sample_dir" remote add origin https://github.com/NVIDIA/cuda-python.git
git -C "$sample_dir" config gc.auto 0
fetch_args=(-c protocol.version=2 fetch --no-tags --prune --no-recurse-submodules)
case "$sample" in
narrow-filtered)
fetch_args+=(--depth=2147483647 --filter=blob:none origin
'+refs/tags/*:refs/tags/*' "+$target_sha:refs/remotes/origin/benchmark")
;;
all-filtered)
fetch_args+=(--filter=blob:none origin
'+refs/heads/*:refs/remotes/origin/*' '+refs/tags/*:refs/tags/*')
;;
narrow-unfiltered)
fetch_args+=(--depth=2147483647 origin
'+refs/tags/*:refs/tags/*' "+$target_sha:refs/remotes/origin/benchmark")
;;
esac
if ! /usr/bin/time -f '%e' -o "$sample_dir/fetch.seconds" \
git -C "$sample_dir" "${fetch_args[@]}" > "$sample_dir/fetch.log" 2>&1; then
cat "$sample_dir/fetch.log" >&2
exit 1
fi
before_checkout_bytes=$(du -sb "$sample_dir/.git" | cut -f1)
git -C "$sample_dir" count-objects -vH > "$sample_dir/objects-before-checkout.txt"
if ! /usr/bin/time -f '%e' -o "$sample_dir/checkout.seconds" \
git -C "$sample_dir" checkout --quiet --detach "$target_sha" > "$sample_dir/checkout.log" 2>&1; then
cat "$sample_dir/checkout.log" >&2
exit 1
fi
after_checkout_bytes=$(du -sb "$sample_dir/.git" | cut -f1)
test "$(git -C "$sample_dir" rev-parse HEAD)" = "$target_sha"
test "$(git -C "$sample_dir" rev-parse --is-shallow-repository)" = false
branch_count=$(git -C "$sample_dir" for-each-ref --format='%(refname)' refs/remotes/origin | wc -l)
tag_count=$(git -C "$sample_dir" tag --list | wc -l)
reachable_commit_count=$(git -C "$sample_dir" rev-list --all --count)
source_commit_count=$(git -C "$sample_dir" rev-list HEAD --count)
for namespace in 'v*[0-9]*' 'cuda-core-v*[0-9]*' 'cuda-pathfinder-v*[0-9]*'; do
git -C "$sample_dir" describe --tags --long --abbrev=40 --match "$namespace" HEAD >> "$sample_dir/describe.txt"
done
jq -nc \
--arg strategy "$sample" --arg path "$sample_dir" \
--argjson fetch_seconds "$(cat "$sample_dir/fetch.seconds")" \
--argjson checkout_seconds "$(cat "$sample_dir/checkout.seconds")" \
--argjson before_checkout_bytes "$before_checkout_bytes" \
--argjson after_checkout_bytes "$after_checkout_bytes" \
--argjson branches "$branch_count" --argjson tags "$tag_count" \
--argjson reachable_commits "$reachable_commit_count" \
--argjson source_commits "$source_commit_count" \
'{strategy:$strategy,path:$path,fetch_seconds:$fetch_seconds,checkout_seconds:$checkout_seconds,total_seconds:($fetch_seconds+$checkout_seconds),before_checkout_bytes:$before_checkout_bytes,after_checkout_bytes:$after_checkout_bytes,branches:$branches,tags:$tags,reachable_commits:$reachable_commits,source_commits:$source_commits}' \
| tee -a "$benchmark_root/results.jsonl"
done
for describe_file in "$benchmark_root"/*/describe.txt; do
if [[ -z ${first_describe:-} ]]; then
first_describe=$describe_file
else
cmp "$first_describe" "$describe_file"
fi
done
jq -s 'group_by(.strategy) | map({strategy:.[0].strategy, samples:length, mean_fetch_seconds:(map(.fetch_seconds)|add/length), mean_total_seconds:(map(.total_seconds)|add/length), git_MiB:(.[0].after_checkout_bytes/1048576), before_checkout_MiB:(.[0].before_checkout_bytes/1048576),branches:.[0].branches,tags:.[0].tags,reachable_commits:.[0].reachable_commits,source_commits:.[0].source_commits})' "$benchmark_root/results.jsonl" |
| # base and classify its changed paths, but historical blobs aren't. | ||
| fetch-depth: 0 | ||
| # Only PR mirrors need full ancestry to find their merge base. | ||
| fetch-depth: ${{ startsWith(github.ref_name, 'pull-request/') && '0' || '1' }} |
There was a problem hiding this comment.
To explain this change:
This is a drive-by optimization: non-PR runs don’t need history, so use a shallow checkout.
There was a problem hiding this comment.
Wouldn't this break tagging which needs the full history?
There was a problem hiding this comment.
codex:
Good question. This setting only controls detect-changes’ own checkout. On tag runs, it skips the PR merge-base steps and selects the full build plan without inspecting Git history. The wheel and sdist jobs each make their own full-history checkout, so setuptools-scm still has the ancestry and tags it needs.
That follows from the planner’s non-PR path and the unchanged checkouts in wheel builds, Linux sdists, and Windows sdists.
There was a problem hiding this comment.
If so, shouldn't we be able to set fetch-depth unconditionally to 1? Maybe I am still missing something?
|
rwgk
changed the title
rwgk
changed the title
|
Link to full CI (passed) before the final commit e6bc8b1: |
rwgk
changed the title
3 participants
Description
The Pixi workflows used full-history checkouts without blob filtering, so they
downloaded historical file contents, including the large
gh-pageshistory.This PR adds
filter: blob:nonewhile retainingfetch-depth: 0: completeancestry for all branches and tags remains available, and Git retrieves file
contents as needed.
The starting point was Ralf noticing
slow checkouts
in passing while watching the Pixi lockfile freshness workflow run against
main.That observation prompted a Codex session to investigate, followed by an audit of all 23
workflows and the improvements in this PR.
Related to #2197. This reduces CI checkout cost; the repository-wide clone-size
issue remains open.
checkouts, preserving full ancestry and tags for package versions and the
freshness check's base-commit worktree.
detect-changesat full depth for PR mirrors and use depth one for otherruns. PR merge-base calculation still uses the actual base branch made
available by the full-history checkout.
should-skipcheckout and supplyGH_REPOexplicitly;use a shallow source checkout for preview cleanup, whose script fetches
gh-pagesitself; and fetch only the selected Pathfinder release tag.ci/README.md.Example runtime savings
This is a fairly extreme example, but it is real. The savings come from Git fetching; the actual Pixi lockfile-checking work stays the same.
Validation
pre-commit run --all-filespassed, including actionlint.Git-derived version descriptions, and the PR merge base with blob filtering.
It also verified that creating an older base-commit worktree lazily fetched
a file blob absent from the initial checkout and produced the expected contents.
gh-pagesfetch anda concurrent deployment and push/rebase/retry scenario.
a44c4dede2ddabaed7018ceedd8d9d3ae3a852f6), the automaticPixi source-build smoke
and freshness check
passed. The full CI run
is in progress;
should-skip,detect-changes, and both Linux and Windowssdist jobs have already passed. The coverage conclusion below assumes the
remaining CI checks pass.
Why the usual
/ok to testCI is sufficient for validating this PRThe usual PR CI, together with the passing automatic Pixi checks and local
validation above, covers the checkout behaviors changed here. Each behavior
needs representative coverage rather than a separate run of every workflow.
/ok to testsynchronizes thepull-request/3008mirror and triggers CIthrough a push. This exercises checkout-free PR metadata lookup and
full-history change detection, including the actual PR base branch and
merge-base calculation.
fetch-depth: 0plusfilter: blob:nonecheckouts. Building the packagesexercises the ancestry and tag requirements of
setuptools-scmand accessto source blobs with this same checkout configuration.
full-history checkouts on a PR merge ref. The local worktree fixture covers
deferred access to older source blobs when freshness needs a base-commit
worktree; a fresh lockfile does not necessarily exercise that path live.
history. Its shallow checkout belongs only to that job and is not shared
with builders, so tag-triggered builds retain their independent full-history
checkouts. The existing planner tests cover selection of the full build plan
without a reusable baseline.
Pathfinder's explicit release tag remains available for
lookup-run-id;preview cleanup still fetches
gh-pagesitself, with its retry behaviorchecked locally.
The other Pixi jobs reuse the same full-history blob-filtered checkout
configuration. Separate refresh, cleanup, and release runs would repeat these
fetch behaviors while also performing unrelated work or writes. Once the
current full CI run passes, this provides sufficient confidence for the changes
in this PR.
Checklist
were run; no automated regression tests were added.