gopls/internal/lsp/cache: simplify canonical URI cache
The View's URI canonicalization cache entry, fileBase,
used to hold a list of URIs even though only the first
is ever needed. This change simplifies it to a pair
of (URI, filename). This should reduce allocation slightly.
Also:
- Eliminate the unnecessary pointer type to further reduce allocation.
- Use a dedicated mutex for the two maps.
- Update the names to better reflect the purpose.
- Document various failed approaches to optimization
and two problems in the existing logic, one
of correctness, the other of performance.
Change-Id: Ic74ae4bae8864de292fe97d26c5f9aacf2367961
Reviewed-on: https://go-review.googlesource.com/c/tools/+/454435
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 80de11a..1ef4622 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -1287,12 +1287,12 @@
}
func (s *snapshot) FindFile(uri span.URI) source.VersionedFileHandle {
- f := s.view.getFile(uri)
+ uri, _ = s.view.canonicalURI(uri, true)
s.mu.Lock()
defer s.mu.Unlock()
- result, _ := s.files.Get(f.URI())
+ result, _ := s.files.Get(uri)
return result
}
@@ -1302,11 +1302,22 @@
// GetVersionedFile succeeds even if the file does not exist. A non-nil error return
// indicates some type of internal error, for example if ctx is cancelled.
func (s *snapshot) GetVersionedFile(ctx context.Context, uri span.URI) (source.VersionedFileHandle, error) {
- f := s.view.getFile(uri)
+ uri, _ = s.view.canonicalURI(uri, true)
s.mu.Lock()
defer s.mu.Unlock()
- return s.getFileLocked(ctx, f)
+
+ if fh, ok := s.files.Get(uri); ok {
+ return fh, nil
+ }
+
+ fh, err := s.view.cache.getFile(ctx, uri) // read the file
+ if err != nil {
+ return nil, err
+ }
+ closed := &closedFile{fh}
+ s.files.Set(uri, closed)
+ return closed, nil
}
// GetFile implements the fileSource interface by wrapping GetVersionedFile.
@@ -1314,20 +1325,6 @@
return s.GetVersionedFile(ctx, uri)
}
-func (s *snapshot) getFileLocked(ctx context.Context, f *fileBase) (source.VersionedFileHandle, error) {
- if fh, ok := s.files.Get(f.URI()); ok {
- return fh, nil
- }
-
- fh, err := s.view.cache.getFile(ctx, f.URI()) // read the file
- if err != nil {
- return nil, err
- }
- closed := &closedFile{fh}
- s.files.Set(f.URI(), closed)
- return closed, nil
-}
-
func (s *snapshot) IsOpen(uri span.URI) bool {
s.mu.Lock()
defer s.mu.Unlock()
