internal/lsp: move initialization entirely into the snapshot
The majority of the initialization logic is already based on the
snapshot, not the view, so it makes sense to move it there. The
initialization status of the snapshot is copied and invalidated in
clone.
Fixes golang/go#41764
Change-Id: I93234b394318964e7af4696e5ebd465088a05728
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266700
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 678e9c4..4afcb79 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -182,9 +182,6 @@
v := &View{
session: s,
- initialized: make(chan struct{}),
- initializationSema: make(chan struct{}, 1),
- initializeOnce: &sync.Once{},
id: strconv.FormatInt(index, 10),
options: options,
baseCtx: baseCtx,
@@ -205,23 +202,26 @@
},
}
v.snapshot = &snapshot{
- id: snapshotID,
- view: v,
- generation: s.cache.store.Generation(generationName(v, 0)),
- packages: make(map[packageKey]*packageHandle),
- ids: make(map[span.URI][]packageID),
- metadata: make(map[packageID]*metadata),
- files: make(map[span.URI]source.VersionedFileHandle),
- goFiles: make(map[parseKey]*parseGoHandle),
- importedBy: make(map[packageID][]packageID),
- actions: make(map[actionKey]*actionHandle),
- workspacePackages: make(map[packageID]packagePath),
- unloadableFiles: make(map[span.URI]struct{}),
- parseModHandles: make(map[span.URI]*parseModHandle),
- modTidyHandles: make(map[span.URI]*modTidyHandle),
- modUpgradeHandles: make(map[span.URI]*modUpgradeHandle),
- modWhyHandles: make(map[span.URI]*modWhyHandle),
- workspace: workspace,
+ id: snapshotID,
+ view: v,
+ initialized: make(chan struct{}),
+ initializationSema: make(chan struct{}, 1),
+ initializeOnce: &sync.Once{},
+ generation: s.cache.store.Generation(generationName(v, 0)),
+ packages: make(map[packageKey]*packageHandle),
+ ids: make(map[span.URI][]packageID),
+ metadata: make(map[packageID]*metadata),
+ files: make(map[span.URI]source.VersionedFileHandle),
+ goFiles: make(map[parseKey]*parseGoHandle),
+ importedBy: make(map[packageID][]packageID),
+ actions: make(map[actionKey]*actionHandle),
+ workspacePackages: make(map[packageID]packagePath),
+ unloadableFiles: make(map[span.URI]struct{}),
+ parseModHandles: make(map[span.URI]*parseModHandle),
+ modTidyHandles: make(map[span.URI]*modTidyHandle),
+ modUpgradeHandles: make(map[span.URI]*modUpgradeHandle),
+ modWhyHandles: make(map[span.URI]*modWhyHandle),
+ workspace: workspace,
}
// Initialize the view without blocking.
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 55884bb..b691ab3 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -46,6 +46,33 @@
// builtin pins the AST and package for builtin.go in memory.
builtin *builtinPackageHandle
+ // The snapshot's initialization state is controlled by the fields below.
+ // These fields are propagated across snapshots to avoid multiple
+ // concurrent initializations. They may be invalidated during cloning.
+ //
+ // initialized is closed when the snapshot has been fully initialized. On
+ // initialization, the snapshot's workspace packages are loaded. All of the
+ // fields below are set as part of initialization. If we failed to load, we
+ // only retry if the go.mod file changes, to avoid too many go/packages
+ // calls.
+ //
+ // When the view is created, its snapshot's initializeOnce is non-nil,
+ // initialized is open. Once initialization completes, initializedErr may
+ // be set and initializeOnce becomes nil. If initializedErr is non-nil,
+ // initialization may be retried (depending on how files are changed). To
+ // indicate that initialization should be retried, initializeOnce will be
+ // set. The next time a caller requests workspace packages, the
+ // initialization will retry.
+ initialized chan struct{}
+
+ // initializationSema is used as a mutex to guard initializeOnce and
+ // initializedErr, which will be updated after each attempt to initialize
+ // the snapshot. We use a channel instead of a mutex to avoid blocking when
+ // a context is canceled.
+ initializationSema chan struct{}
+ initializeOnce *sync.Once
+ initializedErr error
+
// mu guards all of the maps in the snapshot.
mu sync.Mutex
@@ -848,7 +875,7 @@
s.mu.Lock()
defer s.mu.Unlock()
if len(s.metadata) == 0 {
- return s.view.initializedErr
+ return s.initializedErr
}
return nil
}
@@ -857,7 +884,7 @@
select {
case <-ctx.Done():
return
- case <-s.view.initialized:
+ case <-s.initialized:
}
// We typically prefer to run something as intensive as the IWL without
// blocking. I'm not sure if there is a way to do that here.
@@ -993,7 +1020,7 @@
return fmt.Sprintf("v%v/%v", v.id, snapshotID)
}
-func (s *snapshot) clone(ctx context.Context, changes map[span.URI]*fileChange, forceReloadMetadata bool) (*snapshot, reinitializeView) {
+func (s *snapshot) clone(ctx context.Context, changes map[span.URI]*fileChange, forceReloadMetadata bool) *snapshot {
newWorkspace, workspaceChanged := s.workspace.invalidate(ctx, changes)
s.mu.Lock()
@@ -1001,24 +1028,28 @@
newGen := s.view.session.cache.store.Generation(generationName(s.view, s.id+1))
result := &snapshot{
- id: s.id + 1,
- generation: newGen,
- view: s.view,
- builtin: s.builtin,
- ids: make(map[span.URI][]packageID),
- importedBy: make(map[packageID][]packageID),
- metadata: make(map[packageID]*metadata),
- packages: make(map[packageKey]*packageHandle),
- actions: make(map[actionKey]*actionHandle),
- files: make(map[span.URI]source.VersionedFileHandle),
- goFiles: make(map[parseKey]*parseGoHandle),
- workspacePackages: make(map[packageID]packagePath),
- unloadableFiles: make(map[span.URI]struct{}),
- parseModHandles: make(map[span.URI]*parseModHandle),
- modTidyHandles: make(map[span.URI]*modTidyHandle),
- modUpgradeHandles: make(map[span.URI]*modUpgradeHandle),
- modWhyHandles: make(map[span.URI]*modWhyHandle),
- workspace: newWorkspace,
+ id: s.id + 1,
+ generation: newGen,
+ view: s.view,
+ builtin: s.builtin,
+ initialized: s.initialized,
+ initializationSema: s.initializationSema,
+ initializeOnce: s.initializeOnce,
+ initializedErr: s.initializedErr,
+ ids: make(map[span.URI][]packageID),
+ importedBy: make(map[packageID][]packageID),
+ metadata: make(map[packageID]*metadata),
+ packages: make(map[packageKey]*packageHandle),
+ actions: make(map[actionKey]*actionHandle),
+ files: make(map[span.URI]source.VersionedFileHandle),
+ goFiles: make(map[parseKey]*parseGoHandle),
+ workspacePackages: make(map[packageID]packagePath),
+ unloadableFiles: make(map[span.URI]struct{}),
+ parseModHandles: make(map[span.URI]*parseModHandle),
+ modTidyHandles: make(map[span.URI]*modTidyHandle),
+ modUpgradeHandles: make(map[span.URI]*modUpgradeHandle),
+ modWhyHandles: make(map[span.URI]*modWhyHandle),
+ workspace: newWorkspace,
}
if !workspaceChanged && s.workspaceDirHandle != nil {
@@ -1252,9 +1283,22 @@
if s.workspaceMode() != result.workspaceMode() {
result.workspacePackages = map[packageID]packagePath{}
}
- return result, reinitialize
+
+ // The snapshot may need to be reinitialized.
+ if reinitialize != doNotReinit {
+ result.reinitialize(reinitialize == definitelyReinit)
+ }
+ return result
}
+type reinitializeView int
+
+const (
+ doNotReinit = reinitializeView(iota)
+ maybeReinit
+ definitelyReinit
+)
+
// guessPackagesForURI returns all packages related to uri. If we haven't seen this
// URI before, we guess based on files in the same directory. This is of course
// incorrect in build systems where packages are not organized by directory.
@@ -1305,14 +1349,6 @@
return found
}
-type reinitializeView int
-
-const (
- doNotReinit = reinitializeView(iota)
- maybeReinit
- definitelyReinit
-)
-
// fileWasSaved reports whether the FileHandle passed in has been saved. It
// accomplishes this by checking to see if the original and current FileHandles
// are both overlays, and if the current FileHandle is saved while the original
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 4b97ee6..84a5499 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -68,33 +68,12 @@
filesByURI map[span.URI]*fileBase
filesByBase map[string][]*fileBase
- snapshotMu sync.Mutex
- snapshot *snapshot
-
- // initialized is closed when the view has been fully initialized. On
- // initialization, the view's workspace packages are loaded. All of the
- // fields below are set as part of initialization. If we failed to load, we
- // only retry if the go.mod file changes, to avoid too many go/packages
- // calls.
- //
- // When the view is created, initializeOnce is non-nil, initialized is
- // open, and initCancelFirstAttempt can be used to terminate
- // initialization. Once initialization completes, initializedErr may be set
- // and initializeOnce becomes nil. If initializedErr is non-nil,
- // initialization may be retried (depending on how files are changed). To
- // indicate that initialization should be retried, initializeOnce will be
- // set. The next time a caller requests workspace packages, the
- // initialization will retry.
- initialized chan struct{}
+ // initCancelFirstAttempt can be used to terminate the view's first
+ // attempt at initialization.
initCancelFirstAttempt context.CancelFunc
- // initializationSema is used as a mutex to guard initializeOnce and
- // initializedErr, which will be updated after each attempt to initialize
- // the view. We use a channel instead of a mutex to avoid blocking when a
- // context is canceled.
- initializationSema chan struct{}
- initializeOnce *sync.Once
- initializedErr error
+ snapshotMu sync.Mutex
+ snapshot *snapshot
// workspaceInformation tracks various details about this view's
// environment variables, go version, and use of modules.
@@ -486,21 +465,21 @@
select {
case <-ctx.Done():
return
- case s.view.initializationSema <- struct{}{}:
+ case s.initializationSema <- struct{}{}:
}
defer func() {
- <-s.view.initializationSema
+ <-s.initializationSema
}()
- if s.view.initializeOnce == nil {
+ if s.initializeOnce == nil {
return
}
- s.view.initializeOnce.Do(func() {
+ s.initializeOnce.Do(func() {
defer func() {
- s.view.initializeOnce = nil
+ s.initializeOnce = nil
if firstAttempt {
- close(s.view.initialized)
+ close(s.initialized)
}
}()
@@ -546,9 +525,9 @@
if err != nil {
event.Error(ctx, "initial workspace load failed", err)
if modErrors != nil {
- s.view.initializedErr = errors.Errorf("errors loading modules: %v: %w", err, modErrors)
+ s.initializedErr = errors.Errorf("errors loading modules: %v: %w", err, modErrors)
} else {
- s.view.initializedErr = err
+ s.initializedErr = err
}
}
})
@@ -573,15 +552,9 @@
defer v.snapshotMu.Unlock()
oldSnapshot := v.snapshot
-
- var reinitialize reinitializeView
- v.snapshot, reinitialize = oldSnapshot.clone(ctx, changes, forceReloadMetadata)
+ v.snapshot = oldSnapshot.clone(ctx, changes, forceReloadMetadata)
go oldSnapshot.generation.Destroy()
- if reinitialize == maybeReinit || reinitialize == definitelyReinit {
- v.reinitialize(reinitialize == definitelyReinit)
- }
-
return v.snapshot, v.snapshot.generation.Acquire(ctx)
}
@@ -596,17 +569,17 @@
v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx)
}
-func (v *View) reinitialize(force bool) {
- v.initializationSema <- struct{}{}
+func (s *snapshot) reinitialize(force bool) {
+ s.initializationSema <- struct{}{}
defer func() {
- <-v.initializationSema
+ <-s.initializationSema
}()
- if !force && v.initializedErr == nil {
+ if !force && s.initializedErr == nil {
return
}
var once sync.Once
- v.initializeOnce = &once
+ s.initializeOnce = &once
}
func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI, options *source.Options) (*workspaceInformation, error) {
