diff --git a/gopls/internal/lsp/cache/pkg.go b/gopls/internal/lsp/cache/pkg.go index 0b767b4b5b..ddfb9ea7e9 100644 --- a/gopls/internal/lsp/cache/pkg.go +++ b/gopls/internal/lsp/cache/pkg.go @@ -36,6 +36,8 @@ type pkg struct { analyses memoize.Store // maps analyzer.Name to Promise[actionResult] } +func (p *pkg) String() string { return p.ID() } + // A loadScope defines a package loading scope for use with go/packages. type loadScope interface { aScope() diff --git a/gopls/internal/lsp/diagnostics.go b/gopls/internal/lsp/diagnostics.go index ff3e4b2ec2..603acca607 100644 --- a/gopls/internal/lsp/diagnostics.go +++ b/gopls/internal/lsp/diagnostics.go @@ -217,6 +217,10 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn defer done() // Wait for a free diagnostics slot. + // TODO(adonovan): opt: shouldn't it be the analysis implementation's + // job to de-dup and limit resource consumption? In any case this + // this function spends most its time waiting for awaitLoaded, at + // least initially. select { case <-ctx.Done(): return @@ -226,73 +230,62 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn <-s.diagnosticsSema }() - // First, diagnose the go.mod file. - modReports, modErr := mod.Diagnostics(ctx, snapshot) - if ctx.Err() != nil { - log.Trace.Log(ctx, "diagnose cancelled") - return - } - if modErr != nil { - event.Error(ctx, "warning: diagnose go.mod", modErr, tag.Directory.Of(snapshot.View().Folder().Filename()), tag.Snapshot.Of(snapshot.ID())) - } - for id, diags := range modReports { - if id.URI == "" { - event.Error(ctx, "missing URI for module diagnostics", fmt.Errorf("empty URI"), tag.Directory.Of(snapshot.View().Folder().Filename())) - continue + // common code for dispatching diagnostics + store := func(dsource diagnosticSource, operation string, diagsByFileID map[source.VersionedFileIdentity][]*source.Diagnostic, err error) { + if err != nil { + event.Error(ctx, "warning: while "+operation, err, + tag.Directory.Of(snapshot.View().Folder().Filename()), + tag.Snapshot.Of(snapshot.ID())) } - s.storeDiagnostics(snapshot, id.URI, modSource, diags) - } - upgradeModReports, upgradeErr := mod.UpgradeDiagnostics(ctx, snapshot) - if ctx.Err() != nil { - log.Trace.Log(ctx, "diagnose cancelled") - return - } - if upgradeErr != nil { - event.Error(ctx, "warning: diagnose go.mod upgrades", upgradeErr, tag.Directory.Of(snapshot.View().Folder().Filename()), tag.Snapshot.Of(snapshot.ID())) - } - for id, diags := range upgradeModReports { - if id.URI == "" { - event.Error(ctx, "missing URI for module diagnostics", fmt.Errorf("empty URI"), tag.Directory.Of(snapshot.View().Folder().Filename())) - continue + for id, diags := range diagsByFileID { + if id.URI == "" { + event.Error(ctx, "missing URI while "+operation, fmt.Errorf("empty URI"), tag.Directory.Of(snapshot.View().Folder().Filename())) + continue + } + s.storeDiagnostics(snapshot, id.URI, dsource, diags) } - s.storeDiagnostics(snapshot, id.URI, modCheckUpgradesSource, diags) - } - vulnerabilityReports, vulnErr := mod.VulnerabilityDiagnostics(ctx, snapshot) - if ctx.Err() != nil { - log.Trace.Log(ctx, "diagnose cancelled") - return - } - if vulnErr != nil { - event.Error(ctx, "warning: checking vulnerabilities", vulnErr, tag.Directory.Of(snapshot.View().Folder().Filename()), tag.Snapshot.Of(snapshot.ID())) - } - for id, diags := range vulnerabilityReports { - if id.URI == "" { - event.Error(ctx, "missing URI for module diagnostics", fmt.Errorf("empty URI"), tag.Directory.Of(snapshot.View().Folder().Filename())) - continue - } - s.storeDiagnostics(snapshot, id.URI, modVulncheckSource, diags) } - // Diagnose the go.work file, if it exists. + // Diagnose go.mod upgrades. + upgradeReports, upgradeErr := mod.UpgradeDiagnostics(ctx, snapshot) + if ctx.Err() != nil { + log.Trace.Log(ctx, "diagnose cancelled") + return + } + store(modCheckUpgradesSource, "diagnosing go.mod upgrades", upgradeReports, upgradeErr) + + // Diagnose vulnerabilities. + vulnReports, vulnErr := mod.VulnerabilityDiagnostics(ctx, snapshot) + if ctx.Err() != nil { + log.Trace.Log(ctx, "diagnose cancelled") + return + } + store(modVulncheckSource, "diagnosing vulnerabilities", vulnReports, vulnErr) + + // Diagnose go.work file. workReports, workErr := work.Diagnostics(ctx, snapshot) if ctx.Err() != nil { log.Trace.Log(ctx, "diagnose cancelled") return } - if workErr != nil { - event.Error(ctx, "warning: diagnose go.work", workErr, tag.Directory.Of(snapshot.View().Folder().Filename()), tag.Snapshot.Of(snapshot.ID())) - } - for id, diags := range workReports { - if id.URI == "" { - event.Error(ctx, "missing URI for work file diagnostics", fmt.Errorf("empty URI"), tag.Directory.Of(snapshot.View().Folder().Filename())) - continue - } - s.storeDiagnostics(snapshot, id.URI, workSource, diags) - } + store(workSource, "diagnosing go.work file", workReports, workErr) - // Diagnose all of the packages in the workspace. - wsPkgs, err := snapshot.ActivePackages(ctx) - if s.shouldIgnoreError(ctx, snapshot, err) { + // All subsequent steps depend on the completion of + // type-checking of the all active packages in the workspace. + // This step may take many seconds initially. + // (mod.Diagnostics would implicitly wait for this too, + // but the control is clearer if it is explicit here.) + activePkgs, activeErr := snapshot.ActivePackages(ctx) + + // Diagnose go.mod file. + modReports, modErr := mod.Diagnostics(ctx, snapshot) + if ctx.Err() != nil { + log.Trace.Log(ctx, "diagnose cancelled") + return + } + store(modSource, "diagnosing go.mod file", modReports, modErr) + + if s.shouldIgnoreError(ctx, snapshot, activeErr) { return } criticalErr := snapshot.GetCriticalError(ctx) @@ -303,7 +296,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn // error progress reports will be closed. s.showCriticalErrorStatus(ctx, snapshot, criticalErr) - // There may be .tmpl files. + // Diagnose template (.tmpl) files. for _, f := range snapshot.Templates() { diags := template.Diagnose(f) s.storeDiagnostics(snapshot, f.URI(), typeCheckSource, diags) @@ -311,29 +304,31 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn // If there are no workspace packages, there is nothing to diagnose and // there are no orphaned files. - if len(wsPkgs) == 0 { + if len(activePkgs) == 0 { return } + // Run go/analysis diagnosis of packages in parallel. + // TODO(adonovan): opt: it may be more efficient to + // have diagnosePkg take a set of packages. var ( wg sync.WaitGroup seen = map[span.URI]struct{}{} ) - for _, pkg := range wsPkgs { - wg.Add(1) - + for _, pkg := range activePkgs { for _, pgf := range pkg.CompiledGoFiles() { seen[pgf.URI] = struct{}{} } + wg.Add(1) go func(pkg source.Package) { defer wg.Done() - s.diagnosePkg(ctx, snapshot, pkg, forceAnalysis) }(pkg) } wg.Wait() + // Orphaned files. // Confirm that every opened file belongs to a package (if any exist in // the workspace). Otherwise, add a diagnostic to the file. for _, o := range s.session.Overlays() { diff --git a/gopls/internal/lsp/general.go b/gopls/internal/lsp/general.go index 43973b94ed..57348cd564 100644 --- a/gopls/internal/lsp/general.go +++ b/gopls/internal/lsp/general.go @@ -308,18 +308,18 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol originalViews := len(s.session.Views()) viewErrors := make(map[span.URI]error) - var wg sync.WaitGroup + var ndiagnose sync.WaitGroup // number of unfinished diagnose calls if s.session.Options().VerboseWorkDoneProgress { work := s.progress.Start(ctx, DiagnosticWorkTitle(FromInitialWorkspaceLoad), "Calculating diagnostics for initial workspace load...", nil, nil) defer func() { go func() { - wg.Wait() + ndiagnose.Wait() work.End(ctx, "Done.") }() }() } // Only one view gets to have a workspace. - var allFoldersWg sync.WaitGroup + var nsnapshots sync.WaitGroup // number of unfinished snapshot initializations for _, folder := range folders { uri := span.URIFromURI(folder.URI) // Ignore non-file URIs. @@ -338,41 +338,40 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol } // Inv: release() must be called once. - var swg sync.WaitGroup - swg.Add(1) - allFoldersWg.Add(1) - // TODO(adonovan): this looks fishy. Is AwaitInitialized - // supposed to be called once per folder? - go func() { - defer swg.Done() - defer allFoldersWg.Done() - snapshot.AwaitInitialized(ctx) - work.End(ctx, "Finished loading packages.") - }() - // Print each view's environment. - buf := &bytes.Buffer{} - if err := snapshot.WriteEnv(ctx, buf); err != nil { + var buf bytes.Buffer + if err := snapshot.WriteEnv(ctx, &buf); err != nil { viewErrors[uri] = err release() continue } event.Log(ctx, buf.String()) - // Diagnose the newly created view. - wg.Add(1) + // Initialize snapshot asynchronously. + initialized := make(chan struct{}) + nsnapshots.Add(1) + go func() { + snapshot.AwaitInitialized(ctx) + work.End(ctx, "Finished loading packages.") + nsnapshots.Done() + close(initialized) // signal + }() + + // Diagnose the newly created view asynchronously. + ndiagnose.Add(1) go func() { s.diagnoseDetached(snapshot) - swg.Wait() + <-initialized release() - wg.Done() + ndiagnose.Done() }() } + // Wait for snapshots to be initialized so that all files are known. + // (We don't need to wait for diagnosis to finish.) + nsnapshots.Wait() + // Register for file watching notifications, if they are supported. - // Wait for all snapshots to be initialized first, since all files might - // not yet be known to the snapshots. - allFoldersWg.Wait() if err := s.updateWatchedDirectories(ctx); err != nil { event.Error(ctx, "failed to register for file watching notifications", err) } diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go index 829a03f911..6e0067c5ef 100644 --- a/gopls/internal/lsp/mod/diagnostics.go +++ b/gopls/internal/lsp/mod/diagnostics.go @@ -25,6 +25,8 @@ import ( ) // Diagnostics returns diagnostics for the modules in the workspace. +// +// It waits for completion of type-checking of all active packages. func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.VersionedFileIdentity][]*source.Diagnostic, error) { ctx, done := event.Start(ctx, "mod.Diagnostics", tag.Snapshot.Of(snapshot.ID())) defer done() @@ -73,8 +75,9 @@ func collectDiagnostics(ctx context.Context, snapshot source.Snapshot, diagFn fu return reports, nil } -// ModDiagnostics returns diagnostics from diagnosing the packages in the workspace and -// from tidying the go.mod file. +// ModDiagnostics waits for completion of type-checking of all active +// packages, then returns diagnostics from diagnosing the packages in +// the workspace and from tidying the go.mod file. func ModDiagnostics(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) (diagnostics []*source.Diagnostic, err error) { pm, err := snapshot.ParseMod(ctx, fh) if err != nil {