Skip to content

Commit d7f4359

Browse files
committed
gopls/internal/lsp/mod: optimizations for mod tidy diagnostics
Run mod tidy diagnostics in parallel, and don't parse files to determine missing imports if there are no missing requires. BenchmarkDiagnoseSave: 8s->1.8s For golang/go#60089 Change-Id: I5d41827914e4eb9264b16ed14af323c017eb327c Reviewed-on: https://go-review.googlesource.com/c/tools/+/496439 TryBot-Result: Gopher Robot <gobot@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
1 parent 2eb726b commit d7f4359

File tree

6 files changed

+66
-29
lines changed

6 files changed

+66
-29
lines changed

gopls/doc/commands.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,9 @@ Args:
267267
"URI": string,
268268
// The module path to remove.
269269
"ModulePath": string,
270+
// If the module is tidied apart from the one unused diagnostic, we can
271+
// run `go get module@none`, and then run `go mod tidy`. Otherwise, we
272+
// must make textual edits.
270273
"OnlyDiagnostic": bool,
271274
}
272275
```

gopls/internal/lsp/cache/mod_tidy.go

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,33 @@ func modTidyDiagnostics(ctx context.Context, snapshot *snapshot, pm *source.Pars
183183
// go.mod file. The fixes will be for the go.mod file, but the
184184
// diagnostics should also appear in both the go.mod file and the import
185185
// statements in the Go files in which the dependencies are used.
186+
// Finally, add errors for any unused dependencies.
187+
if len(missing) > 0 {
188+
missingModuleDiagnostics, err := missingModuleDiagnostics(ctx, snapshot, pm, ideal, missing)
189+
if err != nil {
190+
return nil, err
191+
}
192+
diagnostics = append(diagnostics, missingModuleDiagnostics...)
193+
}
194+
195+
// Opt: if this is the only diagnostic, we can avoid textual edits and just
196+
// run the Go command.
197+
//
198+
// See also the documentation for command.RemoveDependencyArgs.OnlyDiagnostic.
199+
onlyDiagnostic := len(diagnostics) == 0 && len(unused) == 1
200+
for _, req := range unused {
201+
srcErr, err := unusedDiagnostic(pm.Mapper, req, onlyDiagnostic)
202+
if err != nil {
203+
return nil, err
204+
}
205+
diagnostics = append(diagnostics, srcErr)
206+
}
207+
return diagnostics, nil
208+
}
209+
210+
func missingModuleDiagnostics(ctx context.Context, snapshot *snapshot, pm *source.ParsedModule, ideal *modfile.File, missing map[string]*modfile.Require) ([]*source.Diagnostic, error) {
186211
missingModuleFixes := map[*modfile.Require][]source.SuggestedFix{}
212+
var diagnostics []*source.Diagnostic
187213
for _, req := range missing {
188214
srcDiag, err := missingModuleDiagnostic(pm, req)
189215
if err != nil {
@@ -290,15 +316,6 @@ func modTidyDiagnostics(ctx context.Context, snapshot *snapshot, pm *source.Pars
290316
}
291317
}
292318
}
293-
// Finally, add errors for any unused dependencies.
294-
onlyDiagnostic := len(diagnostics) == 0 && len(unused) == 1
295-
for _, req := range unused {
296-
srcErr, err := unusedDiagnostic(pm.Mapper, req, onlyDiagnostic)
297-
if err != nil {
298-
return nil, err
299-
}
300-
diagnostics = append(diagnostics, srcErr)
301-
}
302319
return diagnostics, nil
303320
}
304321

gopls/internal/lsp/command.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,10 +360,9 @@ func (c *commandHandler) RemoveDependency(ctx context.Context, args command.Remo
360360
progress: "Removing dependency",
361361
forURI: args.URI,
362362
}, func(ctx context.Context, deps commandDeps) error {
363-
// If the module is tidied apart from the one unused diagnostic, we can
364-
// run `go get module@none`, and then run `go mod tidy`. Otherwise, we
365-
// must make textual edits.
366-
// TODO(rstambler): In Go 1.17+, we will be able to use the go command
363+
// See the documentation for OnlyDiagnostic.
364+
//
365+
// TODO(rfindley): In Go 1.17+, we will be able to use the go command
367366
// without checking if the module is tidy.
368367
if args.OnlyDiagnostic {
369368
return c.s.runGoModUpdateCommands(ctx, deps.snapshot, args.URI.SpanURI(), func(invoke func(...string) (*bytes.Buffer, error)) error {

gopls/internal/lsp/command/interface.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,10 @@ type RemoveDependencyArgs struct {
236236
// The go.mod file URI.
237237
URI protocol.DocumentURI
238238
// The module path to remove.
239-
ModulePath string
239+
ModulePath string
240+
// If the module is tidied apart from the one unused diagnostic, we can
241+
// run `go get module@none`, and then run `go mod tidy`. Otherwise, we
242+
// must make textual edits.
240243
OnlyDiagnostic bool
241244
}
242245

gopls/internal/lsp/mod/diagnostics.go

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,14 @@ package mod
99
import (
1010
"context"
1111
"fmt"
12+
"runtime"
1213
"sort"
1314
"strings"
15+
"sync"
1416

1517
"golang.org/x/mod/modfile"
1618
"golang.org/x/mod/semver"
19+
"golang.org/x/sync/errgroup"
1720
"golang.org/x/tools/gopls/internal/govulncheck"
1821
"golang.org/x/tools/gopls/internal/lsp/command"
1922
"golang.org/x/tools/gopls/internal/lsp/protocol"
@@ -58,24 +61,36 @@ func VulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot) (ma
5861
}
5962

6063
func collectDiagnostics(ctx context.Context, snapshot source.Snapshot, diagFn func(context.Context, source.Snapshot, source.FileHandle) ([]*source.Diagnostic, error)) (map[span.URI][]*source.Diagnostic, error) {
64+
65+
g, ctx := errgroup.WithContext(ctx)
66+
cpulimit := runtime.GOMAXPROCS(0)
67+
g.SetLimit(cpulimit)
68+
69+
var mu sync.Mutex
6170
reports := make(map[span.URI][]*source.Diagnostic)
71+
6272
for _, uri := range snapshot.ModFiles() {
63-
fh, err := snapshot.ReadFile(ctx, uri)
64-
if err != nil {
65-
return nil, err
66-
}
67-
reports[fh.URI()] = []*source.Diagnostic{}
68-
diagnostics, err := diagFn(ctx, snapshot, fh)
69-
if err != nil {
70-
return nil, err
71-
}
72-
for _, d := range diagnostics {
73-
fh, err := snapshot.ReadFile(ctx, d.URI)
73+
uri := uri
74+
g.Go(func() error {
75+
fh, err := snapshot.ReadFile(ctx, uri)
7476
if err != nil {
75-
return nil, err
77+
return err
7678
}
77-
reports[fh.URI()] = append(reports[fh.URI()], d)
78-
}
79+
diagnostics, err := diagFn(ctx, snapshot, fh)
80+
if err != nil {
81+
return err
82+
}
83+
for _, d := range diagnostics {
84+
mu.Lock()
85+
reports[d.URI] = append(reports[fh.URI()], d)
86+
mu.Unlock()
87+
}
88+
return nil
89+
})
90+
}
91+
92+
if err := g.Wait(); err != nil {
93+
return nil, err
7994
}
8095
return reports, nil
8196
}

gopls/internal/lsp/source/api_json.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)