diff --git a/tsc/internal/execute/build/buildtask.go b/tsc/internal/execute/build/buildtask.go index 96d776a4ccc3a..f3cbb41e298c8 100644 --- a/tsc/internal/execute/build/buildtask.go +++ b/tsc/internal/execute/build/buildtask.go @@ -61,9 +61,8 @@ type BuildTask struct { done chan struct{} // task reporting - result *taskResult - prevReporter *BuildTask - reportDone chan struct{} + result *taskResult + built chan struct{} // closed when result is ready to be reported buildInfoEntry *buildInfoEntry buildInfoEntryMu sync.Mutex @@ -118,9 +117,6 @@ func (t *BuildTask) reportDiagnostic(err *ast.Diagnostic) { } func (t *BuildTask) report(orchestrator *Orchestrator, configPath tspath.Path, buildResult *orchestratorResult) { - if t.prevReporter != nil { - <-t.prevReporter.reportDone - } if len(t.errors) > 0 { buildResult.errors = append(core.IfElse(buildResult.errors != nil, buildResult.errors, []*ast.Diagnostic{}), t.errors...) } @@ -144,7 +140,6 @@ func (t *BuildTask) report(orchestrator *Orchestrator, configPath tspath.Path, b } buildResult.filesToDelete = append(buildResult.filesToDelete, t.result.filesToDelete...) t.result = nil - close(t.reportDone) } func (t *BuildTask) buildProject(orchestrator *Orchestrator, path tspath.Path) { diff --git a/tsc/internal/execute/build/graph_test.go b/tsc/internal/execute/build/graph_test.go index f3a38aa2b4e16..471d8a483daa4 100644 --- a/tsc/internal/execute/build/graph_test.go +++ b/tsc/internal/execute/build/graph_test.go @@ -16,15 +16,15 @@ import ( func TestBuildOrderGenerator(t *testing.T) { t.Parallel() testCases := []*buildOrderTestCase{ - {"specify two roots", []string{"A", "G"}, []string{"D", "E", "C", "B", "A", "G"}, false}, - {"multiple parts of the same graph in various orders", []string{"A"}, []string{"D", "E", "C", "B", "A"}, false}, - {"multiple parts of the same graph in various orders", []string{"A", "C", "D"}, []string{"D", "E", "C", "B", "A"}, false}, - {"multiple parts of the same graph in various orders", []string{"D", "C", "A"}, []string{"D", "E", "C", "B", "A"}, false}, - {"other orderings", []string{"F"}, []string{"E", "F"}, false}, - {"other orderings", []string{"E"}, []string{"E"}, false}, - {"other orderings", []string{"F", "C", "A"}, []string{"E", "F", "D", "C", "B", "A"}, false}, - {"returns circular order", []string{"H"}, []string{"E", "J", "I", "H"}, true}, - {"returns circular order", []string{"A", "H"}, []string{"D", "E", "C", "B", "A", "J", "I", "H"}, true}, + {"specify two roots", []string{"A", "G"}, []string{"D", "E", "C", "B", "A", "G"}, []string{"D", "E", "G", "C", "B", "A"}, false}, + {"multiple parts of the same graph in various orders", []string{"A"}, []string{"D", "E", "C", "B", "A"}, []string{"D", "E", "C", "B", "A"}, false}, + {"multiple parts of the same graph in various orders", []string{"A", "C", "D"}, []string{"D", "E", "C", "B", "A"}, []string{"D", "E", "C", "B", "A"}, false}, + {"multiple parts of the same graph in various orders", []string{"D", "C", "A"}, []string{"D", "E", "C", "B", "A"}, []string{"D", "E", "C", "B", "A"}, false}, + {"other orderings", []string{"F"}, []string{"E", "F"}, []string{"E", "F"}, false}, + {"other orderings", []string{"E"}, []string{"E"}, []string{"E"}, false}, + {"other orderings", []string{"F", "C", "A"}, []string{"E", "F", "D", "C", "B", "A"}, []string{"E", "D", "F", "C", "B", "A"}, false}, + {"returns circular order", []string{"H"}, []string{"E", "J", "I", "H"}, []string{"E", "J", "I", "H"}, true}, + {"returns circular order", []string{"A", "H"}, []string{"D", "E", "C", "B", "A", "J", "I", "H"}, []string{"D", "E", "C", "J", "B", "I", "A", "H"}, true}, } for _, testcase := range testCases { testcase.run(t) @@ -32,10 +32,11 @@ func TestBuildOrderGenerator(t *testing.T) { } type buildOrderTestCase struct { - name string - projects []string - expected []string - circular bool + name string + projects []string + expected []string + expectedSchedule []string + circular bool } func (b *buildOrderTestCase) configName(project string) string { @@ -119,6 +120,9 @@ func (b *buildOrderTestCase) run(t *testing.T) { buildOrder := core.Map(orchestrator.Order(), b.projectName) assert.DeepEqual(t, buildOrder, b.expected) verifyDeps(orchestrator, buildOrder, false) + scheduleOrder := core.Map(orchestrator.ScheduleOrder(), b.projectName) + assert.DeepEqual(t, scheduleOrder, b.expectedSchedule) + verifyDeps(orchestrator, scheduleOrder, false) if !b.circular { for project, projectDeps := range deps { diff --git a/tsc/internal/execute/build/orchestrator.go b/tsc/internal/execute/build/orchestrator.go index a5ca48c30c234..2a17d073342a0 100644 --- a/tsc/internal/execute/build/orchestrator.go +++ b/tsc/internal/execute/build/orchestrator.go @@ -80,6 +80,8 @@ type Orchestrator struct { // fswatch event-based watching wm *watchmanager.WatchManager + // order sorted by dependency depth, so builders rarely block on upstream projects + scheduleOrder []string } var _ tsc.Watcher = (*Orchestrator)(nil) @@ -103,6 +105,34 @@ func (o *Orchestrator) Order() []string { return o.order } +// ScheduleOrder is the order in which builders pick up projects: Order() stably sorted by dependency depth. +func (o *Orchestrator) ScheduleOrder() []string { + return o.scheduleOrder +} + +// computeScheduleOrder sorts the build order by dependency depth (projects with no +// upstream first, then their dependents, and so on). Builders take projects from this +// order and block until upstream projects are done, so with the plain depth-first order +// a builder that picks the root of a long chain sits idle while another builder works +// through the chain, even when unrelated projects are ready to build. Depth order avoids +// that blocking: when a builder takes a project, every shallower project has already been +// picked up. A pure chain still serializes, since nothing else is buildable. The stable +// sort preserves the original order within a depth, and reporting still follows Order(). +func (o *Orchestrator) computeScheduleOrder() []string { + depth := make(map[*BuildTask]int, len(o.order)) + for _, config := range o.order { + task := o.getTask(o.toPath(config)) + for _, upstream := range task.upStream { + depth[task] = max(depth[task], depth[upstream.task]+1) + } + } + scheduleOrder := slices.Clone(o.order) + slices.SortStableFunc(scheduleOrder, func(a, b string) int { + return depth[o.getTask(o.toPath(a))] - depth[o.getTask(o.toPath(b))] + }) + return scheduleOrder +} + func (o *Orchestrator) Upstream(configName string) []string { path := o.toPath(configName) task := o.getTask(path) @@ -195,11 +225,7 @@ func (o *Orchestrator) setupBuildTask( } circularityStack = circularityStack[:len(circularityStack)-1] completed.Add(path) - task.reportDone = make(chan struct{}) - prev := core.LastOrNil(o.order) - if prev != "" { - task.prevReporter = o.getTask(o.toPath(prev)) - } + task.built = make(chan struct{}) task.done = make(chan struct{}) o.order = append(o.order, configName) } @@ -231,6 +257,7 @@ func (o *Orchestrator) GenerateGraph(oldTasks *collections.SyncMap[tspath.Path, for _, project := range projects { o.setupBuildTask(project, nil, false, &completed, &analyzing, circularityStack) } + o.scheduleOrder = o.computeScheduleOrder() if oldTasks != nil { oldTasks.Range(func(path tspath.Path, oldTask *BuildTask) bool { if task, ok := o.tasks.Load(path); ok && task == oldTask { @@ -431,7 +458,7 @@ func (o *Orchestrator) checkTasksForEventChanges(changedPaths map[string]fswatch } } - task.reportDone = make(chan struct{}) + task.built = make(chan struct{}) task.done = make(chan struct{}) newConfig := task.resolved.ReloadFileNamesOfParsedCommandLine(o.host.FS()) @@ -450,7 +477,7 @@ func (o *Orchestrator) checkTasksForEventChanges(changedPaths map[string]fswatch if o.wm.IsPathUnderWatch(eventPath, opts) { o.rangeTask(func(path tspath.Path, task *BuildTask) { task.resetStatus() - task.reportDone = make(chan struct{}) + task.built = make(chan struct{}) task.done = make(chan struct{}) }) needsUpdate.Store(true) @@ -625,7 +652,7 @@ func (o *Orchestrator) DoCycle() { // Overflow: reset all tasks to force a full rebuild. o.rangeTask(func(path tspath.Path, task *BuildTask) { task.resetConfig(o, path) - task.reportDone = make(chan struct{}) + task.built = make(chan struct{}) task.done = make(chan struct{}) }) needsConfigUpdate.Store(true) @@ -669,9 +696,21 @@ func (o *Orchestrator) buildOrClean() tsc.CommandLineResult { var buildResult orchestratorResult if len(o.errors) == 0 { buildResult.statistics.Projects = len(o.Order()) + // Builders run in scheduleOrder; results are reported in build order as they become available + reported := make(chan struct{}) + go func() { + defer close(reported) + for _, config := range o.order { + path := o.toPath(config) + task := o.getTask(path) + <-task.built + task.report(o, path, &buildResult) + } + }() o.rangeTask(func(path tspath.Path, task *BuildTask) { - o.buildOrCleanProject(task, path, &buildResult) + o.buildOrCleanProject(task, path) }) + <-reported } else { // Circularity errors prevent any project from being built buildResult.result.Status = tsc.ExitStatusProjectReferenceCycle_OutputsSkipped @@ -696,10 +735,10 @@ func (o *Orchestrator) rangeTask(f func(path tspath.Path, task *BuildTask)) { var currentTaskIndex atomic.Int64 getNextTask := func() (tspath.Path, *BuildTask, bool) { index := int(currentTaskIndex.Add(1) - 1) - if index >= len(o.order) { + if index >= len(o.scheduleOrder) { return "", nil, false } - config := o.order[index] + config := o.scheduleOrder[index] path := o.toPath(config) task := o.getTask(path) return path, task, true @@ -721,7 +760,7 @@ func (o *Orchestrator) rangeTask(f func(path tspath.Path, task *BuildTask)) { } } -func (o *Orchestrator) buildOrCleanProject(task *BuildTask, path tspath.Path, buildResult *orchestratorResult) { +func (o *Orchestrator) buildOrCleanProject(task *BuildTask, path tspath.Path) { task.result = &taskResult{} task.result.reportStatus = o.createBuilderStatusReporter(task) task.result.diagnosticReporter = o.createDiagnosticReporter(task) @@ -730,7 +769,12 @@ func (o *Orchestrator) buildOrCleanProject(task *BuildTask, path tspath.Path, bu } else { task.cleanProject(o, path) } - task.report(o, path, buildResult) + if o.opts.Testing == nil { + // The program is only needed by Testing.OnProgram at report time; drop it now so a task + // that has finished but is not yet reported does not keep its program alive. + task.result.program = nil + } + close(task.built) } func (o *Orchestrator) getWriter(task *BuildTask) io.Writer {