From e47c3d98c3071163206f20da3b00aaf9e9bbe4dc Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 28 Jun 2019 19:32:53 -0400 Subject: [PATCH] internal/lsp: check file content on disk when opening As per discussion on golang/go#32810, to avoid the `go list` storm caused by many files being opened, we check if the file content opened is equivalent to the content on disk. If so, we mark this file as "on disk" so that we don't send it as an overlay to go/packages. Updates golang/go#32810 Change-Id: I0a520cf91bbe933c9afb76d0842f5556ac4e5b28 Reviewed-on: https://go-review.googlesource.com/c/tools/+/184257 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cache/session.go | 41 +++++++++++++++++++++++----- internal/lsp/source/view.go | 2 +- internal/lsp/text_synchronization.go | 22 +++++++++------ 3 files changed, 49 insertions(+), 16 deletions(-) diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 16a928ea3a8..f16814f0882 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -44,9 +44,9 @@ type overlay struct { hash string kind source.FileKind - // onDisk is true if a file has been saved on disk, + // sameContentOnDisk is true if a file has been saved on disk, // and therefore does not need to be part of the overlay sent to go/packages. - onDisk bool + sameContentOnDisk bool } func (s *session) Shutdown(ctx context.Context) { @@ -182,14 +182,17 @@ func (s *session) Logger() xlog.Logger { return s.log } -func (s *session) DidOpen(ctx context.Context, uri span.URI) { +func (s *session) DidOpen(ctx context.Context, uri span.URI, text []byte) { + // Mark the file as open. s.openFiles.Store(uri, true) + // Read the file on disk and compare it to the text provided. + // If it is the same as on disk, we can avoid sending it as an overlay to go/packages. + s.openOverlay(ctx, uri, text) + // Mark the file as just opened so that we know to re-run packages.Load on it. // We do this because we may not be aware of all of the packages the file belongs to. - // A file may be in multiple views. - // For each view, get the file and mark it as just opened. for _, view := range s.views { if strings.HasPrefix(string(uri), string(view.Folder())) { f, err := view.GetFile(ctx, uri) @@ -215,7 +218,7 @@ func (s *session) DidSave(uri span.URI) { defer s.overlayMu.Unlock() if overlay, ok := s.overlays[uri]; ok { - overlay.onDisk = true + overlay.sameContentOnDisk = true } } @@ -255,6 +258,30 @@ func (s *session) SetOverlay(uri span.URI, data []byte) { } } +// openOverlay adds the file content to the overlay. +// It also checks if the provided content is equivalent to the file's content on disk. +func (s *session) openOverlay(ctx context.Context, uri span.URI, data []byte) { + s.overlayMu.Lock() + defer func() { + s.overlayMu.Unlock() + s.filesWatchMap.Notify(uri) + }() + s.overlays[uri] = &overlay{ + session: s, + uri: uri, + data: data, + hash: hashContents(data), + } + _, hash, err := s.cache.GetFile(uri).Read(ctx) + if err != nil { + s.log.Errorf(ctx, "failed to read %s: %v", uri, err) + return + } + if hash == s.overlays[uri].hash { + s.overlays[uri].sameContentOnDisk = true + } +} + func (s *session) readOverlay(uri span.URI) *overlay { s.overlayMu.Lock() defer s.overlayMu.Unlock() @@ -272,7 +299,7 @@ func (s *session) buildOverlay() map[string][]byte { overlays := make(map[string][]byte) for uri, overlay := range s.overlays { - if overlay.onDisk { + if overlay.sameContentOnDisk { continue } overlays[uri.Filename()] = overlay.data diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index b2619e99161..872e463cb89 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -154,7 +154,7 @@ type Session interface { FileSystem // DidOpen is invoked each time a file is opened in the editor. - DidOpen(ctx context.Context, uri span.URI) + DidOpen(ctx context.Context, uri span.URI, text []byte) // DidSave is invoked each time an open file is saved in the editor. DidSave(uri span.URI) diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 4cd885322a2..7fe8d81ec62 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -17,8 +17,18 @@ import ( func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error { uri := span.NewURI(params.TextDocument.URI) - s.session.DidOpen(ctx, uri) - return s.cacheAndDiagnose(ctx, uri, []byte(params.TextDocument.Text)) + text := []byte(params.TextDocument.Text) + + // Open the file. + s.session.DidOpen(ctx, uri, text) + + // Run diagnostics on the newly-changed file. + view := s.session.ViewOf(uri) + go func() { + ctx := view.BackgroundContext() + s.Diagnostics(ctx, view, uri) + }() + return nil } func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDocumentParams) error { @@ -46,16 +56,12 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo } } } - // Cache the new file content and send fresh diagnostics. - return s.cacheAndDiagnose(ctx, uri, []byte(text)) -} - -func (s *Server) cacheAndDiagnose(ctx context.Context, uri span.URI, content []byte) error { view := s.session.ViewOf(uri) - if err := view.SetContent(ctx, uri, content); err != nil { + if err := view.SetContent(ctx, uri, []byte(text)); err != nil { return err } + // Run diagnostics on the newly-changed file. go func() { ctx := view.BackgroundContext() s.Diagnostics(ctx, view, uri)