Skip to content

Commit

Permalink
gopls/internal/lsp/cache: simplify tracking of snapshot directories
Browse files Browse the repository at this point in the history
Great care was taken to track known directories in the snapshot without
blocking in snapshot.Clone, introducing significant complexity.

This complexity can be avoided by instead keeping track of observed
directories as files are set in the snapshot. These directories need
only be reset when files are deleted from the snapshot, which is a
relatively rare event.

Also rename filesMap->fileMap, and move to filemap.go, with a new unit
test.

This reduces some path dependence on seen files, as the set of
directories is well defined and depends only on the files in the
snapshot. Previously, when a file was removed, gopls called Stat to
check if the directory still existed, which leads to path dependence: an
add+remove was not the same as nothing at all.

Updates golang/go#57558

Change-Id: I5fd89ce870fa7d8afd19471d150396b1e4ea8875
Reviewed-on: https://go-review.googlesource.com/c/tools/+/525616
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
findleyr committed Sep 5, 2023
1 parent fe324ac commit 36c4f98
Show file tree
Hide file tree
Showing 8 changed files with 324 additions and 301 deletions.
134 changes: 134 additions & 0 deletions gopls/internal/lsp/cache/filemap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Copyright 2022 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package cache

import (
"path/filepath"

"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/persistent"
)

// A fileMap maps files in the snapshot, with some additional bookkeeping:
// It keeps track of overlays as well as directories containing any observed
// file.
type fileMap struct {
files *persistent.Map[span.URI, source.FileHandle]
overlays *persistent.Map[span.URI, *Overlay] // the subset of files that are overlays
dirs *persistent.Set[string] // all dirs containing files; if nil, dirs have not been initialized
}

func newFileMap() *fileMap {
return &fileMap{
files: new(persistent.Map[span.URI, source.FileHandle]),
overlays: new(persistent.Map[span.URI, *Overlay]),
dirs: new(persistent.Set[string]),
}
}

func (m *fileMap) Clone() *fileMap {
m2 := &fileMap{
files: m.files.Clone(),
overlays: m.overlays.Clone(),
}
if m.dirs != nil {
m2.dirs = m.dirs.Clone()
}
return m2
}

func (m *fileMap) Destroy() {
m.files.Destroy()
m.overlays.Destroy()
if m.dirs != nil {
m.dirs.Destroy()
}
}

// Get returns the file handle mapped by the given key, or (nil, false) if the
// key is not present.
func (m *fileMap) Get(key span.URI) (source.FileHandle, bool) {
return m.files.Get(key)
}

// Range calls f for each (uri, fh) in the map.
func (m *fileMap) Range(f func(uri span.URI, fh source.FileHandle)) {
m.files.Range(f)
}

// Set stores the given file handle for key, updating overlays and directories
// accordingly.
func (m *fileMap) Set(key span.URI, fh source.FileHandle) {
m.files.Set(key, fh, nil)

// update overlays
if o, ok := fh.(*Overlay); ok {
m.overlays.Set(key, o, nil)
} else {
// Setting a non-overlay must delete the corresponding overlay, to preserve
// the accuracy of the overlay set.
m.overlays.Delete(key)
}

// update dirs
if m.dirs == nil {
m.initDirs()
} else {
m.addDirs(key)
}
}

func (m *fileMap) initDirs() {
m.dirs = new(persistent.Set[string])
m.files.Range(func(u span.URI, _ source.FileHandle) {
m.addDirs(u)
})
}

// addDirs adds all directories containing u to the dirs set.
func (m *fileMap) addDirs(u span.URI) {
dir := filepath.Dir(u.Filename())
for dir != "" && !m.dirs.Contains(dir) {
m.dirs.Add(dir)
dir = filepath.Dir(dir)
}
}

