Skip to content

implement xargs -P using rayon and a spin lock - #794

Open
codspeed-hq[bot] wants to merge 3 commits into
mainfrom
push-zlxwkxlkpsls
Open

implement xargs -P using rayon and a spin lock#794
codspeed-hq[bot] wants to merge 3 commits into
mainfrom
push-zlxwkxlkpsls

Conversation

@codspeed-hq

@codspeed-hq codspeed-hq Bot commented Jul 26, 2026

Copy link
Copy Markdown

Add CodSpeed benchmarks for the parallel xargs -P path

This PR introduces parallel execution for xargs -P (via rayon), but the existing benchmark suite (benches/xargs_bench.rs) only exercised the serial path — so CodSpeed reported the new code as untouched.

This change extends benches/xargs_bench.rs with three benchmarks that drive the new rayon-based dispatch in process_input:

  • parallel_p2-n 10 -P 2: a low degree of parallelism. This is the spin-lock worst case noted earlier in the review (high contention between two workers).
  • parallel_p8-n 10 -P 8: higher parallelism, exercising the scheduler under more contention.
  • parallel_p8_batched_n-n 50 -P 8: fewer, larger batches so the balance shifts back toward argument assembly while still going through the parallel bridge.

Implementation notes

  • The parallel benchmarks use a smaller dedicated corpus (400 tokens) than the serial ones (4000). Each batch spawns a real true child, so a smaller corpus keeps the number of spawns reasonable under the CodSpeed simulation instrument while still producing enough batches to feed the parallel bridge.
  • The serial benchmarks (split_whitespace, split_null, batched_n, batched_size) are unchanged.
  • Measurements are dominated by xargs's own work (reading, splitting, batching, and parallel dispatch); child-process spawns are not traced under simulation.

All benchmarks build and run successfully under codspeed run --mode simulation.

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.38843% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.15%. Comparing base (2a3eac9) to head (3ec89f4).

Files with missing lines Patch % Lines
src/xargs/mod.rs 93.38% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #794   +/-   ##
=======================================
  Coverage   92.15%   92.15%           
=======================================
  Files          35       35           
  Lines        7377     7457   +80     
  Branches      383      384    +1     
=======================================
+ Hits         6798     6872   +74     
- Misses        438      442    +4     
- Partials      141      143    +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

Commit 017bea8 has test result changes:

GNU findutils testsuite:

Test results comparison:
  Current:   TOTAL: 495 / PASSED: 413 / FAILED: 81 / SKIPPED: 1
  Reference: TOTAL: 495 / PASSED: 417 / FAILED: 77 / SKIPPED: 1

Changes from main branch:
  TOTAL: +0
  PASSED: -4
  FAILED: +4

New test failures (4):
  - idef-s26-0
  - s14
  - s14-0
  - s42

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 314 / PASSED: 267 / FAILED: 41 / SKIPPED: 6
  Reference: TOTAL: 314 / PASSED: 267 / FAILED: 41 / SKIPPED: 6

New test failures (1):
  - gnu/files0_from_ok

Test improvements (1):
  + gnu/okdir_path_empty

@codspeed-hq

codspeed-hq Bot commented Jul 26, 2026

Copy link
Copy Markdown
Author

Merging this PR will not alter performance

✅ 20 untouched benchmarks
🆕 3 new benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
🆕 parallel_p2 N/A 564.3 ms N/A
🆕 parallel_p8 N/A 22.4 ms N/A
🆕 parallel_p8_batched_n N/A 6 ms N/A

Comparing push-zlxwkxlkpsls (3ec89f4) with main (2a3eac9)

Open in CodSpeed

@codspeed-hq
codspeed-hq Bot marked this pull request as ready for review July 26, 2026 18:23
@sylvestre

Copy link
Copy Markdown
Contributor

could you please fix the conflicts? thanks

@codspeed-hq

codspeed-hq Bot commented Aug 18, 2026

Copy link
Copy Markdown
Author

could you please fix the conflicts? thanks

Done — main is merged into push-zlxwkxlkpsls and the conflicts are resolved.

Conflicts and how they were resolved

  • src/xargs/mod.rs
    • Import block: kept the rayon::iter imports from this branch and main's uucore::error::strip_errno.
    • --arg-file opening: kept the Box<dyn Read + Send> type (the Send bound is required by the parallel path) while adopting main's strip_errno-based error message.
  • tests/test_xargs.rs: kept both sides — the new xargs_parallel_one / xargs_parallel_many tests from this branch, plus main's xargs_trailing_blanks / xargs_quoted_empty_argument.

main's rework of the whitespace-delimited argument reader (the new in_argument tracking) merged cleanly and is preserved unchanged.

Verification

  • cargo build --all-targets: OK
  • cargo test --test test_xargs: 34 passed, 0 failed
  • cargo clippy --all-targets: no new warnings (only a pre-existing one in src/locate/mod.rs coming from main)
  • codspeed run --mode simulation -- cargo codspeed run --bench xargs_bench: all 7 benchmarks (4 serial + 3 parallel) run successfully

One note from that local run: parallel_p2 is by far the heaviest benchmark (~207 ms locally vs ~3.5 ms for parallel_p8), which matches the expectation that two spinning workers are the worst case for the spin lock. The upcoming CI report will give the reference numbers on the CodSpeed runners.

Summary
  • Warming up the performance wizard...
  • Handling comment — Comment handling finished successfully
  • Resolving merge conflicts with main — Conflicts in src/xargs/mod.rs and tests/test_xargs.rs resolved; build and xargs tests pass
  • Validating benchmarks — All 7 xargs benchmarks ran successfully under the simulation instrument
  • Pushing the merge — Merge commit pushed to push-zlxwkxlkpsls

Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.

Resolve conflicts in src/xargs/mod.rs (keep the rayon imports alongside
the new strip_errno error formatting, and keep the Send bound on the
argument reader required by the parallel path) and in tests/test_xargs.rs
(keep both the new parallel tests and the argument-splitting tests from
main).
@github-actions

Copy link
Copy Markdown

Commit 3ec89f4 has test result changes:

GNU findutils testsuite:

Test results comparison:
  Current:   TOTAL: 495 / PASSED: 417 / FAILED: 77 / SKIPPED: 1
  Reference: TOTAL: 495 / PASSED: 421 / FAILED: 73 / SKIPPED: 1

Changes from main branch:
  TOTAL: +0
  PASSED: -4
  FAILED: +4

New test failures (4):
  - idef-s26-0
  - s14
  - s14-0
  - s42

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 316 / PASSED: 273 / FAILED: 37 / SKIPPED: 6
  Reference: TOTAL: 317 / PASSED: 274 / FAILED: 37 / SKIPPED: 6

Changes from main branch:
  TOTAL: -1
  PASSED: -1
  FAILED: +0

No result in this run (1) - hung, crashed, or renamed:
  ? posix/o (was PASS)

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.

3 participants