internal/lsp: don't queue content changes
This updates overlays immeditely, and uses the handle identity change to
correctly update the content on demand.
Fixes golang/go#32348
Change-Id: I3125a6350cac358b7c0f7dc11f2bd11ae1f41031
Reviewed-on: https://go-review.googlesource.com/c/tools/+/179922
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go
index 424a322..e37ddcf 100644
--- a/internal/lsp/cache/file.go
+++ b/internal/lsp/cache/file.go
@@ -29,6 +29,7 @@
fname string
view *view
+ fh source.FileHandle
fc *source.FileContent
token *token.File
}
@@ -70,20 +71,12 @@
f.fc = &source.FileContent{Error: err}
return
}
- if f.fc != nil {
- if len(f.view.contentChanges) == 0 {
- return
- }
-
- f.view.mcache.mu.Lock()
- err := f.view.applyContentChanges(ctx)
- f.view.mcache.mu.Unlock()
-
- if err != nil {
- f.fc = &source.FileContent{Error: err}
- return
- }
+ oldFH := f.fh
+ f.fh = f.view.Session().GetFile(f.URI())
+ // do we already have the right contents?
+ if f.fc != nil && f.fh.Identity() == oldFH.Identity() {
+ return
}
- // We don't know the content yet, so read it.
- f.fc = f.view.Session().GetFile(f.URI()).Read(ctx)
+ // update the contents
+ f.fc = f.fh.Read(ctx)
}
diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go
index ac6d5c4..350c84e 100644
--- a/internal/lsp/cache/gofile.go
+++ b/internal/lsp/cache/gofile.go
@@ -85,7 +85,7 @@
// isDirty is true if the file needs to be type-checked.
// It assumes that the file's view's mutex is held by the caller.
func (f *goFile) isDirty() bool {
- return f.meta == nil || f.imports == nil || f.token == nil || f.ast == nil || f.pkg == nil || len(f.view.contentChanges) > 0
+ return f.meta == nil || f.imports == nil || f.token == nil || f.ast == nil || f.pkg == nil
}
func (f *goFile) astIsTrimmed() bool {
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 772c350..8fdd6f0 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -13,11 +13,6 @@
v.mcache.mu.Lock()
defer v.mcache.mu.Unlock()
- // Apply any queued-up content changes.
- if err := v.applyContentChanges(ctx); err != nil {
- return nil, err
- }
-
// If the package for the file has not been invalidated by the application
// of the pending changes, there is no need to continue.
if !f.isDirty() {
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 1d71e0e..6bf962d 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -65,17 +65,16 @@
ctx := context.Background()
backgroundCtx, cancel := context.WithCancel(ctx)
v := &view{
- session: s,
- id: strconv.FormatInt(index, 10),
- baseCtx: ctx,
- backgroundCtx: backgroundCtx,
- cancel: cancel,
- name: name,
- env: os.Environ(),
- folder: folder,
- filesByURI: make(map[span.URI]viewFile),
- filesByBase: make(map[string][]viewFile),
- contentChanges: make(map[span.URI]func()),
+ session: s,
+ id: strconv.FormatInt(index, 10),
+ baseCtx: ctx,
+ backgroundCtx: backgroundCtx,
+ cancel: cancel,
+ name: name,
+ env: os.Environ(),
+ folder: folder,
+ filesByURI: make(map[span.URI]viewFile),
+ filesByBase: make(map[string][]viewFile),
mcache: &metadataCache{
packages: make(map[string]*metadata),
},
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index e18db6b..a2e7a81 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -56,13 +56,6 @@
filesByURI map[span.URI]viewFile
filesByBase map[string][]viewFile
- // contentChanges saves the content changes for a given state of the view.
- // When type information is requested by the view, all of the dirty changes
- // are applied, potentially invalidating some data in the caches. The
- // closures in the dirty slice assume that their caller is holding the
- // view's mutex.
- contentChanges map[span.URI]func()
-
// mcache caches metadata for the packages of the opened files in a view.
mcache *metadataCache
@@ -224,33 +217,14 @@
v.cancel()
v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx)
- v.contentChanges[uri] = func() {
- v.session.SetOverlay(uri, content)
- }
-
- return nil
-}
-
-// applyContentChanges applies all of the changed content stored in the view.
-// It is assumed that the caller has locked both the view's and the mcache's
-// mutexes.
-func (v *view) applyContentChanges(ctx context.Context) error {
- if ctx.Err() != nil {
- return ctx.Err()
- }
-
- v.pcache.mu.Lock()
- defer v.pcache.mu.Unlock()
-
- for uri, change := range v.contentChanges {
- change()
- delete(v.contentChanges, uri)
- }
+ v.session.SetOverlay(uri, content)
return nil
}
func (f *goFile) invalidate() {
+ f.view.pcache.mu.Lock()
+ defer f.view.pcache.mu.Unlock()
// TODO(rstambler): Should we recompute these here?
f.ast = nil
f.token = nil
