Add --builders to limit number of projects can build concurrently #2307
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for the --builders command-line flag to limit the number of projects that can build concurrently in TypeScript project builds. The implementation introduces:
- A new
--buildersoption that accepts a numeric value (minimum 1) - Semaphore-based concurrency control in the build orchestrator
- Validation to ensure the value is greater than or equal to 1
- Test coverage for various scenarios including error cases
- Updated baseline files demonstrating the feature with different project counts
Reviewed changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
internal/core/buildoptions.go |
Adds Builders field to BuildOptions struct |
internal/tsoptions/declsbuild.go |
Declares the --builders command-line option with validation |
internal/tsoptions/declscompiler.go |
Adds minimum value validation to checkers option |
internal/tsoptions/parsinghelpers.go |
Adds parsing logic for the builders option |
internal/tsoptions/commandlineparser.go |
Implements validation for minimum value constraint on numeric options |
internal/tsoptions/commandlineoption.go |
Adds minValue field to CommandLineOption struct |
internal/execute/build/orchestrator.go |
Initializes semaphore for limiting concurrent builds |
internal/execute/build/buildtask.go |
Acquires/releases semaphore slots during compilation |
internal/diagnostics/extraDiagnosticMessages.json |
Adds diagnostic messages for the feature |
internal/diagnostics/diagnostics_generated.go |
Generated diagnostic code |
internal/execute/tsctests/tscbuild_test.go |
Adds test scenarios for the --builders flag |
internal/tsoptions/commandlineparser_test.go |
Adds validation tests for edge cases |
| Multiple baseline files | Test output baselines showing expected behavior |
jakebailey
left a comment
jakebailey
left a comment
There was a problem hiding this comment.
I tested, and this seems to work. On the old compiler, --builders 1 performs the same as letting it be unconstrained. I need to find a better test case.
| } | ||
| // If we want to build more than one project at a time, create a semaphore to limit concurrency | ||
| if builders := opts.Command.BuildOptions.Builders; builders != nil { | ||
| orchestrator.buildSemaphore = make(chan struct{}, *builders) |
There was a problem hiding this comment.
Do we need to max(1, *builders) like we do for the checker count?
There was a problem hiding this comment.
Ah, I see that you added something to config parsing for this.
| func (t *BuildTask) compileAndEmit(orchestrator *Orchestrator, path tspath.Path) { | ||
| if orchestrator.buildSemaphore != nil { | ||
| orchestrator.buildSemaphore <- struct{}{} // acquire slot | ||
| defer func() { <-orchestrator.buildSemaphore }() // release slot |
There was a problem hiding this comment.
Just to double check, we don't actually load anything related to the project until after this point, right?
There was a problem hiding this comment.
correct. we actually create program and do emit after this.
this does not affect uptodate ness checks
Per #1622 (comment)