Revert "internal/lsp: move initialization entirely into the snapshot"
This reverts commit deb1282f0497b70cd2a6848d21a9442892ee664e.
Reason for revert: Merge conflicts for Rob's CL stack
Change-Id: I166b3bd60ec2b727a79a090049f0764864656d47
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266699
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index dd8930c..0d07070 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -91,6 +91,8 @@
ctx, done := event.Start(ctx, "cache.view.load", tag.Query.Of(query))
defer done()
+ cleanup := func() {}
+
_, inv, cleanup, err := s.goCommandInvocation(ctx, source.ForTypeChecking, &gocommand.Invocation{
WorkingDir: s.view.rootURI.Filename(),
})
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 6e34538..733c937 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -186,6 +186,9 @@
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,
@@ -206,26 +209,23 @@
},
}
v.snapshot = &snapshot{
- 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),
- modules: modules,
+ 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),
+ modules: modules,
}
v.snapshot.workspaceDirectories = v.snapshot.findWorkspaceDirectories(ctx)
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index ecd0219..f26772e 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -46,33 +46,6 @@
// 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
@@ -893,7 +866,7 @@
s.mu.Lock()
defer s.mu.Unlock()
if len(s.metadata) == 0 {
- return s.initializedErr
+ return s.view.initializedErr
}
return nil
}
@@ -902,7 +875,7 @@
select {
case <-ctx.Done():
return
- case <-s.initialized:
+ case <-s.view.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.
@@ -1019,7 +992,7 @@
return fmt.Sprintf("v%v/%v", v.id, snapshotID)
}
-func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.VersionedFileHandle, forceReloadMetadata bool) *snapshot {
+func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.VersionedFileHandle, forceReloadMetadata bool) (*snapshot, reinitializeView) {
s.mu.Lock()
defer s.mu.Unlock()
@@ -1029,10 +1002,6 @@
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),
@@ -1329,17 +1298,20 @@
// Don't bother copying the importedBy graph,
// as it changes each time we update metadata.
+ var reinitialize reinitializeView
+ if modulesChanged {
+ reinitialize = maybeReinit
+ }
+ if shouldReinitializeView {
+ reinitialize = definitelyReinit
+ }
+
// If the snapshot's workspace mode has changed, the packages loaded using
// the previous mode are no longer relevant, so clear them out.
if s.workspaceMode() != result.workspaceMode() {
result.workspacePackages = map[packageID]packagePath{}
}
-
- // The snapshot may need to be reinitialized.
- if modulesChanged || shouldReinitializeView {
- result.reinitialize(shouldReinitializeView)
- }
- return result
+ return result, reinitialize
}
// guessPackagesForURI returns all packages related to uri. If we haven't seen this
@@ -1392,6 +1364,14 @@
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 008acef..42017a8 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -68,13 +68,34 @@
filesByURI map[span.URI]*fileBase
filesByBase map[string][]*fileBase
- // initCancelFirstAttempt can be used to terminate the view's first
- // attempt at initialization.
- initCancelFirstAttempt context.CancelFunc
-
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 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
+
// workspaceInformation tracks various details about this view's
// environment variables, go version, and use of modules.
workspaceInformation
@@ -474,21 +495,21 @@
select {
case <-ctx.Done():
return
- case s.initializationSema <- struct{}{}:
+ case s.view.initializationSema <- struct{}{}:
}
defer func() {
- <-s.initializationSema
+ <-s.view.initializationSema
}()
- if s.initializeOnce == nil {
+ if s.view.initializeOnce == nil {
return
}
- s.initializeOnce.Do(func() {
+ s.view.initializeOnce.Do(func() {
defer func() {
- s.initializeOnce = nil
+ s.view.initializeOnce = nil
if firstAttempt {
- close(s.initialized)
+ close(s.view.initialized)
}
}()
@@ -536,9 +557,9 @@
if err != nil {
event.Error(ctx, "initial workspace load failed", err)
if modErrors != nil {
- s.initializedErr = errors.Errorf("errors loading modules: %v: %w", err, modErrors)
+ s.view.initializedErr = errors.Errorf("errors loading modules: %v: %w", err, modErrors)
} else {
- s.initializedErr = err
+ s.view.initializedErr = err
}
}
})
@@ -563,9 +584,14 @@
defer v.snapshotMu.Unlock()
oldSnapshot := v.snapshot
- v.snapshot = oldSnapshot.clone(ctx, uris, forceReloadMetadata)
+ var reinitialize reinitializeView
+ v.snapshot, reinitialize = oldSnapshot.clone(ctx, uris, forceReloadMetadata)
go oldSnapshot.generation.Destroy()
+ if reinitialize == maybeReinit || reinitialize == definitelyReinit {
+ v.reinitialize(reinitialize == definitelyReinit)
+ }
+
return v.snapshot, v.snapshot.generation.Acquire(ctx)
}
@@ -580,17 +606,17 @@
v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx)
}
-func (s *snapshot) reinitialize(force bool) {
- s.initializationSema <- struct{}{}
+func (v *View) reinitialize(force bool) {
+ v.initializationSema <- struct{}{}
defer func() {
- <-s.initializationSema
+ <-v.initializationSema
}()
- if !force && s.initializedErr == nil {
+ if !force && v.initializedErr == nil {
return
}
var once sync.Once
- s.initializeOnce = &once
+ v.initializeOnce = &once
}
func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI, options *source.Options) (*workspaceInformation, error) {
