diff --git a/cli/dispatcher/internal/compilecheck/scheduler.go b/cli/dispatcher/internal/compilecheck/scheduler.go index 48181ebb7..77c69dce5 100644 --- a/cli/dispatcher/internal/compilecheck/scheduler.go +++ b/cli/dispatcher/internal/compilecheck/scheduler.go @@ -33,7 +33,9 @@ type schedule struct { pendingOwn []int // ownSources marks, per unit, that it was selected for its own sources. ownSources []bool - dependents [][]int + // sourceCounts holds, per unit, how many sources it compiles. + sourceCounts []int + dependents [][]int // references lists, per unit, the units of the plan it references. references [][]int ready []int @@ -353,6 +355,7 @@ func newSchedule(plan BuildPlan) *schedule { pending: make([]int, len(plan.Units)), pendingOwn: make([]int, len(plan.Units)), ownSources: make([]bool, len(plan.Units)), + sourceCounts: make([]int, len(plan.Units)), dependents: make([][]int, len(plan.Units)), references: make([][]int, len(plan.Units)), ready: []int{}, @@ -363,6 +366,7 @@ func newSchedule(plan BuildPlan) *schedule { } for index, unit := range plan.Units { state.ownSources[index] = !unit.SelectedAsDependent + state.sourceCounts[index] = len(unit.Sources) } for index, unit := range plan.Units { for _, reference := range unit.PlanReferences { diff --git a/cli/dispatcher/internal/compilecheck/scheduler_optimistic_test.go b/cli/dispatcher/internal/compilecheck/scheduler_optimistic_test.go index db0725c06..8b22a9fd3 100644 --- a/cli/dispatcher/internal/compilecheck/scheduler_optimistic_test.go +++ b/cli/dispatcher/internal/compilecheck/scheduler_optimistic_test.go @@ -562,3 +562,65 @@ func TestCompileUnitsQueuesAUnitOnceWhenItsLastReferenceFinishesWhileItWaitsForA t.Errorf("D compiled %d times, want 1", run.recorder.runs["D"]) } } + +// withSourceCounts gives each named unit of a plan that many sources. +func withSourceCounts(plan BuildPlan, counts map[string]int) BuildPlan { + for index, unit := range plan.Units { + sources := []string{} + for source := 0; source < counts[unit.Assembly.AssemblyName]; source++ { + sources = append(sources, fmt.Sprintf("Assets/%s/File%d.cs", unit.Assembly.AssemblyName, source)) + } + plan.Units[index].Sources = sources + } + + return plan +} + +// Verifies the unit with the most sources starts first among those ready at the same time, since it +// is likely to take longest, while the results stay in the plan's order. +func TestCompileUnitsStartsTheReadyUnitWithTheMostSourcesFirst(t *testing.T) { + plan := withSourceCounts(newSchedulerPlan(nil, "A", "B", "C"), map[string]int{"A": 1, "B": 3, "C": 2}) + recorder := newSchedulerRecorder(0) + + outcome, err := compileUnits(context.Background(), newSchedulerCompiler(t, recorder), plan, 1) + if err != nil { + t.Fatalf("expected the run to succeed, got error: %v", err) + } + + assertStrings(t, "start order", recorder.startOrder, []string{"B", "C", "A"}) + assertStrings(t, "results", compiledNames(outcome.Units), []string{"A", "B", "C"}) +} + +// Verifies units ready at the same time with as many sources start in the plan's order, not in the +// order they became ready: B is ready from the start and A only once Z finished, yet A goes first. +func TestCompileUnitsBreaksATieInSourceCountByPlanOrder(t *testing.T) { + plan := withSourceCounts(newSchedulerPlan(map[string][]string{"A": {"Z"}}, "A", "B", "Z"), + map[string]int{"A": 1, "B": 1, "Z": 2}) + recorder := newSchedulerRecorder(0) + + if _, err := compileUnits(context.Background(), newSchedulerCompiler(t, recorder), plan, 1); err != nil { + t.Fatalf("expected the run to succeed, got error: %v", err) + } + + assertStrings(t, "start order", recorder.startOrder, []string{"Z", "A", "B"}) +} + +// Verifies a unit whose references are all final starts before a larger unit queued early that +// references it: starting the larger one first would compile it against a reference not yet +// compiled, and again once that reference changed its surface. +func TestCompileUnitsStartsAUnitWhoseReferencesAreFinalBeforeALargerUnitQueuedEarly(t *testing.T) { + run := newOptimisticPlanRun(t, map[string][]string{"A": {"X"}, "B": {"X"}, "D": {"X", "A"}}, + []string{"A", "B", "D"}, "X", "A", "B", "D") + run.plan = withSourceCounts(run.plan, map[string]int{"X": 1, "A": 1, "B": 1, "D": 3}) + run.jobs = 1 + + outcome := run.compile(t) + + if run.recorder.runs["D"] != 1 { + t.Errorf("D compiled %d times, want 1", run.recorder.runs["D"]) + } + assertStrings(t, "start order", run.recorder.startOrder, []string{"X", "A", "D", "B"}) + if again := resultOf(t, outcome.Units, "D").CompiledAgainAfter; again != "" { + t.Errorf("D compiled again after %q, want once", again) + } +} diff --git a/cli/dispatcher/internal/compilecheck/scheduler_settle.go b/cli/dispatcher/internal/compilecheck/scheduler_settle.go index 9bffbb557..3ede0081d 100644 --- a/cli/dispatcher/internal/compilecheck/scheduler_settle.go +++ b/cli/dispatcher/internal/compilecheck/scheduler_settle.go @@ -54,15 +54,42 @@ func (state *schedule) enqueue(index int) { state.ready = append(state.ready, index) } -// takeNext removes the next unit to start from the ready queue. +// takeNext removes the next unit to start from the ready queue: a unit whose references are all final +// before one queued early, then the one with the most sources, and among those the one first in the +// plan. +// Why a unit with final references goes first: a unit queued early may reference one still waiting +// in the queue, and starting it ahead of that reference would compile it against Unity's file, only +// to compile it again once the reference changed its surface. +// Why the most sources first: how long a compile takes follows its source count closely, and +// starting the longest compiles first keeps them from finishing last while the other jobs sit idle. +// Why plan order breaks a tie rather than the order units became ready: it keeps the start order the +// same from run to run. func (state *schedule) takeNext() int { - index := state.ready[0] - state.ready = state.ready[1:] + best := 0 + for position := 1; position < len(state.ready); position++ { + if state.startsBefore(state.ready[position], state.ready[best]) { + best = position + } + } + index := state.ready[best] + state.ready = append(state.ready[:best], state.ready[best+1:]...) state.queued[index] = false return index } +// startsBefore reports whether unit a should start before unit b when both are ready. +func (state *schedule) startsBefore(a int, b int) bool { + if (state.pending[a] == 0) != (state.pending[b] == 0) { + return state.pending[a] == 0 + } + if state.sourceCounts[a] != state.sourceCounts[b] { + return state.sourceCounts[a] > state.sourceCounts[b] + } + + return a < b +} + // mayStartEarly reports whether a unit with references still lacking a final state may start before // them: it has to compile whatever they turn out to be, every reference still missing is a dependent // rather than one selected for its own sources, and nothing has ruled it out. @@ -111,11 +138,16 @@ func (run *schedulerRun) releaseAndOffer(index int, changed bool) { } } -// startAndSettle starts what is ready and decides the final state of what finished, until neither +// startAndSettle decides the final state of what finished and starts what is ready, until neither // moves anything. // Why the two alternate: a skip decided while starting units gives a reference its final state, // which can let a unit that already finished be decided, and deciding one can make others ready. +// Why it decides before it starts: the units a finished one releases then compete for the free jobs +// with the ones already waiting, instead of queuing behind whatever started first. func (run *schedulerRun) startAndSettle(jobs int) error { + if _, err := run.settleFinished(); err != nil { + return err + } for { if err := run.startReadyUnits(jobs); err != nil { return err diff --git a/cli/dispatcher/internal/compilecheck/scheduler_test.go b/cli/dispatcher/internal/compilecheck/scheduler_test.go index c73faf78a..bf31ecaff 100644 --- a/cli/dispatcher/internal/compilecheck/scheduler_test.go +++ b/cli/dispatcher/internal/compilecheck/scheduler_test.go @@ -68,6 +68,8 @@ type schedulerRecorder struct { // surfacesByRun holds, per unit, the reference assembly the n-th invocation of it writes; an // empty one writes none. An invocation past its end falls back to surfaces. surfacesByRun map[string][]string + // startOrder lists the units in the order their invocations started. + startOrder []string // begun names the units whose compile or changed-files check has begun. begun map[string]bool // heldUntilBegun holds, per unit, the units its fake invocation waits to see begun before it @@ -166,6 +168,7 @@ func (recorder *schedulerRecorder) runner() CommandRunner { recorder.started[name] = true recorder.runs[name]++ run := recorder.runs[name] + recorder.startOrder = append(recorder.startOrder, name) recorder.begun[name] = true recorder.running++ if recorder.running > recorder.maxRunning { diff --git a/docs/compile-check.md b/docs/compile-check.md index b215b4cbd..569de3b02 100644 --- a/docs/compile-check.md +++ b/docs/compile-check.md @@ -113,6 +113,9 @@ references, scripting defines and analyzers. `compile-check` replays those respo apply. `--jobs ` caps how many run at once; the default is half the machine's CPUs and never less than one, because one `csc` process already keeps two to three cores busy on its own. `--jobs 1` compiles the assemblies one after another. + Among the assemblies ready at the same time, one whose references have all finished starts + before one started early, then the one with the most sources, and a tie goes to the one first in + dependency order. The reported order does not depend on the job count: results are collected in dependency order whatever finishes first. An assembly that fails to compile stops the run at that point of the tree: it wrote no reference assembly, so everything below it has nothing to compile against and