diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index 4d835474ef..e6f3b4f8b2 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -45,16 +45,17 @@ import ( ) const ( - descriptorPath = "google/protobuf/descriptor.proto" - refreshCheckStagger = 5 * time.Millisecond + descriptorPath = "google/protobuf/descriptor.proto" + checkRefreshPeriod = 3 * time.Second ) // file is a file that has been opened by the client. // // Mutating a file is thread-safe. type file struct { - lsp *lsp - uri protocol.URI + lsp *lsp + uri protocol.URI + checkWork chan<- struct{} text string // Version is an opaque version identifier given to us by the LSP client. This @@ -134,6 +135,10 @@ func (f *file) Reset(ctx context.Context) { // for this file. func (f *file) Close(ctx context.Context) { f.lsp.fileManager.Close(ctx, f.uri) + if f.checkWork != nil { + close(f.checkWork) + f.checkWork = nil + } } // IsOpenInEditor returns whether this file was opened in the LSP client's @@ -290,28 +295,7 @@ func (f *file) Refresh(ctx context.Context) { f.FindModule(ctx) progress.Report(ctx, "Running Checks", 4.0/6) - // Since checks are a more expensive operation, we do not want to run a check on every - // Refresh call. Instead, we can stagger the checks and only run them periodically by - // spinning them off into a go routine. Then we attempt to lock using the top-level LSP - // lock. It is safe to use because if another LSP call is made, we allow checks to finish - // before resolving a subsequent LSP request. - go func() { - // We stagger the check operation by 5ms and run it for the latest Refresh state. - time.Sleep(refreshCheckStagger) - // Call TryLock, if unnsuccessful, then another thread holds the lock, so we provide a - // debug log and move on. - if !f.lsp.lock.TryLock() { - f.lsp.logger.Debug( - fmt.Sprintf("another thread holds the LSP lock, no new checks started for %v", f.uri), - ) - return - } - // We have successfully obtained the lock, we can now run the checks. - defer f.lsp.lock.Unlock() - f.BuildImages(ctx) - f.RunLints(ctx) - f.RunBreaking(ctx) - }() + f.RunChecks(ctx) progress.Report(ctx, "Indexing Symbols", 5.0/6) f.IndexSymbols(ctx) @@ -324,6 +308,58 @@ func (f *file) Refresh(ctx context.Context) { f.PublishDiagnostics(ctx) } +// RunChecks initiates background checks (lint and breaking) on this file and +// returns immediately. +// +// Checks are executed in a background goroutine to avoid blocking the LSP +// call. Each call to RunChecks invalidates any ongoing checks, triggering a +// fresh run. However, previous checks are not interrupted. The checks acquire +// the LSP mutex. Subsequent LSP calls will wait for the current check to +// complete before proceeding. +// +// Checks are debounce (with the delay defined by checkRefreshPeriod) to avoid +// overwhelming the client with expensive checks. If the file is not open in the +// editor, checks are skipped. Diagnostics are published after checks are run. +func (f *file) RunChecks(ctx context.Context) { + // If we have not yet started a goroutine to run checks, start one. + // This goroutine will run checks in the background and publish diagnostics. + // We debounce checks to avoid spamming the client. + if f.checkWork == nil { + // We use a buffered channel of length one as the check invalidation mechanism. + work := make(chan struct{}, 1) + f.checkWork = work + runChecks := func(ctx context.Context) { + f.lsp.lock.Lock() + defer f.lsp.lock.Unlock() + if !f.IsOpenInEditor() { + // Skip checks if the file is not open in the editor. + return + } + f.lsp.logger.Info(fmt.Sprintf("running checks for %v, %v", f.uri, f.version)) + f.BuildImages(ctx) + f.RunLints(ctx) + f.RunBreaking(ctx) + f.PublishDiagnostics(ctx) // Publish the latest diagnostics. + } + // Start a goroutine to process checks. + go func() { + // Detach from the parent RPC context. + ctx := context.WithoutCancel(ctx) + for range work { + runChecks(ctx) + // Debounce checks to prevent thrashing expensive checks. + time.Sleep(checkRefreshPeriod) + } + }() + } + // Signal the goroutine to invalidate and rerun checks. + select { + case f.checkWork <- struct{}{}: + default: + // Channel is full, checks are already invalidated and will be rerun. + } +} + // RefreshAST reparses the file and generates diagnostics if necessary. // // Returns whether a reparse was necessary.