Skip to content

Commit

Permalink
internal/lsp: limit diagnostics concurrency
Browse files Browse the repository at this point in the history
Diagnostics runs cannot be canceled until they finish a package. If a
user has a very expensive package, we may stack up diagnostics runs to
the point where the machine runs out of memory. We see hints of this in
various issues.

To avoid that, only allow one diagnostics run at a time. We can change
the limit later if we want.

Updates golang/go#37223.

Change-Id: Icd0eec4da936153306cf0a1f7175ae2b4b265272
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219958
Reviewed-by: Rebecca Stambler <[email protected]>
  • Loading branch information
heschi committed Feb 19, 2020
1 parent 66de428 commit 3315324
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 9 deletions.
8 changes: 8 additions & 0 deletions internal/lsp/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
defer done()

// Wait for a free diagnostics slot.
select {
case <-ctx.Done():
return nil
case s.diagnosticsSema <- struct{}{}:
}
defer func() { <-s.diagnosticsSema }()

allReports := make(map[diagnosticKey][]source.Diagnostic)
var reportsMu sync.Mutex
var wg sync.WaitGroup
Expand Down
9 changes: 3 additions & 6 deletions internal/lsp/lsp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,9 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
t.Fatal(err)
}
r := &runner{
server: &Server{
session: session,
delivered: map[span.URI]sentDiagnostics{},
},
data: datum,
ctx: ctx,
server: NewServer(session, nil),
data: datum,
ctx: ctx,
}
t.Run(datum.Folder, func(t *testing.T) {
t.Helper()
Expand Down
12 changes: 9 additions & 3 deletions internal/lsp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@ import (
"golang.org/x/tools/internal/span"
)

const concurrentAnalyses = 1

// NewServer creates an LSP server and binds it to handle incoming client
// messages on on the supplied stream.
func NewServer(session source.Session, client protocol.Client) *Server {
return &Server{
delivered: make(map[span.URI]sentDiagnostics),
session: session,
client: client,
delivered: make(map[span.URI]sentDiagnostics),
session: session,
client: client,
diagnosticsSema: make(chan struct{}, concurrentAnalyses),
}
}

Expand Down Expand Up @@ -54,6 +57,9 @@ type Server struct {
// delivered is a cache of the diagnostics that the server has sent.
deliveredMu sync.Mutex
delivered map[span.URI]sentDiagnostics

// diagnosticsSema limits the concurrency of diagnostics runs, which can be expensive.
diagnosticsSema chan struct{}
}

// sentDiagnostics is used to cache diagnostics that have been sent for a given file.
Expand Down

0 comments on commit 3315324

Please sign in to comment.