feat: compile-check starts the largest ready assembly first so long compiles do not finish last

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 {

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)
+	}
+}

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

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 {

docs/compile-check.md

@@ -113,6 +113,9 @@ references, scripting defines and analyzers. `compile-check` replays those respo
    apply. `--jobs <n>` 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