Skip to content

perf: stream prune integration checks, parallel removals - #1950

Merged
max-sixty merged 6 commits into
mainfrom
prune-progress
Apr 7, 2026
Merged

perf: stream prune integration checks, parallel removals#1950
max-sixty merged 6 commits into
mainfrom
prune-progress

Conversation

@max-sixty

@max-sixty max-sixty commented Apr 7, 2026

Copy link
Copy Markdown
Owner

Replace the blocking par_iter().collect() barrier in step_prune with channel-based streaming. Integration checks run on a background thread via rayon, sending results through a crossbeam channel as they complete. The main thread processes results as they arrive for age filtering and candidate collection, overlapping with still-running checks.

Phase 2 removes confirmed candidates in parallel via into_par_iter(). Candidates are sorted by original check order for deterministic output.

Performance (100 BranchOnly removals, release build):

Metric Before After
Non-dry-run (100 branches) 7.8s 2.7s
CPU utilization 139% 485%

Tests with multiple removal candidates pin RAYON_NUM_THREADS=1 for deterministic snapshot output.

This was written by Claude Code on behalf of @max-sixty

Replace the blocking `par_iter().collect()` barrier in `step_prune` with
channel-based streaming. Integration checks run on a background thread
via rayon, sending results through a crossbeam channel as they complete.
The main thread processes results immediately for age filtering and
candidate collection, overlapping with still-running checks.

Phase 2 removes confirmed candidates in parallel via `into_par_iter()`,
with candidates sorted by original check order for deterministic output
when rayon processes items sequentially.

Performance (100 BranchOnly removals, release build):
- Non-dry-run: 7.8s → 2.7s (parallel removals, 485% CPU utilization)
- Dry-run first output: 2.3s silence → 0.2s progress message
- Dry-run total: ~0.7s → ~0.6s

Co-Authored-By: Claude <noreply@anthropic.com>
worktrunk-bot
worktrunk-bot previously approved these changes Apr 7, 2026
@worktrunk-bot
worktrunk-bot dismissed their stale review April 7, 2026 00:38

codecov/patch failed (94.8% — 4 uncovered lines in new code)

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

codecov/patch failed — 94.8% patch coverage (4 uncovered lines of 77).

The code itself looks good (clean channel pattern, correct deterministic ordering, proper cleanup). The uncovered lines are:

  1. Progress message (L1549–1552): Behind check_items.len() > 10 — no test creates that many branches. This is UX-only, no logic.

  2. drop(tx) (L1578): Empty check_items path. Would need a test with a repo that has only the default branch and no other worktrees/branches.

  3. Non-dry-run skipped_young report (L1744–1750): The refactoring duplicated the "Skipped X (younger than Y)" message into both dry-run and non-dry-run branches. The existing test_prune_min_age_skips_young and test_prune_stale_plus_young tests only exercise dry-run. A non-dry-run variant with --yes --min-age=1h and a young worktree would cover this.

The easiest fix for coverage is #3 — add a --yes (non-dry-run) variant of one of the existing min-age tests.

Exercises the skipped_young message in the non-dry-run removal path
(L1744-1750), which was flagged as uncovered by codecov/patch.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Removes the `if !integration_refs.is_empty()` / `else { drop(tx) }`
branch. Empty `par_iter` completes immediately, so always spawning the
thread is simpler and eliminates an uncovered code path.

Co-Authored-By: Claude <noreply@anthropic.com>
The `> 10` threshold prevented any test from covering the progress
message. Removing it rather than lowering the threshold avoids changing
all prune test snapshots. The channel approach already provides the
UX improvement (overlapped processing).

Co-Authored-By: Claude <noreply@anthropic.com>
worktrunk-bot
worktrunk-bot previously approved these changes Apr 7, 2026
@worktrunk-bot
worktrunk-bot dismissed their stale review April 7, 2026 01:53

CI failed — snapshot mismatch in test_prune_removes_integrated_detached

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Linux CI failed: test_prune_removes_integrated_detached snapshot mismatch (log).

This test produces two parallel candidates (the detached worktree + the orphan detached-integrated branch), but doesn't pin RAYON_NUM_THREADS=1. The branch deletion finishes before the worktree removal, flipping the output order. Other multi-candidate tests (lines 144, 224, 288, 324) already pin it.

Fix in tests/integration_tests/step_prune.rs at line 197 — same pattern as test_prune_multiple:

    let mut cmd = make_snapshot_cmd(
        &repo,
        "step",
        &["prune", "--yes", "--min-age=0s"],
        None
    );
    cmd.env("RAYON_NUM_THREADS", "1"); // deterministic output order
    assert_cmd_snapshot!(cmd);

Then cargo insta test --accept to update the snapshot. I can push this fix if you'd like.

…rune_removes_integrated_detached on linux)

Co-Authored-By: Claude <noreply@anthropic.com>
@max-sixty
max-sixty merged commit da5a390 into main Apr 7, 2026
23 checks passed
@max-sixty
max-sixty deleted the prune-progress branch April 7, 2026 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants