Skip to content

ci: cache workspace crate artifacts in integration build#6261

Merged
desmondcheongzx merged 3 commits intomainfrom
ci/cache-workspace-crates
Feb 20, 2026
Merged

ci: cache workspace crate artifacts in integration build#6261
desmondcheongzx merged 3 commits intomainfrom
ci/cache-workspace-crates

Conversation

@desmondcheongzx
Copy link
Copy Markdown
Collaborator

@desmondcheongzx desmondcheongzx commented Feb 20, 2026

Add cache-workspace-crates: "true" to the Swatinem/rust-cache config for the integration-test-build job.

cache-all-crates only controls registry cleanup (downloaded .crate files and extracted sources in ~/.cargo/registry/). Workspace crate fingerprints, build script outputs, and compiled .rlib/.rmeta artifacts in target/ are cleaned by cleanTargetDir() during cache save — which builds its keep-list from getPackagesOutsideWorkspaceRoot(), explicitly excluding workspace members. This caused all ~70 local crates to recompile from scratch every run (~25 min at opt-level=3).

cache-workspace-crates: "true" adds workspace members to the keep-list so their artifacts survive the save cleanup.

Results

Tested on this PR with temporary save-if: true to bootstrap the cache:

Metric Cold cache Warm cache
Cache restore 2s 21s
Build wheels 36 min 10 sec
Total job ~37 min ~1 min 41 sec

Combined with the restore-mtime fix from #6246 (source file mtimes older than cached dep-info mtimes), cargo now sees all workspace crates as fresh.

See #6244, #6246.

Add `cache-workspace-crates: "true"` to the Swatinem/rust-cache config
for the integration-test-build job. Previously, `cache-all-crates` only
preserved registry crates (downloaded .crate files and extracted sources),
while workspace crate fingerprints, build scripts, and compiled artifacts
in target/ were cleaned before cache save — causing all ~70 local crates
to recompile from scratch every run (~25 min at opt-level=3).

Temporarily sets save-if: true so this PR branch can bootstrap the cache.
Must be reverted to main-only before merge.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@desmondcheongzx desmondcheongzx requested a review from a team as a code owner February 20, 2026 08:46
@github-actions github-actions Bot added the ci label Feb 20, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Feb 20, 2026

Greptile Summary

This PR adds cache-workspace-crates: "true" to the Swatinem/rust-cache configuration for the integration-test-build job. This change preserves compiled artifacts for all ~70 workspace crates in the target/ directory, preventing them from being cleaned during cache save operations.

Key changes:

  • Enables cache-workspace-crates: "true" to include workspace member artifacts in the keep-list
  • Temporarily sets save-if: true to bootstrap the cache (marked with TODO to revert before merge)
  • Works in conjunction with the mtime restoration from ci: share rust cache across branches and restore file mtimes #6246 to ensure cargo sees workspace crates as fresh

Expected impact:

  • Reduces integration build time by ~25 minutes by avoiding recompilation of unchanged workspace crates
  • First build after merge will save the new cache format; subsequent builds should only recompile changed crates

Confidence Score: 5/5

  • This PR is safe to merge after reverting the temporary save-if setting
  • The change is a straightforward CI optimization that adds a single configuration option to cache workspace crates. The implementation follows the action's documented behavior, builds on the mtime fix from ci: share rust cache across branches and restore file mtimes #6246, and includes a clear TODO for the temporary bootstrap setting
  • Verify that the TODO comment on lines 342-344 is addressed before merge (revert save-if: true to main-only)

Important Files Changed

Filename Overview
.github/workflows/pr-test-suite.yml Adds cache-workspace-crates: "true" to preserve compiled workspace artifacts, temporarily enables save-if: true for cache bootstrap (needs revert before merge)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Checkout with fetch-depth: 0] --> B[Restore file mtimes from git history]
    B --> C[Setup Rust with cache: false]
    C --> D[Swatinem/rust-cache restore]
    D --> E{Cache hit?}
    E -->|Yes| F[Restore target/ with workspace artifacts]
    E -->|No| G[Cold build all crates]
    F --> H[Cargo checks mtimes vs dep-info]
    H --> I{Files fresh?}
    I -->|Yes| J[Skip recompilation]
    I -->|No| K[Recompile changed crates only]
    G --> L[Build wheels]
    J --> L
    K --> L
    L --> M{On main branch or save-if: true?}
    M -->|Yes| N[Save cache with workspace artifacts]
    M -->|No| O[Skip cache save]
    N --> P[Upload wheels]
    O --> P
Loading

Last reviewed commit: 50be6b2

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread .github/workflows/pr-test-suite.yml Outdated
Comment on lines +342 to +344
# TODO: revert to main-only before merge:
# save-if: ${{ github.ref == 'refs/heads/main' }}
save-if: true
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.

Revert save-if: true to save-if: ${{ github.ref == 'refs/heads/main' }} before merge

