Skip to content

perf: remove coordinator result lock contention#335

Merged
ciaranra merged 3 commits into
devfrom
copilot-effective-spoon
Jul 8, 2026
Merged

perf: remove coordinator result lock contention#335
ciaranra merged 3 commits into
devfrom
copilot-effective-spoon

Conversation

@qartik

@qartik qartik commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • remove the shared Arc<Mutex<Vec<_>>> result collector and final global sort from ParallelCoordinator::run()
  • collect each worker output locally with Rayon and flatten in worker order after the parallel phase
  • add a regression test that locks in deterministic worker-order result aggregation

Why this change

The previous implementation forced every worker to contend on one mutex while pushing results, then paid an extra global sort to restore deterministic ordering. This refactor keeps the ordering guarantee while removing both sources of overhead from the hot parallel coordinator path.

Validation

  • cargo test -p pecos-neo coordinator --quiet
  • cargo test -p pecos-neo --quiet

Performance metrics (before/after)

Benchmark command:

cargo bench -p benchmarks --bench benchmarks -- 'ParallelCoordinator/'

Before = origin/dev (602ebd0)
After = PR head (f300415)

Benchmark Before (time) After (time) Delta (time) Delta (throughput)
ParallelCoordinator / 100 shots 163.56 µs 158.60 µs -4.84% +5.08%
ParallelCoordinator / 1,000 shots 1.0301 ms 1.0097 ms -5.28% +5.57%
ParallelCoordinator / 10,000 shots 10.647 ms 10.289 ms -3.36% +3.48%

Criterion reported "Performance has improved." for all three shot sizes in the after run.

Collect per-worker results and stats via rayon map/collect, avoiding the shared results mutex and final sort in ParallelCoordinator::run. This keeps deterministic worker ordering while removing a serialization point from the hot parallel path.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@qartik
qartik requested a review from ciaranra as a code owner July 2, 2026 11:06
@qartik
qartik requested a review from qciaran July 7, 2026 20:56
@ciaranra

ciaranra commented Jul 8, 2026

Copy link
Copy Markdown
Member

Looks good to me. (Straight-foward improvement.) Will wait for CI and do a quick review.

@ciaranra ciaranra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Thanks!

@ciaranra

ciaranra commented Jul 8, 2026

Copy link
Copy Markdown
Member

I am guessing I might still need to merge due to the "verified" thing which I probably need to look at turning off...

@ciaranra

ciaranra commented Jul 8, 2026

Copy link
Copy Markdown
Member

I'll go ahead and merge :D

@ciaranra
ciaranra merged commit 6dde81d into dev Jul 8, 2026
19 checks passed
@ciaranra
ciaranra deleted the copilot-effective-spoon branch July 8, 2026 15:28
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