perf: compile-check reads the names of the plan only when a run compiles or needs them

cli/dispatcher/internal/compilecheck/compiler.go

@@ -113,6 +113,17 @@ type Compiler struct {
 	// compares the two. Why it can be replaced: the tests have to see which units the run compares at
 	// all, and a byte comparison leaves nothing behind to count.
 	compareSurface func(projectRoot string, plan BuildPlan, unit CompileUnit) SurfaceVerdict
+	// beforeCompilerWork is called right before a unit compiles in full or the worker checks its
+	// changed files; nil calls nothing. Why: a run that does either reads what it may need later while
+	// that work runs, and one that only replays never does.
+	beforeCompilerWork func()
+}
+
+// startCompilerWork calls the hook set for the moment a unit's compile or check begins, if any.
+func (c Compiler) startCompilerWork() {
+	if c.beforeCompilerWork != nil {
+		c.beforeCompilerWork()
+	}
 }
 
 // surfaceVerdict says whether compiling this unit left its reference assembly showing its dependents

cli/dispatcher/internal/compilecheck/fast_path.go

@@ -140,6 +140,7 @@ func (runner *fastPathRunner) check(
 		return fastPathAttempt{Used: true, Result: fastCheckedUnitResult(unit, recorded, time.Since(startedAt), true)}
 	}
 
