Repository navigation
Revert "perf(ci): pin Rust deps with a GC root" - #1275
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR removes a Nix garbage-collection root pinning mechanism from both the CI workflow and the static package derivation. The GitHub Action no longer creates a GC-root directory or invokes ChangesRemove Nix GC-root pinning infrastructure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — clean revert of #1274 after confirming the GC-root pin didn't prevent full dependency rebuilds.
- Revert CI GC-root pin step — removes the "Pin dependency cache root" step from
.github/actions/build-linux-native-package/action.yaml - Remove
passthru.cargoArtifactsfrom Nix — drops thestatic-package.nixpassthru that exposed deps artifacts for GC-root pinning
Big Pickle (free via Pullfrog for OSS) ｜ 𝕏
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
Reverts #1274.
The GC-root pin did not fix the dependency rebuild: a rerun of
build-linux-arm64still rebuilt all deps (~234s) with the root in place. The sticky disk persists
~24GB for this job (confirmed in the Blacksmith dashboard), so the store is being
retained but the
cargoArtifactspath is still missing on restore — the rootdidn't prevent it. Reverting the no-op change while we inspect the disk directly
via a Blacksmith testbox.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Reverts the CI GC-root pin for Rust dependency artifacts and removes the
cargoArtifactspassthru in Nix, since the change didn’t reduce rebuilds. This returns the Linux build to previous behavior while we debug the sticky disk separately..github/actions/build-linux-native-package/action.yaml.passthru.cargoArtifactsfromnix/static-package.nix.Written for commit fa9b08b. Summary will update on new commits.
Summary by CodeRabbit