- Notifications
You must be signed in to change notification settings - Fork664
Add configurable concurrency option#1176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this 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 introduces a configurable concurrency option, replacing the old single-threaded testing flag with a new --concurrency flag that allows detailed control over the number of checkers and threads used during compilation. Key changes include:
- Adding a new "concurrency" field to compiler options and command line declarations.
- Creating a new core package for parsing and handling concurrency values.
- Updating test utilities, the compiler runner, and CI configurations to support the new flag.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tsoptions/parsinghelpers.go | Added parsing for "concurrency" flag in compiler options. |
| internal/tsoptions/declscompiler.go | Declared the new "concurrency" option for the compiler. |
| internal/testutil/harnessutil/harnessutil.go | Set default concurrency from environment if not provided via flag. |
| internal/testrunner/compiler_runner.go | Updated concurrency condition for skipping error baselines. |
| internal/core/concurrency.go | Introduced parsing logic and tests for various concurrency values. |
| internal/compiler/program.go | Updated program initialization to use new concurrency settings. |
| Herebyfile.mjs | Updated environment variable usage to use "concurrency" instead of the old flag. |
| .github/workflows/ci.yml | Updated environment variable in CI to support the new concurrency flag. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
5eb5b31 to99fc50cCompareInstead of |
Except that this also controls the concurrency of the parse/bind/emit stages too... I wasn't happy with any name I came up with. |
Uh oh!
There was an error while loading.Please reload this page.
This adds an experimental
--concurrencyflag. Example usage:--concurrency <default|auto|on|yes>- default behavior, uses 4 checkers, plus all cores for parse/bind/emit. Theoretically we could change this to some magic algorithm.--concurrency <single|off|no>,--singleThreaded- one checker, single threaded parse/bind/emit.--concurrency max- UsesGOMAXPROCScheckers, plus all cores for parse/bind/emit.--concurrency half- UsesGOMAXPROCS/2checkers, plus all cores for parse/bind/emit. (For me, this option is best for checking VS Code.)--concurrency 10- Uses 10 checkers, plus all cores for parse/bind/emit. Can be any number all the way up to the number of files.--concurrency checker-per-filecreates a checker for every file; max concurrency, for testing race conditions.Maybe this is overkill, I can drop some of this.