+	c.startCompilerWork()
 	responseFilePath, err := writeRewrittenResponseFile(
 		c.ProjectRoot, plan, unit, unit.Assembly.AssemblyName+fastCheckResponseFileSuffix, nil)
 	if err != nil {

cli/dispatcher/internal/compilecheck/name_index.go

@@ -9,6 +9,8 @@ import (
 	"path/filepath"
 	"slices"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"time"
 	"unicode"
 	"unicode/utf8"
@@ -34,12 +36,16 @@ type NameSkip struct {
 
 // NameIndexReport says what reading the identifiers of the units with references in the plan took.
 type NameIndexReport struct {
+	// Duration is the time spent reading the generator output and, when they were read, the sources.
 	Duration time.Duration
 	// Units counts the units whose identifiers were read, WithGenerated those whose saved generator
 	// output was read too, and Unreadable those that could not be read and so compile on any change.
 	Units         int
 	WithGenerated int
 	Unreadable    int
+	// SourcesRead says whether the run needed the sources, by compiling a unit, by having the worker
+	// check its changed files, or by asking a verdict for names.
+	SourcesRead bool
 }
 
 // identifierSet is the identifiers one unit writes.
@@ -162,78 +168,112 @@ func readGeneratedIdentifiers(generatedDirectory string) (identifierSet, bool, e
 	return identifiers, true, nil
 }
 
-// nameIndex reads, in the background, the identifiers of every unit that references another unit of
-// the plan, so a reference that changed only names a unit never writes need not make it compile.
+// nameIndex reads the identifiers of every unit that references another unit of the plan, so a
+// reference that changed only names a unit never writes need not make it compile.
 // Why every such unit and not only the dependents: a unit selected for its own sources compiles
 // anyway, but one that started on the assumption that a reference keeps its surface need not compile
 // again when that reference changed only names it never writes.
 type nameIndex struct {
+	projectRoot   string
+	units         []CompileUnit
 	sets          []identifierSet
 	errs          []error
 	withGenerated []bool
 	indexed       []bool
 	done          chan struct{}
-	duration      time.Duration
-	started       bool
+	// generatedDuration and sourcesDuration are how long each read took, measured from its own start
+	// so the time the run spent before asking for the sources is not counted.
+	generatedDuration time.Duration
+	sourcesDuration   time.Duration
+	started           bool
+	sourcesOnce       sync.Once
+	sourcesRead       atomic.Bool
 }
 
-// start reads the identifiers of the units marked in wanted, at most once per run. generated holds
-// the generator output directory of each unit. The generator output is read before start returns and
-// the sources in the background.
-// Why the generator output is read first: a unit's compile removes and rewrites that directory, and a
-// read racing it would lose the names only the generator wrote. Sources are never removed, and few
-// units have saved generator output, so only the sources are worth reading while compiles run.
+// unitIdentifiers is what was read of one unit's identifiers.
+type unitIdentifiers struct {
+	set           identifierSet
+	withGenerated bool
+	err           error
+}
+
+// start reads the generator output of the units marked in wanted, at most once per run, and leaves
+// their sources for readSources. generated holds the generator output directory of each unit.
+// Why the generator output is read here, before anything compiles: a unit's compile removes and
+// rewrites that directory, and a read racing it would lose the names only the generator wrote.
+// Sources are never removed, so they can wait until a run needs them.
 func (index *nameIndex) start(projectRoot string, units []CompileUnit, generated []string, wanted []bool) {
 	if index.started {
 		return
 	}
 	index.started = true
+	index.projectRoot = projectRoot
+	index.units = units
 	index.done = make(chan struct{})
 	index.indexed = wanted
 	startedAt := time.Now()
-	type unitNames struct {
-		set           identifierSet
-		withGenerated bool
-		err           error
-	}
-	generatedNames := computeInParallel(len(units), func(position int) unitNames {
+	generatedNames := computeInParallel(len(units), func(position int) unitIdentifiers {
 		if !wanted[position] {
-			return unitNames{}
+			return unitIdentifiers{}
 		}
 		set, withGenerated, err := readGeneratedIdentifiers(generated[position])
 
-		return unitNames{set: set, withGenerated: withGenerated, err: err}
+		return unitIdentifiers{set: set, withGenerated: withGenerated, err: err}
 	})
-	go func() {
-		defer close(index.done)
-		results := computeInParallel(len(units), func(position int) unitNames {
-			names := generatedNames[position]
-			if !wanted[position] || names.err != nil {
+	index.keep(generatedNames)
+	index.generatedDuration = time.Since(startedAt)
+}
+
+// readSources starts reading, in the background, the sources of the units start read the generator
+// output of, at most once per run; nothing when start never ran.
+// Why the read waits for this call: a run that only replays and asks no verdict for names would
+// otherwise spend most of its time reading names it never uses. A run that compiles or checks reads
+// them while that runs, and a verdict that needs them waits for them.
+// Safe to call from any goroutine.
+func (index *nameIndex) readSources() {
+	if !index.started {
+		return
+	}
+	index.sourcesOnce.Do(func() {
+		index.sourcesRead.Store(true)
+		go func() {
+			defer close(index.done)
+			startedAt := time.Now()
+			defer func() { index.sourcesDuration = time.Since(startedAt) }()
+			index.keep(computeInParallel(len(index.units), func(position int) unitIdentifiers {
+				names := unitIdentifiers{set: index.sets[position], withGenerated: index.withGenerated[position], err: index.errs[position]}
+				if !index.indexed[position] || names.err != nil {
+					return names
+				}
+				if err := readSourceIdentifiers(index.projectRoot, index.units[position], names.set); err != nil {
+					return unitIdentifiers{err: err}
+				}
+
 				return names
-			}
-			if err := readSourceIdentifiers(projectRoot, units[position], names.set); err != nil {
-				return unitNames{err: err}
-			}
+			}))
+		}()
+	})
+}
 
-			return names
-		})
-		index.sets = make([]identifierSet, len(units))
-		index.errs = make([]error, len(units))
-		index.withGenerated = make([]bool, len(units))
-		for position, result := range results {
-			index.sets[position] = result.set
-			index.errs[position] = result.err
-			index.withGenerated[position] = result.withGenerated
-		}
-		index.duration = time.Since(startedAt)
-	}()
+// keep holds what was read so far.
+func (index *nameIndex) keep(results []unitIdentifiers) {
+	index.sets = make([]identifierSet, len(results))
+	index.errs = make([]error, len(results))
+	index.withGenerated = make([]bool, len(results))
+	for position, result := range results {
+		index.sets[position] = result.set
+		index.errs[position] = result.err
+		index.withGenerated[position] = result.withGenerated
+	}
 }
 
-// lookup waits for the index and returns the identifiers of one unit, or an error when it has none.
+// lookup reads the sources when nothing has yet, waits for them and returns the identifiers of one
+// unit, or an error when it has none.
 func (index *nameIndex) lookup(position int) (identifierSet, error) {
 	if !index.started {
 		return nil, errors.New("the names of the plan were never read")
 	}
+	index.readSources()
 	<-index.done
 	if !index.indexed[position] {
 		return nil, errors.New("the names of this unit were not read")
@@ -245,20 +285,27 @@ func (index *nameIndex) lookup(position int) (identifierSet, error) {
 	return index.sets[position], nil
 }
 
-// wait returns once the background read is over, so nothing outlives the run that started it.
+// wait returns once the background read of the sources is over, so nothing outlives the run that
+// started it; at once when the sources were never asked for.
 func (index *nameIndex) wait() {
-	if index.started {
+	if index.started && index.sourcesRead.Load() {
 		<-index.done
 	}
 }
 
-// report says what the read took; zero when nothing was read.
+// report says what the read took; zero when nothing was read. It waits for the sources when they are
+// being read.
+// Why it waits itself rather than rely on wait: the run builds its outcome, and with it this report,
+// before its deferred wait runs.
 func (index *nameIndex) report() NameIndexReport {
 	if !index.started {
 		return NameIndexReport{}
 	}
-	<-index.done
-	report := NameIndexReport{Duration: index.duration}
+	sourcesRead := index.sourcesRead.Load()
+	if sourcesRead {
+		<-index.done
+	}
+	report := NameIndexReport{Duration: index.generatedDuration + index.sourcesDuration, SourcesRead: sourcesRead}
 	for position, indexed := range index.indexed {
 		if !indexed {
 			continue
@@ -274,8 +321,9 @@ func (index *nameIndex) report() NameIndexReport {
 	return report
 }
 
-// startNameIndex starts reading the identifiers of the units that reference another unit of the plan,
-// when there is any.
+// startNameIndex reads the generator output of the units that reference another unit of the plan,
+// when there is any, and has the run read their sources once it first compiles a unit in full or has
+// the worker check its changed files.
 func (run *schedulerRun) startNameIndex() {
 	wanted := make([]bool, len(run.plan.Units))
 	any := false
@@ -292,6 +340,9 @@ func (run *schedulerRun) startNameIndex() {
 		generated[position] = filepath.Join(run.compiler.ProjectRoot, records.GeneratedDirectory(unit.Assembly.AssemblyName))
 	}
 	run.names.start(run.compiler.ProjectRoot, run.plan.Units, generated, wanted)
+	// Why a compile or a check of changed files starts the read of the sources: either runs long
+	// enough to hide the read, while a run that only replays would wait for it.
+	run.compiler.beforeCompilerWork = run.names.readSources
 }
 
 // referenceEffect is what one reference's final state means for one unit that reads it: whether it

cli/dispatcher/internal/compilecheck/name_index_fifo_test.go

@@ -10,9 +10,9 @@ import (
 	"time"
 )
 
-// Verifies the generator output is read before start returns: with the background read held on a
-// source until the generated directory is gone, as a compile that started meanwhile removes it, the
-// names only the generator wrote are still in the index.
+// Verifies the generator output is read before start returns: with the sources read only after the
+// generated directory is gone, as a compile that started meanwhile removes it, and that read held on a
+// source until the test releases it, the names only the generator wrote are still in the index.
 func TestNameIndexReadsGeneratedCodeBeforeStartReturns(t *testing.T) {
 	root := t.TempDir()
 	writeUnitSources(t, root, map[string]string{"Generated/Gen.g.cs": "partial class First { Foo f; }"})
@@ -32,6 +32,7 @@ func TestNameIndexReadsGeneratedCodeBeforeStartReturns(t *testing.T) {
 	if err := os.RemoveAll(filepath.Join(root, "Generated")); err != nil {
 		t.Fatalf("failed to remove the generated code: %v", err)
 	}
+	index.readSources()
 	releaseFIFO(t, source, "class First { }")
 
 	set, err := index.lookup(0)
@@ -41,6 +42,38 @@ func TestNameIndexReadsGeneratedCodeBeforeStartReturns(t *testing.T) {
 	assertHasIdentifiers(t, set, []string{"First", "Foo"}, nil)
 }
 
+// Verifies a run that never asks for the sources does not read them: with a source no writer ever
+// opens, wait and report still return at once after start.
+func TestNameIndexWaitReturnsAtOnceWhenTheSourcesWereNeverAsked(t *testing.T) {
+	root := t.TempDir()
+	source := filepath.Join(root, "Assets", "A.cs")
+	if err := os.MkdirAll(filepath.Dir(source), 0o755); err != nil {
+		t.Fatalf("failed to create the source directory: %v", err)
+	}
+	// Why a FIFO no one writes: opening it blocks forever, so any read of the sources holds wait.
+	if err := syscall.Mkfifo(source, 0o644); err != nil {
+		t.Fatalf("failed to create the source FIFO: %v", err)
+	}
+	unit := CompileUnit{Assembly: ResponseFile{AssemblyName: "Unit"}, Sources: []string{"Assets/A.cs"}}
+	index := nameIndex{}
+	index.start(root, []CompileUnit{unit}, []string{filepath.Join(root, "Generated")}, []bool{true})
+
+	reported := make(chan NameIndexReport, 1)
+	go func() {
+		index.wait()
+		reported <- index.report()
+	}()
+
+	select {
+	case report := <-reported:
+		if report.SourcesRead || report.Units != 1 {
+			t.Errorf("report = %+v, want one unit whose sources were not read", report)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("wait or report held on sources nothing asked for")
+	}
+}
+
 // releaseFIFO writes content to the FIFO at path once its reader opens it, and fails the test when
 // no reader comes.
 func releaseFIFO(t *testing.T, path string, content string) {

cli/dispatcher/internal/compilecheck/name_index_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 )
 
 func identifiersOf(content string) identifierSet {
@@ -155,13 +156,14 @@ func TestReadSourceIdentifiersRefusesAUnitItCannotRead(t *testing.T) {
 	}
 }
 
-// Verifies the background read is over once wait returns, and that lookup answers only for the
-// units it was asked to read.
+// Verifies the background read of the sources is over once wait returns after it was asked for, and
+// that lookup answers only for the units it was asked to read.
 func TestNameIndexIsOverOnceWaitReturns(t *testing.T) {
 	root := t.TempDir()
 	unit := writeUnitSources(t, root, map[string]string{"Assets/A.cs": "class First { }"})
 	index := nameIndex{}
 	index.start(root, []CompileUnit{unit, unit}, []string{filepath.Join(root, "Missing"), ""}, []bool{true, false})
+	index.readSources()
 
 	index.wait()
 
@@ -180,3 +182,23 @@ func TestNameIndexIsOverOnceWaitReturns(t *testing.T) {
 		t.Error("expected no identifiers from an index that never started")
 	}
 }
+
+// Verifies the reported duration counts only the time spent reading, not the time the run spent
+// before it asked for the sources.
+// Why the bounds hold without flaking: the test waits 300 ms between start and readSources, so a
+// duration measured from start is at least 300 ms, while reading one small file takes well under a
+// millisecond; 150 ms leaves room on both sides for a slow machine.
+func TestNameIndexReportCountsOnlyTheTimeSpentReading(t *testing.T) {
+	root := t.TempDir()
+	unit := writeUnitSources(t, root, map[string]string{"Assets/A.cs": "class First { }"})
+	index := nameIndex{}
+	index.start(root, []CompileUnit{unit}, []string{filepath.Join(root, "Missing")}, []bool{true})
+	time.Sleep(300 * time.Millisecond)
+
+	index.readSources()
+	index.wait()
+
+	if report := index.report(); !report.SourcesRead || report.Duration >= 150*time.Millisecond {
+		t.Errorf("report = %+v, want the sources read in well under 150 ms", report)
+	}
+}

cli/dispatcher/internal/compilecheck/result_cache.go

@@ -53,6 +53,7 @@ func (c Compiler) reuseOrCompile(
 	// Why nothing is recorded unless csc ran to completion: a compile that was cancelled or never
 	// started says nothing about the inputs, and CompileUnit has already removed the manifest of the
 	// compile this one replaces, so an interrupted run leaves no record at all rather than a stale one.
+	c.startCompilerWork()
 	result, err := c.CompileUnit(ctx, plan, unit)
 	result.FastPathFallbackReason = fallbackReason
 	if err != nil || key == "" || result.RawOutput != "" {