-
Notifications
You must be signed in to change notification settings - Fork 677
Snapshot LSP #1505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Snapshot LSP #1505
Conversation
There was a problem hiding this comment.
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 implements a comprehensive architectural overhaul that replaces the mutable project system with an immutable snapshot-based architecture for the LSP server. The goal is to eliminate data races and enable safe concurrent access through immutable state snapshots.
Key changes:
- Introduces snapshot-based state management with immutable project collections and config file registries
- Implements ref-counting AST caches for memory management and efficient sharing
- Replaces synchronous mutable operations with snapshot transitions and background task handling
Reviewed Changes
Copilot reviewed 86 out of 102 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
internal/project/service_test.go | Removes original project service tests in favor of new snapshot-based testing approach |
internal/project/service.go | Removes the original mutable Service implementation and related project management logic |
internal/project/scriptinfo.go | Removes ScriptInfo implementation that was part of the mutable architecture |
internal/project/refcounting_test.go | Adds tests for ref-counting cache behavior for parse and extended config caches |
internal/project/projectreferencesprogram_test.go | Updates tests to use new snapshot-based session API instead of direct service calls |
internal/project/projectlifetime_test.go | Updates project lifecycle tests to work with immutable snapshots and session management |
internal/project/projectcollectionbuilder_test.go | Adds comprehensive tests for the new project collection builder functionality |
internal/project/projectcollectionbuilder.go | Implements the core builder logic for creating new project collection snapshots |
internal/project/projectcollection.go | Implements the immutable ProjectCollection data structure |
internal/project/project_stringer_generated.go | Updates generated stringer to remove unused project kinds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran the LSP and typed in the checker with race mode on, and got this race:
==================
WARNING: DATA RACE
Write at 0x00c02e47df00 by goroutine 49382:
github.com/microsoft/typescript-go/internal/project.(*compilerHost).freeze()
/home/jabaile/work/TypeScript-go/internal/project/compilerhost.go:50 +0xd6e
github.com/microsoft/typescript-go/internal/project.(*Snapshot).Clone()
/home/jabaile/work/TypeScript-go/internal/project/snapshot.go:189 +0xd03
github.com/microsoft/typescript-go/internal/project.(*Session).UpdateSnapshot()
/home/jabaile/work/TypeScript-go/internal/project/session.go:425 +0x232
github.com/microsoft/typescript-go/internal/project.(*Session).GetLanguageService()
/home/jabaile/work/TypeScript-go/internal/project/session.go:387 +0x417
github.com/microsoft/typescript-go/internal/lsp.init.func1.registerLanguageServiceDocumentRequestHandler[go.shape.*uint8,go.shape.struct { FullDocumentDiagnosticReport *github.com/microsoft/typescript-go/internal/lsp/lsproto.RelatedFullDocumentDiagnosticReport; UnchangedDocumentDiagnosticReport *github.com/microsoft/typescript-go/internal/lsp/lsproto.RelatedUnchangedDocumentDiagnosticReport }].10()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:506 +0x15b
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleRequestOrNotification()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:421 +0x1f8
github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop.func1()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:326 +0x64
Previous read at 0x00c02e47df00 by goroutine 49385:
github.com/microsoft/typescript-go/internal/project.(*compilerFS).UseCaseSensitiveFileNames()
/home/jabaile/work/TypeScript-go/internal/project/compilerhost.go:155 +0x28
github.com/microsoft/typescript-go/internal/compiler.(*Program).UseCaseSensitiveFileNames()
/home/jabaile/work/TypeScript-go/internal/compiler/program.go:143 +0x43
github.com/microsoft/typescript-go/internal/compiler.(*Program).toPath()
/home/jabaile/work/TypeScript-go/internal/compiler/program.go:1491 +0x5c
github.com/microsoft/typescript-go/internal/compiler.(*Program).GetSourceFile()
/home/jabaile/work/TypeScript-go/internal/compiler/program.go:1495 +0x5e
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).tryGetProgramAndFile()
/home/jabaile/work/TypeScript-go/internal/ls/languageservice.go:27 +0x4f
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).getProgramAndFile()
/home/jabaile/work/TypeScript-go/internal/ls/languageservice.go:33 +0x53
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).ProvideDocumentSymbols()
/home/jabaile/work/TypeScript-go/internal/ls/symbols.go:21 +0x48
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleDocumentSymbol()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:788 +0x65
github.com/microsoft/typescript-go/internal/lsp.init.func1.registerLanguageServiceDocumentRequestHandler[go.shape.*uint8,go.shape.struct { SymbolInformations *[]*github.com/microsoft/typescript-go/internal/lsp/lsproto.SymbolInformation; DocumentSymbols *[]*github.com/microsoft/typescript-go/internal/lsp/lsproto.DocumentSymbol }].21()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:511 +0x22e
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleRequestOrNotification()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:421 +0x1f8
github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop.func1()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:326 +0x64
Goroutine 49382 (running) created at:
github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:346 +0x976
github.com/microsoft/typescript-go/internal/lsp.(*Server).Run.func1()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:222 +0x46
golang.org/x/sync/errgroup.(*Group).Go.func1()
/home/jabaile/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:93 +0x86
Goroutine 49385 (running) created at:
github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:346 +0x976
github.com/microsoft/typescript-go/internal/lsp.(*Server).Run.func1()
/home/jabaile/work/TypeScript-go/internal/lsp/server.go:222 +0x46
golang.org/x/sync/errgroup.(*Group).Go.func1()
/home/jabaile/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:93 +0x86
==================
Note
Diagrams are generated by Copilot with Python; they’re pretty rough but get the idea across. I used them for an internal team presentation and figured they’re still better than nothing for this.
Summary
This PR replaces the project system backing the LSP server with an immutable snapshot-based architecture.
Problem Statement
The original LSP implementation here in typescript-go inherited microsoft/TypeScript's architecture, which relied on mutable data structures suitable for synchronous operation. However, as we started to parallelize request handling, this approach became unsustainable. We were starting to spend a lot of time chasing down data races, for which the solution was often tacking on yet another mutex for an individual component.
Design Goals
The new architecture addresses these issues through immutable snapshots that can be safely read by multiple goroutines and efficiently cloned to create new state. This enables:
Architecture
Core Concepts
The system centers around snapshots - immutable representations of LSP server state at a point in time. Snapshots can be read concurrently without coordination and cloned to produce new snapshots when state changes are needed.
While TypeScript programs were already immutable, additional components needed to be refactored for immutability:
System Structure
The architecture separates concerns between mutable session management and immutable snapshot state:
Session Layer (Mutable)
Snapshot Layer (Immutable)
State Transitions
State changes occur through snapshot cloning:
Request Processing
When an LSP request needs to invoke a language service operation for a document, we first check if there are any pending changes that need to be applied (e.g. file watchers have been triggered, open files have been changed, or an ATA request has completed). If so, the Session's current snapshot is cloned to incorporate the pending changes while simultaneously ensuring that the default project for the requested file is loaded and up-to-date. If there are no pending changes, we check if the Session's current snapshot is capable of serving that request (that is, if a default project can be found for that file and the project is up-to-date). If not, we clone the snapshot, in this case with no file changes, but a request to load or update the default project for the requested file. Finally, we return a language service that uses the project from the latest snapshot.
Notable behavior changes
references
array, and it terminates work as soon as higher priority matches have been ruled out. In order to support this early termination of work, we can no longer keep every project we loaded open, since that set is nondeterministic. Instead, we keep open only the projects that led us to the one that ultimately contained the target file. Example:/workspace/packages/foo/test.ts
/workspace/packages/foo/tsconfig.json
, doesn't contain the file, has two references:/workspace/packages/bar/tsconfig.json
/workspace/packages/baz/tsconfig.json
/workspace/tsconfig.json
, doesn't contain the file, has 1,000 referencestests.tsconfig.json
had a reference offoo.test.tsconfig.json
foo.test.tsconfig.json
contains the file at index 50foo.test.tsconfig.json
is the default project./workspace/foo.test.tsconfig.json
(the default project)/workspace/tests.tsconfig.json
/workspace/tsconfig.json
/workspace/packages/foo/tsconfig.json