Skip to content

Replace per-iteration std::async with a persistent thread pool#332

Open
saleelk wants to merge 1 commit into
candidatefrom
saleelk/ThreadPoolFix
Open

Replace per-iteration std::async with a persistent thread pool#332
saleelk wants to merge 1 commit into
candidatefrom
saleelk/ThreadPoolFix

Conversation

@saleelk

@saleelk saleelk commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The executor/transfer dispatch created a fresh std::thread on every iteration via std::async, adding thread-creation overhead and jitter to the measured host time.
  • Introduce a persistent ThreadPool created once per RunTransfers call, with separate executor and transfer pools to avoid a nested-blocking deadlock, and a ParallelRun helper that runs unit 0 inline and farms the rest to the pool. Reduces executor-time variance on unpinned runs.
  • Pool pointers are passed explicitly through the call chain (RunExecutorRunGpuExecutor/RunDmaExecutor/RunBmaExecutor) rather than stored as globals, keeping dispatch reentrant and thread-safe.
  • ParallelRun promotes to the most severe ErrType across all units (ERR_FATAL > ERR_WARN > ERR_NONE).

Test Plan

  • Ran p2p preset on MI300X (gfx942) — unidirectional and bidirectional bandwidth numbers match baseline.
  • Verified no regressions in per-transfer timing accuracy.

@saleelk saleelk requested a review from a team as a code owner July 13, 2026 00:47
@nileshnegi nileshnegi requested a review from Copilot July 13, 2026 05:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 replaces per-iteration std::async dispatch (which creates new threads repeatedly) with a persistent in-process ThreadPool and a ParallelRun helper, aiming to reduce thread-creation overhead and timing jitter during RunTransfers.

Changes:

  • Added a persistent ThreadPool implementation and ParallelRun helper for running “unit 0” inline and farming remaining work to the pool.
  • Switched executor-level and transfer-level parallelism from std::async to ParallelRun using separate executor and transfer pools.
  • Created/destroyed the pools once per RunTransfers invocation and wired them through global pool pointers.

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

Comment thread src/header/TransferBench.hpp
Comment thread src/header/TransferBench.hpp Outdated
Comment thread src/header/TransferBench.hpp Outdated
- The executor/transfer dispatch created a fresh std::thread on every
  iteration via std::async, adding thread-creation overhead and jitter to
  the measured host time.
- Introduce a persistent ThreadPool created once per RunTransfers call,
  with separate executor and transfer pools to avoid a nested-blocking
  deadlock, and a ParallelRun helper that runs unit 0 inline and farms
  the rest to the pool. Reduces executor-time variance on unpinned runs.
@saleelk saleelk force-pushed the saleelk/ThreadPoolFix branch from 5537c30 to 94fa8ce Compare July 14, 2026 19:27
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