// Delete removes a file from the map, and updates overlays and dirs
// accordingly.
func (m *fileMap) Delete(key span.URI) {
m.files.Delete(key)
m.overlays.Delete(key)

// Deleting a file may cause the set of dirs to shrink; therefore we must
// re-evaluate the dir set.
//
// Do this lazily, to avoid work if there are multiple deletions in a row.
if m.dirs != nil {
m.dirs.Destroy()
m.dirs = nil
}
}

// Overlays returns a new unordered array of overlay files.
func (m *fileMap) Overlays() []*Overlay {
var overlays []*Overlay
m.overlays.Range(func(_ span.URI, o *Overlay) {
overlays = append(overlays, o)
})
return overlays
}

// Dirs reports returns the set of dirs observed by the fileMap.
//
// This operation mutates the fileMap.
// The result must not be mutated by the caller.
func (m *fileMap) Dirs() *persistent.Set[string] {
if m.dirs == nil {
m.initDirs()
}
return m.dirs
}
108 changes: 108 additions & 0 deletions gopls/internal/lsp/cache/filemap_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright 2023 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package cache

import (
"path/filepath"
"sort"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/span"
)

func TestFileMap(t *testing.T) {
const (
set = iota
del
)
type op struct {
op int // set or remove
path string
overlay bool
}
tests := []struct {
label string
ops []op
wantFiles []string
wantOverlays []string
wantDirs []string
}{
{"empty", nil, nil, nil, nil},
{"singleton", []op{
{set, "/a/b", false},
}, []string{"/a/b"}, nil, []string{"/", "/a"}},
{"overlay", []op{
{set, "/a/b", true},
}, []string{"/a/b"}, []string{"/a/b"}, []string{"/", "/a"}},
{"replace overlay", []op{
{set, "/a/b", true},
{set, "/a/b", false},
}, []string{"/a/b"}, nil, []string{"/", "/a"}},
{"multi dir", []op{
{set, "/a/b", false},
{set, "/c/d", false},
}, []string{"/a/b", "/c/d"}, nil, []string{"/", "/a", "/c"}},
{"empty dir", []op{
{set, "/a/b", false},
{set, "/c/d", false},
{del, "/a/b", false},
}, []string{"/c/d"}, nil, []string{"/", "/c"}},
}

// Normalize paths for windows compatibility.
normalize := func(path string) string {
return strings.TrimPrefix(filepath.ToSlash(path), "C:") // the span packages adds 'C:'
}

for _, test := range tests {
t.Run(test.label, func(t *testing.T) {
m := newFileMap()
for _, op := range test.ops {
uri := span.URIFromPath(filepath.FromSlash(op.path))
switch op.op {
case set:
var fh source.FileHandle
if op.overlay {
fh = &Overlay{uri: uri}
} else {
fh = &DiskFile{uri: uri}
}
m.Set(uri, fh)
case del:
m.Delete(uri)
}
}

var gotFiles []string
m.Range(func(uri span.URI, _ source.FileHandle) {
gotFiles = append(gotFiles, normalize(uri.Filename()))
})
sort.Strings(gotFiles)
if diff := cmp.Diff(test.wantFiles, gotFiles); diff != "" {
t.Errorf("Files mismatch (-want +got):\n%s", diff)
}

var gotOverlays []string
for _, o := range m.Overlays() {
gotOverlays = append(gotOverlays, normalize(o.URI().Filename()))
}
if diff := cmp.Diff(test.wantOverlays, gotOverlays); diff != "" {
t.Errorf("Overlays mismatch (-want +got):\n%s", diff)
}

var gotDirs []string
m.Dirs().Range(func(dir string) {
gotDirs = append(gotDirs, normalize(dir))
})
sort.Strings(gotDirs)
if diff := cmp.Diff(test.wantDirs, gotDirs); diff != "" {
t.Errorf("Dirs mismatch (-want +got):\n%s", diff)
}
})
}
}
78 changes: 0 additions & 78 deletions gopls/internal/lsp/cache/maps.go

This file was deleted.

Loading

0 comments on commit 36c4f98

Please sign in to comment.