Support running pub concurrently up to 1/2 the number of processors#5908
Support running pub concurrently up to 1/2 the number of processors#5908
Conversation
With an option (`dart.runPubConcurrently`) to opt-out. This only applies to the proper batches, and not to manually triggered pubs invocations (which will still queue). Fixes #5907
DanTup
added
is enhancement
|
@codex review /gemini review |
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces concurrent execution for pub commands, which is a significant performance improvement for multi-project workspaces. The implementation uses a worker-based concurrency limit and provides a unified progress reporting mechanism. I have identified a logic bug in the new resolvePackageTargetUri helper and a UX regression in the progress display for batch operations. Additionally, there is an inaccurate comment in sdk.ts regarding the timeout duration.
![chatgpt-codex-connector[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a8da1c280
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR adds support for running pub operations concurrently across multiple folders, up to half the number of available CPU cores. This significantly improves performance when working with large mono-repos containing many packages (like flutter-packages with 205 projects).
Changes:
- Added concurrent execution support for batch pub/flutter operations with a configurable concurrency limit (defaulting to CPU count / 2)
- Added a new configuration option
dart.runPubConcurrently(defaults to true) to allow users to opt-out - Refactored batch operation progress reporting to show active packages and completion counts
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/dart/commands/pub_concurrency.test.ts | New test file for concurrency utilities |
| src/shared/pub/utils.ts | Core concurrency utilities and max concurrent process calculation |
| src/extension/config.ts | New configuration option for concurrent pub execution |
| src/extension/commands/sdk.ts | Removed old progress tracking code and updated timeout constant |
| src/extension/commands/packages.ts | Refactored to use new batch operation handler |
| src/extension/commands/flutter.ts | Refactored flutter clean to use new batch operation handler |
| src/extension/commands/batch_progress.ts | New progress tracking for concurrent batch operations |
| package.json | Configuration schema for the new setting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #5908 +/- ##
==========================================
+ Coverage 67.51% 67.67% +0.16%
==========================================
Files 168 169 +1
Lines 12939 12976 +37
Branches 2563 2564 +1
==========================================
+ Hits 8736 8782 +46
+ Misses 3749 3741 -8
+ Partials 454 453 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces concurrent execution for batch pub operations, controlled by a new setting dart.runPubConcurrently. The concurrency is limited to half the number of CPU cores. This is implemented with a new generic runWithConcurrencyLimit utility. The existing commands for batch operations (flutter clean, pub get, pub upgrade) are refactored to use this new concurrent mechanism, which cleans up the code. A new file batch_progress.ts is added to handle progress reporting for these batch operations. Tests for the new concurrency logic are also included.
The changes are well-structured and the new concurrency utility is robust. I have one suggestion to improve the stability of the progress message to reduce flickering.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
With an option (
dart.runPubConcurrently) to opt-out.This only applies to the proper batches, and not to manually triggered pubs invocations (which will still queue).
Fixes #5907