fix(processConcurrency): improve process concurrency count#143
fix(processConcurrency): improve process concurrency count#143yamadashy merged 4 commits intoyamadashy:mainfrom
Conversation
|
|
📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve the modification of the Changes
Sequence Diagram(s)sequenceDiagram
participant A as User
participant B as getProcessConcurrency
participant C as os
A->>B: Call getProcessConcurrency()
B->>C: Call os.availableParallelism()
C-->>B: Return available CPU count
B-->>A: Return adjusted CPU count (Math.max(1, cpuCount - 1))
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/shared/processConcurrency.ts (1 hunks)
🔇 Additional comments (1)
src/shared/processConcurrency.ts (1)
4-7: LGTM! Good practice for CPU allocation.
The implementation correctly:
- Uses
availableParallelism()which is more accurate thancpus().length - Reserves one CPU for system tasks with
cpuCount - 1 - Ensures at least one CPU is available with
Math.max(1, ...)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #143 +/- ##
==========================================
+ Coverage 75.29% 75.37% +0.08%
==========================================
Files 36 36
Lines 1550 1547 -3
Branches 270 270
==========================================
- Hits 1167 1166 -1
+ Misses 383 381 -2 ☔ View full report in Codecov by Sentry. |
`|| 1` is not needed since we already have Math.max
|
Hi, @twlite I wasn't aware of os.availableParallelism() - this is an excellent improvement that enables more accurate process concurrency control. The implementation looks good, so I'll go ahead and merge this PR. |
This method returns an estimate of the default amount of parallelism a program should use, compared to
cpus().lengthwhich returns the cpu core count.