Introduce fast path for program updates in language service #879
Conversation
ahejlsberg
requested review from
Copilot,
andrewbranch,
DanielRosenwasser and
jakebailey
and removed request for
Copilot
There was a problem hiding this comment.
Pull Request Overview
This PR adds a fast-path for program updates by tracking when exactly one file is modified and reusing the existing program to improve responsiveness.
- Track a single dirty file (
dirtyFilePath) and version inProject - Conditionally choose between a full rebuild or a clone-and-replace fast path in
updateProgram - Add
NewProgramWithChangedFile(and helpers) to perform the fast-path clone in the compiler
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/project/project.go | Introduce dirtyFilePath, update locking in markFileAsDirty/markAsDirty, and add fast-path branch in updateProgram |
| internal/compiler/program.go | Refactor checker-pool init, add NewProgramWithChangedFile, initCheckerPool, and comparison helpers for file replacement |
Comments suppressed due to low confidence (1)
internal/project/project.go:471
- Add unit tests covering the fast path in
updateProgram, verifying thatNewProgramWithChangedFileis invoked when only one file is dirty and a full rebuild is skipped.
if p.program == nil || p.dirtyFilePath == "" {
| defer p.dirtyStateMu.Unlock() | ||
| if !p.dirty { | ||
| p.dirty = true | ||
| p.dirtyFilePath = path |
There was a problem hiding this comment.
I think this may need to be a Set or something; file watching can cause this to be overwritten with subsequent updates, or even open files on branch switches or similar.
There was a problem hiding this comment.
Right, but the minute anything else changes, we clear out p.dirtyFilePath and updateGraph will then completely re-create the program.
There was a problem hiding this comment.
Ah, I see what you mean in updateProgram; you're right that it's safe. I guess we don't need to be super optimal (though I think it might just be a couple lines to keep track of multiple dirty files).
There was a problem hiding this comment.
Is this temporary? because we have this https://github.com/microsoft/TypeScript/blob/main/src/compiler/program.ts#L2458 in strada thats much more resilient to multiple file changes (can happen as part of open and close when navigating source code)
There was a problem hiding this comment.
But as a trade-off, typing performance in Strada is extremely poor compared to what it could be. We definitely need new optimizations for the 99 percentile case of typing in a single file. Program construction shouldn’t have to loop over every source file to determine what’s changed when the project infrastructure already knows what changed.
| if p.program == nil || p.dirtyFilePath == "" { | ||
| rootFileNames := p.GetRootFileNames() | ||
| compilerOptions := p.compilerOptions | ||
| p.program = compiler.NewProgram(compiler.ProgramOptions{ |
There was a problem hiding this comment.
If we somehow propagate out whether the old program was cloned or not, we can use that to skip reevaluating the file watchers at the end of updateGraph too.
There was a problem hiding this comment.
Now in my latest commit.
|
Just as an FYI, with this PR, diagnostics requests in |
| } | ||
|
|
||
| func (p *Project) markAsDirty() { | ||
| p.dirtyStateMu.Lock() | ||
| defer p.dirtyStateMu.Unlock() | ||
| if !p.dirty { | ||
| p.dirty = true | ||
| p.dirtyFilePath = "" |
There was a problem hiding this comment.
I think we need to reset p.dirtyFilePath unconditionally here. Suppose we call p.markFileAsDirty() and it sets p.dirtyFilePath to some path, and then right after we call p.markAsDirty(): we won't reset p.dirtyFilePath because p.dirty is already true.
This PR introduces a fast path for program updates in the language service. When only a single file has been modified and none of the changes impact the program's structure, we create a new program by just cloning the existing program and replacing the single file. This dramatically improves responsiveness in the typical path of typing in a file.