Remove redundant work from polling watcher (#3677)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: andrewbranch

internal/core/watchoptions.go

@@ -45,7 +45,7 @@ const (
 )
 
 func (w *WatchOptions) WatchInterval() time.Duration {
-	watchInterval := 1000 * time.Millisecond
+	watchInterval := 2000 * time.Millisecond
 	if w != nil && w.Interval != nil {
 		watchInterval = time.Duration(*w.Interval) * time.Millisecond
 	}

internal/execute/watcher.go

@@ -115,6 +115,10 @@ func (w *Watcher) start() {
 		w.configFilePaths = append([]string{w.configFileName}, w.config.ExtendedSourceFiles()...)
 	}
 
+	if w.sys.GetEnvironmentVariable("TS_WATCH_DEBUG") != "" {
+		w.fileWatcher.SetDebugLog(w.sys.Writer())
+	}
+
 	w.reportWatchStatus(ast.NewCompilerDiagnostic(diagnostics.Starting_compilation_in_watch_mode))
 	w.doBuild()
 	w.mu.Unlock()

internal/vfs/vfswatch/vfswatch.go

@@ -3,6 +3,8 @@
 package vfswatch
 
 import (
+	"fmt"
+	"io"
 	"slices"
 	"sync"
 	"time"
@@ -27,6 +29,7 @@ type FileWatcher struct {
 	watchState          map[string]WatchEntry
 	wildcardDirectories map[string]bool
 	mu                  sync.Mutex
+	debugLog            io.Writer // nil = silent; non-nil = write timing lines here
 }
 
 func NewFileWatcher(fs vfs.FS, pollInterval time.Duration, testing bool, callback func()) *FileWatcher {
@@ -38,6 +41,14 @@ func NewFileWatcher(fs vfs.FS, pollInterval time.Duration, testing bool, callbac
 	}
 }
 
+// SetDebugLog enables per-scan timing output written to w.
+// Pass nil to disable. Safe to call at any time.
+func (fw *FileWatcher) SetDebugLog(w io.Writer) {
+	fw.mu.Lock()
+	defer fw.mu.Unlock()
+	fw.debugLog = w
+}
+
 func (fw *FileWatcher) SetPollInterval(d time.Duration) {
 	fw.mu.Lock()
 	defer fw.mu.Unlock()
@@ -70,15 +81,14 @@ func (fw *FileWatcher) WaitForSettled(now func() time.Time) {
 		return
 	}
 	fw.mu.Lock()
-	wildcardDirs := fw.wildcardDirectories
 	pollInterval := fw.pollInterval
 	fw.mu.Unlock()
 	current := fw.currentState()
 	settledAt := now()
 	tick := min(pollInterval, debounceWait)
 	for now().Sub(settledAt) < debounceWait {
 		time.Sleep(tick)
-		if fw.hasChanges(current, wildcardDirs) {
+		if fw.hasChanges(current) {
 			current = fw.currentState()
 			settledAt = now()
 		}
@@ -118,12 +128,7 @@ func snapshotPaths(fs vfs.FS, paths []string, wildcardDirs map[string]bool) map[
 	state := make(map[string]WatchEntry, len(paths))
 	for _, fn := range paths {
 		if s := fs.Stat(fn); s != nil {
-			entry := WatchEntry{ModTime: s.ModTime(), Exists: true}
-			if s.IsDir() {
-				entries := fs.GetAccessibleEntries(fn)
-				entry.ChildrenHash = hashEntries(entries)
-			}
-			state[fn] = entry
+			state[fn] = WatchEntry{ModTime: s.ModTime(), Exists: true}
 		} else {
 			state[fn] = WatchEntry{Exists: false}
 		}
@@ -176,21 +181,26 @@ func hashEntries(entries vfs.Entries) uint64 {
 	return h.Sum64()
 }
 
-func dirChanged(fs vfs.FS, baseline map[string]WatchEntry, dir string) bool {
-	entry, ok := baseline[dir]
-	if !ok {
-		return true
-	}
-	if entry.ChildrenHash != 0 {
-		entries := fs.GetAccessibleEntries(dir)
-		if hashEntries(entries) != entry.ChildrenHash {
-			return true
-		}
-	}
-	return false
-}
-
-func (fw *FileWatcher) hasChanges(baseline map[string]WatchEntry, wildcardDirs map[string]bool) bool {
+// hasChanges compares the current filesystem state against baseline.
+//
+// Tracked entries fall into two categories:
+//
+//   - Explicit paths (files the compiler depends on, plus directory paths
+//     accessed via DirectoryExists/Stat/etc. during compilation). For these
+//     we only need to know whether the path exists and, if it does, whether
+//     its mtime has changed. We never depend on *what's inside* a directory
+//     in this category — any specific file we care about is tracked
+//     independently in this same map.
+//
+//   - Wildcard tree directories. snapshotPaths walks every directory under
+//     each recursive wildcard root and stores it with a ChildrenHash that
+//     covers the directory's listing. Re-hashing here detects any new,
+//     deleted, or renamed file or subdirectory in those trees.
+//
+// Iterating baseline once therefore covers both: a single fs.Stat per entry,
+// plus a fs.GetAccessibleEntries only for entries with ChildrenHash != 0
+// (i.e. wildcard tree members).
+func (fw *FileWatcher) hasChanges(baseline map[string]WatchEntry) bool {
 	for path, old := range baseline {
 		s := fw.fs.Stat(path)
 		if !old.Exists {
@@ -209,52 +219,47 @@ func (fw *FileWatcher) hasChanges(baseline map[string]WatchEntry, wildcardDirs m
 			}
 		}
 	}
-	for dir, recursive := range wildcardDirs {
-		if !recursive {
-			if dirChanged(fw.fs, baseline, dir) {
-				return true
-			}
-			continue
-		}
-		found := false
-		_ = fw.fs.WalkDir(dir, func(path string, d vfs.DirEntry, err error) error {
-			if err != nil || !d.IsDir() {
-				return nil
-			}
-			if dirChanged(fw.fs, baseline, path) {
-				found = true
-				return vfs.SkipAll
-			}
-			return nil
-		})
-		if found {
-			return true
-		}
-	}
 	return false
 }
 
 // HasChangesFromWatchState compares the current filesystem against the
-// stored watch state. Safe for concurrent use: watchState and
-// wildcardDirectories are snapshotted under lock; the maps themselves
-// are never mutated after creation (UpdateWatchState replaces them).
+// stored watch state. Safe for concurrent use: watchState is snapshotted
+// under lock; the map itself is never mutated after creation
+// (UpdateWatchState replaces it).
 func (fw *FileWatcher) HasChangesFromWatchState() bool {
 	fw.mu.Lock()
 	ws := fw.watchState
-	wildcardDirs := fw.wildcardDirectories
 	fw.mu.Unlock()
-	return fw.hasChanges(ws, wildcardDirs)
+	return fw.hasChanges(ws)
 }
 
 func (fw *FileWatcher) Run(now func() time.Time) {
 	for {
 		fw.mu.Lock()
 		interval := fw.pollInterval
 		ws := fw.watchState
-		wildcardDirs := fw.wildcardDirectories
+		log := fw.debugLog
 		fw.mu.Unlock()
 		time.Sleep(interval)
-		if ws == nil || fw.hasChanges(ws, wildcardDirs) {
+		start := now()
+		changed := ws == nil || fw.hasChanges(ws)
+		if log != nil {
+			elapsed := now().Sub(start)
+			files, dirs, missing := 0, 0, 0
+			for _, e := range ws {
+				switch {
+				case !e.Exists:
+					missing++
+				case e.ChildrenHash != 0:
+					dirs++
+				default:
+					files++
+				}
+			}
+			fmt.Fprintf(log, "[vfswatch] scan: %d paths (%d files, %d dirs, %d missing), %.1fms, changed=%v\n",
+				len(ws), files, dirs, missing, float64(elapsed.Microseconds())/1000.0, changed)
+		}
+		if changed {
 			fw.WaitForSettled(now)
 			fw.callback()
 		}

internal/vfs/vfswatch/vfswatch_test.go

@@ -0,0 +1,64 @@
+package vfswatch_test
+
+import (
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/microsoft/typescript-go/internal/vfs"
+	"github.com/microsoft/typescript-go/internal/vfs/vfstest"
+	"github.com/microsoft/typescript-go/internal/vfs/vfswatch"
+)
+
+// countingFS wraps a vfs.FS and counts calls to GetAccessibleEntries.
+type countingFS struct {
+	vfs.FS
+	n atomic.Int64
+}
+
+func (c *countingFS) GetAccessibleEntries(path string) vfs.Entries {
+	c.n.Add(1)
+	return c.FS.GetAccessibleEntries(path)
+}
+
+// TestHasChangesNoRedundantGetAccessibleEntries verifies that
+// HasChangesFromWatchState calls GetAccessibleEntries only for directories
+// that are part of a recursive wildcard tree, and exactly once each — never
+// for directories that merely happened to be in the explicit paths list.
+//
+// Setup: /src is a recursive wildcard root containing /src and /src/sub.
+// The explicit paths list also contains /node_modules (a directory accessed
+// during compilation but with no wildcard scope) and the tsconfig file.
+// A single HasChangesFromWatchState call should call GetAccessibleEntries
+// exactly twice: once for /src and once for /src/sub. /node_modules is a
+// directory but is *not* a wildcard tree member, so it gets only a Stat —
+// the watcher does not depend on its listing.
+func TestHasChangesNoRedundantGetAccessibleEntries(t *testing.T) {
+	t.Parallel()
+
+	inner := vfstest.FromMap(map[string]string{
+		"/src/a.ts":          "const a = 1;",
+		"/src/b.ts":          "const b = 2;",
+		"/src/sub/c.ts":      "const c = 3;",
+		"/node_modules/x.js": "",
+		"/tsconfig.json":     "{}",
+	}, true)
+	cfs := &countingFS{FS: inner}
+
+	fw := vfswatch.NewFileWatcher(cfs, 10*time.Millisecond, true, func() {})
+	fw.UpdateWatchState(
+		[]string{"/src/a.ts", "/src/b.ts", "/src/sub/c.ts", "/node_modules", "/tsconfig.json"},
+		map[string]bool{"/src": true},
+	)
+
+	cfs.n.Store(0) // reset counter after baseline snapshot
+
+	fw.HasChangesFromWatchState()
+
+	// Only /src and /src/sub (the wildcard tree) are tracked with ChildrenHash.
+	// /node_modules is a directory in the explicit paths list but should NOT be
+	// hashed: its listing is not something the watcher depends on.
+	if got := cfs.n.Load(); got != 2 {
+		t.Errorf("GetAccessibleEntries called %d times, want 2 (once per wildcard-tree dir)", got)
+	}
+}