Skip to content

Commit

Permalink
gopls/internal/lsp: remove the experimentalWatchedFileDelay setting
Browse files Browse the repository at this point in the history
See the associated issue for more details.

Fixes golang/go#55332

Change-Id: I062423c090838c87dbb0c2d21ab448f6c72399c6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/462496
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
  • Loading branch information
findleyr committed Jan 18, 2023
1 parent f46e418 commit f6ea009
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 108 deletions.
17 changes: 0 additions & 17 deletions gopls/doc/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -358,23 +358,6 @@ This option must be set to a valid duration string, for example `"250ms"`.

Default: `"250ms"`.

##### **experimentalWatchedFileDelay** *time.Duration*

**This setting is experimental and may be deleted.**

experimentalWatchedFileDelay controls the amount of time that gopls waits
for additional workspace/didChangeWatchedFiles notifications to arrive,
before processing all such notifications in a single batch. This is
intended for use by LSP clients that don't support their own batching of
file system notifications.

This option must be set to a valid duration string, for example `"100ms"`.

Deprecated: this setting is deprecated and will be removed in a future
version of gopls (https://go.dev/issue/55332)

Default: `"0s"`.

#### Documentation

##### **hoverKind** *enum*
Expand Down
11 changes: 0 additions & 11 deletions gopls/internal/lsp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func NewServer(session *cache.Session, client protocol.ClientCloser) *Server {
diagnosticsSema: make(chan struct{}, concurrentAnalyses),
progress: progress.NewTracker(client),
diagDebouncer: newDebouncer(),
watchedFileDebouncer: newDebouncer(),
}
}

Expand Down Expand Up @@ -106,22 +105,12 @@ type Server struct {
// diagDebouncer is used for debouncing diagnostics.
diagDebouncer *debouncer

// watchedFileDebouncer is used for batching didChangeWatchedFiles notifications.
watchedFileDebouncer *debouncer
fileChangeMu sync.Mutex
pendingOnDiskChanges []*pendingModificationSet

// When the workspace fails to load, we show its status through a progress
// report with an error message.
criticalErrorStatusMu sync.Mutex
criticalErrorStatus *progress.WorkDone
}

type pendingModificationSet struct {
diagnoseDone chan struct{}
changes []source.FileModification
}

func (s *Server) workDoneProgressCancel(ctx context.Context, params *protocol.WorkDoneProgressCancelParams) error {
return s.progress.Cancel(params.Token)
}
Expand Down
8 changes: 0 additions & 8 deletions gopls/internal/lsp/source/api_json.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 1 addition & 16 deletions gopls/internal/lsp/source/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,18 +434,6 @@ type DiagnosticOptions struct {
//
// This option must be set to a valid duration string, for example `"250ms"`.
DiagnosticsDelay time.Duration `status:"advanced"`

// ExperimentalWatchedFileDelay controls the amount of time that gopls waits
// for additional workspace/didChangeWatchedFiles notifications to arrive,
// before processing all such notifications in a single batch. This is
// intended for use by LSP clients that don't support their own batching of
// file system notifications.
//
// This option must be set to a valid duration string, for example `"100ms"`.
//
// Deprecated: this setting is deprecated and will be removed in a future
// version of gopls (https://go.dev/issue/55332)
ExperimentalWatchedFileDelay time.Duration `status:"experimental"`
}

type InlayHintOptions struct {
Expand Down Expand Up @@ -1120,10 +1108,7 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{})
result.setDuration(&o.DiagnosticsDelay)

case "experimentalWatchedFileDelay":
const msg = "experimentalWatchedFileDelay is deprecated, and will " +
"be removed in a future version of gopls (https://go.dev/issue/55332)"
result.softErrorf(msg)
result.setDuration(&o.ExperimentalWatchedFileDelay)
result.deprecated("")

case "experimentalPackageCacheKey":
result.setBool(&o.ExperimentalPackageCacheKey)
Expand Down
57 changes: 1 addition & 56 deletions gopls/internal/lsp/text_synchronization.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,11 @@ import (
"errors"
"fmt"
"path/filepath"
"time"

"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/jsonrpc2"
"golang.org/x/tools/internal/xcontext"
)

// ModificationSource identifies the originating cause of a file modification.
Expand Down Expand Up @@ -220,59 +217,7 @@ func (s *Server) didModifyFiles(ctx context.Context, modifications []source.File
}

onDisk := cause == FromDidChangeWatchedFiles
delay := s.session.Options().ExperimentalWatchedFileDelay
s.fileChangeMu.Lock()
defer s.fileChangeMu.Unlock()
if !onDisk || delay == 0 {
// No delay: process the modifications immediately.
return s.processModifications(ctx, modifications, onDisk, diagnoseDone)
}
// Debounce and batch up pending modifications from watched files.
pending := &pendingModificationSet{
diagnoseDone: diagnoseDone,
changes: modifications,
}
// Invariant: changes appended to s.pendingOnDiskChanges are eventually
// handled in the order they arrive. This guarantee is only partially
// enforced here. Specifically:
// 1. s.fileChangesMu ensures that the append below happens in the order
// notifications were received, so that the changes within each batch are
// ordered properly.
// 2. The debounced func below holds s.fileChangesMu while processing all
// changes in s.pendingOnDiskChanges, ensuring that no batches are
// processed out of order.
// 3. Session.ExpandModificationsToDirectories and Session.DidModifyFiles
// process changes in order.
s.pendingOnDiskChanges = append(s.pendingOnDiskChanges, pending)
ctx = xcontext.Detach(ctx)
okc := s.watchedFileDebouncer.debounce("", 0, time.After(delay))
go func() {
if ok := <-okc; !ok {
return
}
s.fileChangeMu.Lock()
var allChanges []source.FileModification
// For accurate progress notifications, we must notify all goroutines
// waiting for the diagnose pass following a didChangeWatchedFiles
// notification. This is necessary for regtest assertions.
var dones []chan struct{}
for _, pending := range s.pendingOnDiskChanges {
allChanges = append(allChanges, pending.changes...)
dones = append(dones, pending.diagnoseDone)
}

allDone := make(chan struct{})
if err := s.processModifications(ctx, allChanges, onDisk, allDone); err != nil {
event.Error(ctx, "processing delayed file changes", err)
}
s.pendingOnDiskChanges = nil
s.fileChangeMu.Unlock()
<-allDone
for _, done := range dones {
close(done)
}
}()
return nil
return s.processModifications(ctx, modifications, onDisk, diagnoseDone)
}

// processModifications update server state to reflect file changes, and
Expand Down

0 comments on commit f6ea009

Please sign in to comment.