Skip to content

Watcher performance improvements#4399

Open
johnfav03 wants to merge 5 commits into
microsoft:mainfrom
johnfav03:watcher-perf-improvements
Open

Watcher performance improvements#4399
johnfav03 wants to merge 5 commits into
microsoft:mainfrom
johnfav03:watcher-perf-improvements

Conversation

@johnfav03

Copy link
Copy Markdown
Contributor

Added single-file program reuse via UpdateProgram and skips other unnecessary operations. Also keeps a watchSetDirty flag to track when a full rebuild is required.

Copilot AI review requested due to automatic review settings June 22, 2026 17:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves watch-mode performance in the internal/execute watcher by reusing an existing single-file Program via UpdateProgram when safe, and introducing a watchSetDirty flag to avoid unnecessary work unless a full watch set rebuild is required.

Changes:

  • Add watchSetDirty state to force full rebuilds only when directory/wildcard inclusion changes are suspected.
  • Introduce a fast path that attempts Program.UpdateProgram reuse when exactly one file changed and config/watch set are unchanged.
  • Reduce unnecessary wildcard file-name reload operations during rebuilds.

Comment thread internal/execute/watcher.go
Comment thread internal/execute/watcher.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread internal/execute/tsctests/watcher_race_test.go
Comment thread internal/execute/tsctests/watcher_race_test.go Outdated
@jakebailey

Copy link
Copy Markdown
Member

Do you have any data about how this affects things?

@johnfav03

Copy link
Copy Markdown
Contributor Author

Do you have any data about how this affects things?

In my testing single-file edits show the biggest performance gain, and the gain scales with project size. I had copilot run a benchmark in vscode, and single-file edits see a 50% decrease in rebuild time on average. Higher-volume edits also see an improvement, but it trends closer to 10%.

@jakebailey

Copy link
Copy Markdown
Member

I think this has two correctness regressions in the watcher fast path. I was able to reduce both to focused tests:

func TestWatcherStartsFromExistingBuildInfo(t *testing.T) {
	input := &tscInput{
		files: FileMap{
			"/home/src/workspaces/project/index.ts":      `export const x: number = 1;`,
			"/home/src/workspaces/project/tsconfig.json": `{"compilerOptions":{"composite":true},"files":["index.ts"]}`,
		},
	}
	sys := newTestSys(input, false)

	result := execute.CommandLine(context.Background(), sys, []string{"-p", "tsconfig.json", "--pretty", "false"}, sys)
	assert.Equal(t, result.Status, tsc.ExitStatusSuccess)
	assert.Assert(t, sys.fsFromFileMap().FileExists("/home/src/workspaces/project/tsconfig.tsbuildinfo"))

	sys.clearOutput()
	defer func() {
		if r := recover(); r != nil {
			t.Fatalf("watch startup with existing build info panicked: %v", r)
		}
	}()
	result = execute.CommandLine(context.Background(), sys, []string{"--watch", "--noEmit", "--pretty", "false"}, sys)
	assert.Equal(t, result.Status, tsc.ExitStatusSuccess)
	assert.Assert(t, result.Watcher != nil)
}

func TestWatcherRebuildsWhenJsxImportSourcePragmaChanges(t *testing.T) {
	input := &tscInput{
		files: FileMap{
			"/home/src/workspaces/project/index.tsx": `/** @jsxImportSource foo */
export const x = <div />;`,
			"/home/src/workspaces/project/tsconfig.json": `{
				"compilerOptions":{"jsx":"react-jsx","module":"esnext","moduleResolution":"bundler","noEmit":true},
				"files":["index.tsx"]
			}`,
		},
		commandLineArgs: []string{"--watch"},
	}
	sys := newTestSys(input, false)
	result := execute.CommandLine(context.Background(), sys, []string{"--watch", "--pretty", "false"}, sys)
	if result.Watcher == nil {
		t.Fatal("expected Watcher to be non-nil in watch mode")
	}
	w := result.Watcher.(*execute.Watcher)

	sys.currentWrite.Reset()
	_ = sys.fsFromFileMap().WriteFile("/home/src/workspaces/project/index.tsx", `/** @jsxImportSource bar */
export const x = <div />;`)
	sys.mockWatchBackend.SendEvents([]fswatch.Event{
		{Kind: fswatch.EventUpdate, Path: "/home/src/workspaces/project/index.tsx"},
	})
	w.DoCycle()

	out := sys.currentWrite.String()
	assert.Assert(t, strings.Contains(out, "bar/jsx-runtime"), "expected updated JSX runtime diagnostic, got: %s", out)
	assert.Assert(t, !strings.Contains(out, "foo/jsx-runtime"), "expected stale JSX runtime diagnostic to be gone, got: %s", out)
}

On this branch:

go test -run 'TestWatcher(StartsFromExistingBuildInfo|RebuildsWhenJsxImportSourcePragmaChanges)$' ./internal/execute/tsctests

fails with:

watch startup with existing build info panicked: GetProgram: should not be called without program

and the JSX test continues reporting foo/jsx-runtime after the file was changed to @jsxImportSource bar.

The first one looks like the new watcher fast path is trying tryUpdateProgram when w.program came from ReadBuildInfoProgram, which can be snapshot-only and has no underlying compiler program. The second looks like UpdateProgram is reusing processedFiles / jsxRuntimeImportSpecifiers, but canReplaceFileInProgram doesn’t compare pragmas, so a pragma-only JSX runtime change can reuse stale module resolution state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants