Skip to content

Commit

Permalink
gopls/internal/lsp/cache: pass workspace information into createView
Browse files Browse the repository at this point in the history
As a first step toward inverting control of View construction, pass in
the workspace information rather than querying it inside createView.

This avoids a redundant call to getWorkspaceInformation (which invokes
the Go command) during invalidation.

For golang/go#57979

Change-Id: I6e5308dd7c57c32f1a03c09f501aac8ffeb369e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/539657
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
findleyr committed Nov 6, 2023
1 parent 2ddaad7 commit d2c415d
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 12 deletions.
20 changes: 9 additions & 11 deletions gopls/internal/lsp/cache/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ func (s *Session) NewView(ctx context.Context, folder *Folder) (*View, source.Sn
return nil, nil, nil, source.ErrViewExists
}
}
view, snapshot, release, err := s.createView(ctx, folder, 0)
info, err := getWorkspaceInformation(ctx, s.gocmdRunner, s, folder)
if err != nil {
return nil, nil, nil, err
}
view, snapshot, release, err := s.createView(ctx, info, folder, 0)
if err != nil {
return nil, nil, nil, err
}
Expand All @@ -97,15 +101,9 @@ func (s *Session) NewView(ctx context.Context, folder *Folder) (*View, source.Sn
// TODO(rfindley): clarify that createView can never be cancelled (with the
// possible exception of server shutdown).
// On success, the caller becomes responsible for calling the release function once.
func (s *Session) createView(ctx context.Context, folder *Folder, seqID uint64) (*View, *snapshot, func(), error) {
func (s *Session) createView(ctx context.Context, info *workspaceInformation, folder *Folder, seqID uint64) (*View, *snapshot, func(), error) {
index := atomic.AddInt64(&viewIndex, 1)

// Get immutable workspace information.
info, err := getWorkspaceInformation(ctx, s.gocmdRunner, s, folder)
if err != nil {
return nil, nil, nil, err
}

gowork, _ := info.GOWORK()
wsModFiles, wsModFilesErr := computeWorkspaceModFiles(ctx, info.gomod, gowork, info.effectiveGO111MODULE(), s)

Expand Down Expand Up @@ -289,7 +287,7 @@ func (s *Session) RemoveView(view *View) {
//
// If the resulting error is non-nil, the view may or may not have already been
// dropped from the session.
func (s *Session) updateViewLocked(ctx context.Context, view *View, folder *Folder) error {
func (s *Session) updateViewLocked(ctx context.Context, view *View, info *workspaceInformation, folder *Folder) error {
// Preserve the snapshot ID if we are recreating the view.
view.snapshotMu.Lock()
if view.snapshot == nil {
Expand All @@ -304,7 +302,7 @@ func (s *Session) updateViewLocked(ctx context.Context, view *View, folder *Fold
return fmt.Errorf("view %q not found", view.id)
}

v, snapshot, release, err := s.createView(ctx, folder, seqID)
v, snapshot, release, err := s.createView(ctx, info, folder, seqID)
if err != nil {
// we have dropped the old view, but could not create the new one
// this should not happen and is very bad, but we still need to clean
Expand Down Expand Up @@ -431,7 +429,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif
// could report a bug, but it's not really a bug.
event.Error(ctx, "fetching workspace information", err)
} else if *info != *view.workspaceInformation {
if err := s.updateViewLocked(ctx, view, view.folder); err != nil {
if err := s.updateViewLocked(ctx, view, info, view.folder); err != nil {
// More catastrophic failure. The view may or may not still exist.
// The best we can do is log and move on.
event.Error(ctx, "recreating view", err)
Expand Down
6 changes: 5 additions & 1 deletion gopls/internal/lsp/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,11 @@ func (s *Session) SetFolderOptions(ctx context.Context, uri span.URI, options *s
if v.folder.Dir == uri {
folder2 := *v.folder
folder2.Options = options
if err := s.updateViewLocked(ctx, v, &folder2); err != nil {
info, err := getWorkspaceInformation(ctx, s.gocmdRunner, s, &folder2)
if err != nil {
return err
}
if err := s.updateViewLocked(ctx, v, info, &folder2); err != nil {
return err
}
}
Expand Down

0 comments on commit d2c415d

Please sign in to comment.