Skip to content

CLI: Rewrite ForEachAsync to use threadpool, add timeout#40675

Open
dkbennett wants to merge 8 commits into
masterfrom
user/dkbennett/asyncoptimize
Open

CLI: Rewrite ForEachAsync to use threadpool, add timeout#40675
dkbennett wants to merge 8 commits into
masterfrom
user/dkbennett/asyncoptimize

Conversation

@dkbennett
Copy link
Copy Markdown
Member

Summary of the Pull Request

This is an improvement of the ForEachAsync generic method to use the windows thread pool and keep the pool full of workers whenever one completes instead of waiting for every worker to be complete before starting another batch. This also adds a timeout to the method so workers do not execute endlessly and does some refactoring for easier debugging.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

  • Rewrites ForEachAsync, refactoring out classes and methods into details instead of lambdas for easier debugging and nicer stack frames.
  • Uses windows thread pool and wil wrappers consistent with elsewhere in the codebase.
  • Fills the pool and keeps it full as work completes so longer running threads dont hold up work on other threads and we have full utilization for the duration.
  • Handles timeouts and cancellation of all workers if one worker times out.
  • Adds to and updates tests.
  • Updates container stats usage of this to support the cancellation event.

Validation Steps Performed

  • Manual running of stats with many containers.
  • Updated unit tests run quickly and cleanly.

Copilot AI review requested due to automatic review settings May 29, 2026 23:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 rewrites the WSLC ForEachAsync helper to use the Windows thread pool with bounded concurrency and cooperative cancellation, then updates container stats collection and unit coverage to use the new API.

Changes:

  • Replaces std::async batch execution with a reusable thread-pool worker implementation.
  • Adds timeout/cancel-drain parameters and updates container stats to pass a cancellation handle.
  • Expands unit tests for pool sizing, dispatching beyond the initial pool, timeout, and cancellation behavior.

Reviewed changes

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

File Description
src/windows/wslc/core/AsyncExecution.h Reimplements ForEachAsync with thread-pool workers, cancellation, and timeout handling.
src/windows/wslc/tasks/ContainerTasks.cpp Updates stats collection to the new ForEachAsync signature and timeout parameters.
test/windows/wslc/WSLCCLIExecutionUnitTests.cpp Updates existing tests and adds coverage for new pool and timeout behaviors.

Comment thread src/windows/wslc/core/AsyncExecution.h Outdated
Comment thread src/windows/wslc/core/AsyncExecution.h Outdated
Comment thread src/windows/wslc/core/AsyncExecution.h Outdated
Comment thread src/windows/wslc/tasks/ContainerTasks.cpp Outdated
Copilot AI review requested due to automatic review settings May 30, 2026 00:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/windows/wslc/core/AsyncExecution.h
Comment thread src/windows/wslc/core/AsyncExecution.h
Comment thread src/windows/wslc/core/AsyncExecution.h
Comment thread src/windows/wslc/tasks/ContainerTasks.cpp Outdated
Copilot AI review requested due to automatic review settings May 30, 2026 00:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/windows/wslc/core/AsyncExecution.h Outdated
Comment thread src/windows/wslc/core/AsyncExecution.h Outdated
Comment thread src/windows/wslc/core/AsyncExecution.h
Copilot AI review requested due to automatic review settings May 30, 2026 00:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/windows/wslc/core/AsyncExecution.h
Comment thread src/windows/wslc/core/AsyncExecution.h Outdated
@dkbennett dkbennett marked this pull request as ready for review May 30, 2026 08:12
@dkbennett dkbennett requested a review from a team as a code owner May 30, 2026 08:12
Copilot AI review requested due to automatic review settings May 30, 2026 08:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment on lines +234 to +236
// If onSuccess, onError, or Launch throw, the cancel event is signalled and ForEachAsync
// waits up to cancelDrainTimeout for any in-flight workers to exit before rethrowing.
// This guarantees no background thread pool callbacks outlive the ForEachAsync call.
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