desmondcheongzx and others added 2 commits February 20, 2026 01:25
Warm cache test confirmed: Build wheels dropped from 36 min to 10 sec.
Revert save-if to only save from main branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.43%. Comparing base (94c6f30) to head (5ef868e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6261      +/-   ##
==========================================
- Coverage   73.43%   73.43%   -0.01%     
==========================================
  Files        1001     1001              
  Lines      133185   133096      -89     
==========================================
- Hits        97808    97733      -75     
+ Misses      35377    35363      -14     

see 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@desmondcheongzx desmondcheongzx merged commit f52e4f6 into main Feb 20, 2026
37 checks passed
@desmondcheongzx desmondcheongzx deleted the ci/cache-workspace-crates branch February 20, 2026 19:12
desmondcheongzx added a commit that referenced this pull request Feb 20, 2026
…#6264)

Extract the inline restore-mtime script into a reusable composite action
(`.github/actions/restore-mtime/`) and apply the full cargo cache
treatment to all 10 jobs that use Swatinem/rust-cache:

- `fetch-depth: 0` on checkout (full history for mtime restoration)
- `restore-mtime` composite action (deterministic source file timestamps
for cargo fingerprinting)
- `cache-workspace-crates: "true"` (preserve workspace crate artifacts
in cache — the key fix from #6261)
- `shared-key` by cargo profile (consolidate caches instead of one per
job)
- `save-if: main-only` (single source of truth, avoids PR cache
pollution and LRU eviction)

Cache keys are shared by cargo profile:
- `Linux-dev-build`: unit-test, rust-tests, doctests, test-imports,
style, docgen (build-docs.yml)
- `Linux-integration-build`: integration-test-build, profile-daft,
property-based-tests
- `Linux-dev-bench-build`: benchmark-codspeed (buildjet, separate cache
provider)

Previously each job maintained its own cache via `prefix-key`, resulting
in ~8 separate cache entries for the same profile. Consolidating to 3
shared keys frees significant space against the 10 GB repo cache limit.

On the integration-test-build job (#6261), this approach reduced Build
wheels from 36 min to 10 sec on warm cache. Expect similar improvements
for unit-test (~8 min build) and benchmark-codspeed (~25 min compile +
link).

See #6244, #6246, #6261.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
gavin9402 pushed a commit to gavin9402/Daft that referenced this pull request Apr 7, 2026
…c#6261)

Add `cache-workspace-crates: "true"` to the Swatinem/rust-cache config
for the integration-test-build job.

`cache-all-crates` only controls registry cleanup (downloaded `.crate`
files and extracted sources in `~/.cargo/registry/`). Workspace crate
fingerprints, build script outputs, and compiled `.rlib`/`.rmeta`
artifacts in `target/` are cleaned by `cleanTargetDir()` during cache
save — which builds its keep-list from
`getPackagesOutsideWorkspaceRoot()`, explicitly excluding workspace
members. This caused all ~70 local crates to recompile from scratch
every run (~25 min at `opt-level=3`).

`cache-workspace-crates: "true"` adds workspace members to the keep-list
so their artifacts survive the save cleanup.

## Results

Tested on this PR with temporary `save-if: true` to bootstrap the cache:

| Metric | Cold cache | Warm cache |
|---|---|---|
| Cache restore | 2s | 21s |
| Build wheels | **36 min** | **10 sec** |
| Total job | ~37 min | ~1 min 41 sec |

Combined with the restore-mtime fix from Eventual-Inc#6246 (source file mtimes older
than cached dep-info mtimes), cargo now sees all workspace crates as
fresh.

See Eventual-Inc#6244, Eventual-Inc#6246.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
gavin9402 pushed a commit to gavin9402/Daft that referenced this pull request Apr 7, 2026
…Eventual-Inc#6264)

Extract the inline restore-mtime script into a reusable composite action
(`.github/actions/restore-mtime/`) and apply the full cargo cache
treatment to all 10 jobs that use Swatinem/rust-cache:

- `fetch-depth: 0` on checkout (full history for mtime restoration)
- `restore-mtime` composite action (deterministic source file timestamps
for cargo fingerprinting)
- `cache-workspace-crates: "true"` (preserve workspace crate artifacts
in cache — the key fix from Eventual-Inc#6261)
- `shared-key` by cargo profile (consolidate caches instead of one per
job)
- `save-if: main-only` (single source of truth, avoids PR cache
pollution and LRU eviction)

Cache keys are shared by cargo profile:
- `Linux-dev-build`: unit-test, rust-tests, doctests, test-imports,
style, docgen (build-docs.yml)
- `Linux-integration-build`: integration-test-build, profile-daft,
property-based-tests
- `Linux-dev-bench-build`: benchmark-codspeed (buildjet, separate cache
provider)

Previously each job maintained its own cache via `prefix-key`, resulting
in ~8 separate cache entries for the same profile. Consolidating to 3
shared keys frees significant space against the 10 GB repo cache limit.

On the integration-test-build job (Eventual-Inc#6261), this approach reduced Build
wheels from 36 min to 10 sec on warm cache. Expect similar improvements
for unit-test (~8 min build) and benchmark-codspeed (~25 min compile +
link).

See Eventual-Inc#6244, Eventual-Inc#6246, Eventual-Inc#6261.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants