Improve LSP performance for files that have a lot of cross-file refs (#…

…3403)

Since removing concurrency from the LSP, performance has degraded substantially. This PR fixes some performance issues by being smart about doing less work. Most of this will be mooted by bufbuild/protocompile#355, but until we have intelligent memoization, we can use some dumb heuristics to improve perf.

First, we don't send progress notifications for files that have not been opened by the client's editor. Hammering the Unix socket with notifications is a major source of slowdown, and these notifications are not useful to the user, because they are about files they do not care about.

Second, we don't send diagnostics for the same. These files get reparsed when opened in the editor regardless, so this doesn't risk staleness.

Third, we were previously reindexing imported files once per cross-file ref. This is clearly an oversight on my part, which I suspect was caused due to nasty merge conflicts on my last PR. I noticed because protovalidate/priv/private.proto was getting hammered in the logs -- all because validate.proto references the priv symbol dozens of times. 🤦

After fixing all of these, the LSP went from sluggish to snappy (in VSCode).

private/buf/buflsp/file.go

@@ -131,6 +131,15 @@ func (f *file) Close(ctx context.Context) {
 	f.lsp.fileManager.Close(ctx, f.uri)
 }
 
+// IsOpenInEditor returns whether this file was opened in the LSP client's
+// editor.
+//
+// Some files may be opened as dependencies, so we want to avoid doing extra
+// work like sending progress notifications.
+func (f *file) IsOpenInEditor() bool {
+	return f.version != -1 // See [file.ReadFromDisk].
+}
+
 // ReadFromDisk reads this file from disk if it has never had data loaded into it before.
 //
 // If it has been read from disk before, or has received updates from the LSP client, this
@@ -165,7 +174,12 @@ func (f *file) Update(ctx context.Context, version int32, text string) {
 //
 // If deep is set, this will also load imports and refresh those, too.
 func (f *file) Refresh(ctx context.Context) {
-	progress := newProgress(f.lsp)
+	var progress *progress
+	if f.IsOpenInEditor() {
+		// NOTE: Nil progress does nothing when methods are called. This helps
+		// minimize RPC spam from the client when indexing lots of files.
+		progress = newProgress(f.lsp)
+	}
 	progress.Begin(ctx, "Indexing")
 
 	progress.Report(ctx, "Parsing AST", 1.0/6)
@@ -232,6 +246,14 @@ func (f *file) RefreshAST(ctx context.Context) bool {
 
 // PublishDiagnostics publishes all of this file's diagnostics to the LSP client.
 func (f *file) PublishDiagnostics(ctx context.Context) {
+	if !f.IsOpenInEditor() {
+		// If the file does get opened by the editor, the server will call
+		// Refresh() and this function will retry sending diagnostics. Which is
+		// to say: returning here does not result in stale diagnostics on the
+		// client.
+		return
+	}
+
 	defer slogext.DebugProfile(f.lsp.logger, slog.String("uri", string(f.uri)))()
 
 	// NOTE: We need to avoid sending a JSON null here, so we replace it with
@@ -605,7 +627,8 @@ func (f *file) IndexSymbols(ctx context.Context) {
 	defer slogext.DebugProfile(f.lsp.logger, slog.String("uri", string(f.uri)))()
 
 	// Throw away all the old symbols. Unlike other indexing functions, we rebuild
-	// symbols unconditionally.
+	// symbols unconditionally. This is because if this file depends on a file
+	// that has since been modified, we may need to update references.
 	f.symbols = nil
 
 	// Generate new symbols.
@@ -622,7 +645,6 @@ func (f *file) IndexSymbols(ctx context.Context) {
 		return diff
 	})
 
-	// Now we can drop the lock and search for cross-file references.
 	symbols := f.symbols
 	for _, symbol := range symbols {
 		symbol.ResolveCrossFile(ctx)

private/buf/buflsp/progress.go

@@ -54,6 +54,10 @@ func newProgressFromClient(lsp *lsp, params *protocol.WorkDoneProgressParams) *p
 	}
 }
 
+// Begin informs the client that this progress has begun. Must be called before
+// Report or Done are called.
+//
+// If p is nil, does nothing.
 func (p *progress) Begin(ctx context.Context, title string) {
 	if p == nil {
 		return
@@ -69,6 +73,9 @@ func (p *progress) Begin(ctx context.Context, title string) {
 	})
 }
 
+// Report updates the progress on the client.
+//
+// If p is nil, does nothing.
 func (p *progress) Report(ctx context.Context, message string, percent float64) {
 	if p == nil {
 		return
@@ -85,6 +92,10 @@ func (p *progress) Report(ctx context.Context, message string, percent float64)
 	})
 }
 
+// Done completes the progress on the client, informing the client that it can
+// stop showing a progress bar.
+//
+// If p is nil, does nothing.
 func (p *progress) Done(ctx context.Context) {
 	if p == nil {
 		return

private/buf/buflsp/symbol.go

@@ -169,11 +169,6 @@ func (s *symbol) ResolveCrossFile(ctx context.Context) {
 				return
 			}
 
-			// Fully index the file this reference is in, if different from the current.
-			if s.file != ref.file {
-				ref.file.Refresh(ctx)
-			}
-
 			// Find the definition that contains the type we want.
 			def, node := kind.seeTypeOf.Definition(ctx)
 			if def == nil {