From 33153249e787ac5bed84f629c28a437bbd614eec Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Tue, 18 Feb 2020 16:44:12 -0500 Subject: [PATCH] internal/lsp: limit diagnostics concurrency 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 --- internal/lsp/diagnostics.go | 8 ++++++++ internal/lsp/lsp_test.go | 9 +++------ internal/lsp/server.go | 12 +++++++++--- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index db8eeb59cd..f4ff11d437 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -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 diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index a893834575..d6febccd16 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -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() diff --git a/internal/lsp/server.go b/internal/lsp/server.go index ed1c00c935..1aad63af5b 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -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), } } @@ -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.