Skip to content

perf(sync): raise the default fetch ceiling floor to 50 threads#65

Open
srnnkls wants to merge 3 commits into
mainfrom
fetch-ceiling
Open

perf(sync): raise the default fetch ceiling floor to 50 threads#65
srnnkls wants to merge 3 commits into
mainfrom
fetch-ceiling

Conversation

@srnnkls

@srnnkls srnnkls commented Jul 10, 2026

Copy link
Copy Markdown
Owner

What

When --jobs is unset, the default rayon pool size becomes min(units, max(2×cores, 50)) instead of min(units, 2×cores). --jobs still overrides. Also fixes the benches/fetch_sweep.sh reporter, which BSD awk (macOS) rejected outright.

Why

benches/fetch_sweep.sh (re-run 2026-07-10, 8-core machine; consistent with the 2026-06 baseline in the async-fetch scope findings) shows fetch is network-wait-bound and the pool ceiling — not the box — is the limit for many small sources:

20 tiny git sources, cold real cpu io_wait
jobs=1 14.49s 0.61s 96%
jobs=16 (old default cap) 1.54s 0.68s 56%
jobs=50 0.95s 0.76s 20%

Aggregate CPU stays flat as jobs rise (heavy fixture: 13.96s → 12.59s across jobs 8→32), so the wider ceiling costs parked-thread memory, not cycles. The floor of 50 captures the measured win; boxes where 2×cores already exceeds 50 keep their existing ceiling.

Considered and rejected

Separating the fetch pool from the compute pool: the same sweep shows the machine is never CPU-saturated during sync (~0.44 of 8 cores busy on 11 medium/large repos) and there is no oversubscription penalty, so a dedicated compute pool has nothing to relieve.

Verification

  • cargo test: 1349 lib + all integration suites green
  • cargo clippy --all-targets -- -D warnings: clean
  • benches/fetch_sweep.sh runs end-to-end again (reporter fix) — numbers above are from this branch's harness

srnnkls added 2 commits July 10, 2026 02:15
Fetch is network-wait-bound: the sweep shows 96% io-wait at jobs=1 and
aggregate CPU flat up to jobs=50, while 20 tiny sources drop 1.54s -> 0.95s
going from jobs=16 to jobs=50 on 8 cores. Parked threads cost memory, not
CPU, so the default ceiling becomes max(2*cores, 50); --jobs still
overrides.
macOS awk rejects an unparenthesized ternary in a printf argument list;
compute the wait percentage in a separate statement.
Copilot AI review requested due to automatic review settings July 10, 2026 00:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR increases the default Rayon thread-pool size used by sync when --jobs is not provided, raising the pool cap to at least 50 threads to better overlap network waits during fetch-heavy sync workloads. It also fixes the benchmark sweep script’s awk reporter to be compatible with BSD awk (macOS).

Changes:

  • Raise default pool cap from 2×cores to max(2×cores, 50) (still bounded by units and still overridden by --jobs).
  • Update/expand unit tests to cover the new cap-floor behavior.
  • Fix benches/fetch_sweep.sh awk logic for BSD awk portability.

Reviewed changes

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

File Description
src/sync/resolve.rs Adjusts default sync Rayon thread count cap (adds a minimum cap of 50) and updates tests/docs accordingly.
benches/fetch_sweep.sh Makes the benchmark output formatter compatible with BSD awk by avoiding the prior inline conditional expression.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/sync/resolve.rs Outdated
Comment thread src/sync/resolve.rs
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