diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go index e368fa3639..a08cb1935b 100644 --- a/internal/lsp/cache/gofile.go +++ b/internal/lsp/cache/gofile.go @@ -11,6 +11,7 @@ import ( "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/telemetry" + "golang.org/x/tools/internal/span" errors "golang.org/x/xerrors" ) @@ -27,9 +28,6 @@ type goFile struct { missingImports map[packagePath]struct{} imports []*ast.ImportSpec - - cphs map[packageKey]*checkPackageHandle - meta map[packageID]*metadata } type packageKey struct { @@ -37,117 +35,70 @@ type packageKey struct { mode source.ParseMode } -func (f *goFile) CheckPackageHandles(ctx context.Context) ([]source.CheckPackageHandle, error) { +func (f *goFile) CheckPackageHandles(ctx context.Context) (cphs []source.CheckPackageHandle, err error) { ctx = telemetry.File.With(ctx, f.URI()) fh := f.Handle(ctx) - if f.isDirty(ctx, fh) { - if err := f.view.loadParseTypecheck(ctx, f, fh); err != nil { + cphs = f.isDirty(ctx, fh) + if len(cphs) == 0 { + cphs, err = f.view.loadParseTypecheck(ctx, f, fh) + if err != nil { return nil, err } } - - f.mu.Lock() - defer f.mu.Unlock() - - var results []source.CheckPackageHandle - seenIDs := make(map[string]bool) - for _, cph := range f.cphs { - if seenIDs[cph.ID()] { - continue - } - if cph.mode() < source.ParseFull { - continue - } - results = append(results, cph) - seenIDs[cph.ID()] = true - } - if len(results) == 0 { + if len(cphs) == 0 { return nil, errors.Errorf("no CheckPackageHandles for %s", f.URI()) } - return results, nil + return cphs, nil } -func (f *goFile) GetActiveReverseDeps(ctx context.Context) (files []source.GoFile) { - seen := make(map[packageID]struct{}) // visited packages - results := make(map[*goFile]struct{}) - - f.view.mu.Lock() - defer f.view.mu.Unlock() - - f.view.mcache.mu.Lock() - defer f.view.mcache.mu.Unlock() - - for _, m := range f.metadata() { - f.view.reverseDeps(ctx, seen, results, m.id) - for f := range results { - if f == nil { - continue - } - // Don't return any of the active files in this package. - f.mu.Lock() - _, ok := f.meta[m.id] - f.mu.Unlock() - if ok { - continue - } - - files = append(files, f) +func (v *view) GetActiveReverseDeps(ctx context.Context, uri span.URI) (results []source.CheckPackageHandle) { + var ( + rdeps = v.reverseDependencies(ctx, uri) + files = v.openFiles(ctx, rdeps) + seen = make(map[span.URI]struct{}) + ) + for _, f := range files { + if _, ok := seen[f.URI()]; ok { + continue } - } - return files -} - -func (v *view) reverseDeps(ctx context.Context, seen map[packageID]struct{}, results map[*goFile]struct{}, id packageID) { - if _, ok := seen[id]; ok { - return - } - seen[id] = struct{}{} - m, ok := v.mcache.packages[id] - if !ok { - return - } - for _, uri := range m.files { - // Call unlocked version of getFile since we hold the lock on the view. - if f, err := v.getFile(ctx, uri, source.Go); err == nil && v.session.IsOpen(uri) { - results[f.(*goFile)] = struct{}{} + gof, ok := f.(source.GoFile) + if !ok { + continue } + cphs, err := gof.CheckPackageHandles(ctx) + if err != nil { + continue + } + cph := source.WidestCheckPackageHandle(cphs) + for _, ph := range cph.Files() { + seen[ph.File().Identity().URI] = struct{}{} + } + results = append(results, cph) } - for parentID := range m.parents { - v.reverseDeps(ctx, seen, results, parentID) - } -} - -// metadata assumes that the caller holds the f.mu lock. -func (f *goFile) metadata() []*metadata { - result := make([]*metadata, 0, len(f.meta)) - for _, m := range f.meta { - result = append(result, m) - } - return result + return results } // isDirty is true if the file needs to be type-checked. // It assumes that the file's view's mutex is held by the caller. -func (f *goFile) isDirty(ctx context.Context, fh source.FileHandle) bool { - f.mu.Lock() - defer f.mu.Unlock() - - if len(f.meta) == 0 || len(f.cphs) == 0 { - return true +func (f *goFile) isDirty(ctx context.Context, fh source.FileHandle) []source.CheckPackageHandle { + meta, cphs := f.view.getSnapshot(f.URI()) + if len(meta) == 0 { + return nil } - if len(f.missingImports) > 0 { - return true - } - for key, cph := range f.cphs { + var results []source.CheckPackageHandle + for key, cph := range cphs { + // If we're explicitly checking if a file needs to be type-checked, + // we need it to be fully parsed. if key.mode != source.ParseFull { continue } + // Check if there is a fully-parsed package to which this file belongs. for _, file := range cph.Files() { if file.File().Identity() == fh.Identity() { - return false + results = append(results, cph) } } } - return true + return results } diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index fc2f39988b..fad0ec9683 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -19,62 +19,58 @@ import ( errors "golang.org/x/xerrors" ) -func (view *view) loadParseTypecheck(ctx context.Context, f *goFile, fh source.FileHandle) error { +func (v *view) loadParseTypecheck(ctx context.Context, f *goFile, fh source.FileHandle) ([]source.CheckPackageHandle, error) { ctx, done := trace.StartSpan(ctx, "cache.view.loadParseTypeCheck", telemetry.URI.Of(f.URI())) defer done() - meta, err := view.load(ctx, f, fh) + meta, err := v.load(ctx, f, fh) if err != nil { - return err + return nil, err } + var ( + cphs []*checkPackageHandle + results []source.CheckPackageHandle + ) for _, m := range meta { imp := &importer{ - view: view, - config: view.Config(ctx), + view: v, + config: v.Config(ctx), seen: make(map[packageID]struct{}), topLevelPackageID: m.id, } cph, err := imp.checkPackageHandle(ctx, m) if err != nil { - log.Error(ctx, "loadParseTypeCheck: failed to get CheckPackageHandle", err, telemetry.Package.Of(m.id)) - continue + return nil, err } - // Cache the package type information for the top-level package. for _, ph := range cph.files { - file, _, _, err := ph.Parse(ctx) - if err != nil { - return err - } - f, err := imp.view.GetFile(ctx, ph.File().Identity().URI) - if err != nil { - return errors.Errorf("no such file %s: %v", ph.File().Identity().URI, err) - } - gof, ok := f.(*goFile) - if !ok { - return errors.Errorf("%s is not a Go file", ph.File().Identity().URI) - } - if err := cachePerFile(ctx, gof, ph.Mode(), file.Imports, cph); err != nil { - return err + if err := v.cachePerFile(ctx, ph); err != nil { + return nil, err } } + cphs = append(cphs, cph) + results = append(results, cph) } - return nil + // Cache the package type information for the top-level package. + v.updatePackages(cphs) + return results, nil } -func cachePerFile(ctx context.Context, f *goFile, mode source.ParseMode, imports []*ast.ImportSpec, cph *checkPackageHandle) error { - f.mu.Lock() - defer f.mu.Unlock() - - f.imports = imports - - if f.cphs == nil { - f.cphs = make(map[packageKey]*checkPackageHandle) +func (v *view) cachePerFile(ctx context.Context, ph source.ParseGoHandle) error { + file, _, _, err := ph.Parse(ctx) + if err != nil { + return err } - f.cphs[packageKey{ - id: cph.m.id, - mode: mode, - }] = cph - + f, err := v.GetFile(ctx, ph.File().Identity().URI) + if err != nil { + return err + } + gof, ok := f.(*goFile) + if !ok { + return errors.Errorf("%s is not a Go file", ph.File().Identity().URI) + } + gof.mu.Lock() + gof.imports = file.Imports + gof.mu.Unlock() return nil } @@ -82,12 +78,6 @@ func (view *view) load(ctx context.Context, f *goFile, fh source.FileHandle) ([] ctx, done := trace.StartSpan(ctx, "cache.view.load", telemetry.URI.Of(f.URI())) defer done() - view.mu.Lock() - defer view.mu.Unlock() - - view.mcache.mu.Lock() - defer view.mcache.mu.Unlock() - // Get the metadata for the file. meta, err := view.checkMetadata(ctx, f, fh) if err != nil { @@ -101,23 +91,24 @@ func (view *view) load(ctx context.Context, f *goFile, fh source.FileHandle) ([] // checkMetadata determines if we should run go/packages.Load for this file. // If yes, update the metadata for the file and its package. -func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) (metadata []*metadata, err error) { - // Check if we need to re-run go/packages before loading the package. - var runGopackages bool - func() { - f.mu.Lock() - defer f.mu.Unlock() +func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) ([]*metadata, error) { + var shouldRunGopackages bool - runGopackages, err = v.shouldRunGopackages(ctx, f, fh) - metadata = f.metadata() - }() + m := v.getMetadata(fh.Identity().URI) + if len(m) == 0 { + shouldRunGopackages = true + } + // Get file content in case we don't already have it. + parsed, _, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx) if err != nil { return nil, err } + // Check if we need to re-run go/packages before loading the package. + shouldRunGopackages = shouldRunGopackages || v.shouldRunGopackages(ctx, f, parsed, m) // The package metadata is correct as-is, so just return it. - if !runGopackages { - return metadata, nil + if !shouldRunGopackages { + return m, nil } // Don't bother running go/packages if the context has been canceled. @@ -129,6 +120,8 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandl defer done() pkgs, err := packages.Load(v.Config(ctx), fmt.Sprintf("file=%s", f.filename())) + log.Print(ctx, "go/packages.Load", tag.Of("packages", len(pkgs))) + if len(pkgs) == 0 { if err == nil { err = errors.Errorf("go/packages.Load: no packages found for %s", f.filename()) @@ -136,200 +129,32 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandl // Return this error as a diagnostic to the user. return nil, err } - // Track missing imports as we look at the package's errors. - missingImports := make(map[packagePath]struct{}) - - // Clear metadata since we are re-running go/packages. - // Reset the file's metadata and type information if we are re-running `go list`. - f.mu.Lock() - for k := range f.meta { - delete(f.meta, k) - } - for k := range f.cphs { - delete(f.cphs, k) - } - f.mu.Unlock() - - log.Print(ctx, "go/packages.Load", tag.Of("packages", len(pkgs))) - for _, pkg := range pkgs { - log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles)) - // Build the import graph for this package. - if err := v.link(ctx, &importGraph{ - pkgPath: packagePath(pkg.PkgPath), - pkg: pkg, - parent: nil, - missingImports: make(map[packagePath]struct{}), - }); err != nil { - return nil, err - } - } - m, err := validateMetadata(ctx, missingImports, f) - if err != nil { - return nil, err - } - return m, nil -} - -func validateMetadata(ctx context.Context, missingImports map[packagePath]struct{}, f *goFile) ([]*metadata, error) { - f.mu.Lock() - defer f.mu.Unlock() - - // If `go list` failed to get data for the file in question (this should never happen). - if len(f.meta) == 0 { - return nil, errors.Errorf("loadParseTypecheck: no metadata found for %v", f.filename()) - } - - // If we have already seen these missing imports before, and we have type information, - // there is no need to continue. - if sameSet(missingImports, f.missingImports) && len(f.cphs) != 0 { - return nil, nil - } - - // Otherwise, update the missing imports map. - f.missingImports = missingImports - - return f.metadata(), nil -} - -func sameSet(x, y map[packagePath]struct{}) bool { - if len(x) != len(y) { - return false - } - for k := range x { - if _, ok := y[k]; !ok { - return false - } - } - return true + return v.updateMetadata(ctx, f.URI(), pkgs) } // shouldRunGopackages reparses a file's package and import declarations to // determine if they have changed. // It assumes that the caller holds the lock on the f.mu lock. -func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, fh source.FileHandle) (result bool, err error) { - if len(f.meta) == 0 || len(f.missingImports) > 0 { - return true, nil - } - // Get file content in case we don't already have it. - parsed, _, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx) - if err != nil { - return false, err - } +func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, file *ast.File, metadata []*metadata) bool { // Check if the package's name has changed, by checking if this is a filename // we already know about, and if so, check if its package name has changed. - for _, m := range f.meta { + for _, m := range metadata { for _, uri := range m.files { if span.CompareURI(uri, f.URI()) == 0 { - if m.name != parsed.Name.Name { - return true, nil + if m.name != file.Name.Name { + return true } } } } // If the package's imports have changed, re-run `go list`. - if len(f.imports) != len(parsed.Imports) { - return true, nil + if len(f.imports) != len(file.Imports) { + return true } for i, importSpec := range f.imports { - if importSpec.Path.Value != parsed.Imports[i].Path.Value { - return true, nil + if importSpec.Path.Value != file.Imports[i].Path.Value { + return true } } - return false, nil -} - -type importGraph struct { - pkgPath packagePath - pkg *packages.Package - parent *metadata - missingImports map[packagePath]struct{} -} - -func (v *view) link(ctx context.Context, g *importGraph) error { - // Recreate the metadata rather than reusing it to avoid locking. - m := &metadata{ - id: packageID(g.pkg.ID), - pkgPath: g.pkgPath, - name: g.pkg.Name, - typesSizes: g.pkg.TypesSizes, - errors: g.pkg.Errors, - } - for _, filename := range g.pkg.CompiledGoFiles { - m.files = append(m.files, span.FileURI(filename)) - - // Call the unlocked version of getFile since we are holding the view's mutex. - f, err := v.getFile(ctx, span.FileURI(filename), source.Go) - if err != nil { - log.Error(ctx, "no file", err, telemetry.File.Of(filename)) - continue - } - gof, ok := f.(*goFile) - if !ok { - log.Error(ctx, "not a Go file", nil, telemetry.File.Of(filename)) - continue - } - // Cache the metadata for this file. - gof.mu.Lock() - if gof.meta == nil { - gof.meta = make(map[packageID]*metadata) - } - gof.meta[m.id] = m - gof.mu.Unlock() - } - - // Preserve the import graph. - if original, ok := v.mcache.packages[m.id]; ok { - m.children = original.children - m.parents = original.parents - } - if m.children == nil { - m.children = make(map[packageID]*metadata) - } - if m.parents == nil { - m.parents = make(map[packageID]bool) - } - - // Add the metadata to the cache. - v.mcache.packages[m.id] = m - - // Connect the import graph. - if g.parent != nil { - m.parents[g.parent.id] = true - g.parent.children[m.id] = m - } - for importPath, importPkg := range g.pkg.Imports { - importPkgPath := packagePath(importPath) - if importPkgPath == g.pkgPath { - return fmt.Errorf("cycle detected in %s", importPath) - } - // Don't remember any imports with significant errors. - if importPkgPath != "unsafe" && len(importPkg.CompiledGoFiles) == 0 { - g.missingImports[importPkgPath] = struct{}{} - continue - } - if _, ok := m.children[packageID(importPkg.ID)]; !ok { - if err := v.link(ctx, &importGraph{ - pkgPath: importPkgPath, - pkg: importPkg, - parent: m, - missingImports: g.missingImports, - }); err != nil { - log.Error(ctx, "error in dependency", err) - } - } - } - // Clear out any imports that have been removed since the package was last loaded. - for importID := range m.children { - child, ok := v.mcache.packages[importID] - if !ok { - continue - } - importPath := string(child.pkgPath) - if _, ok := g.pkg.Imports[importPath]; ok { - continue - } - delete(m.children, importID) - delete(child.parents, m.id) - } - return nil + return false } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 34d89d5472..6cd4a61f54 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -98,8 +98,10 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI, opt folder: folder, filesByURI: make(map[span.URI]viewFile), filesByBase: make(map[string][]viewFile), - mcache: &metadataCache{ - packages: make(map[packageID]*metadata), + snapshot: &snapshot{ + packages: make(map[span.URI]map[packageKey]*checkPackageHandle), + ids: make(map[span.URI][]packageID), + metadata: make(map[packageID]*metadata), }, ignoredURIs: make(map[span.URI]struct{}), builtin: &builtinPkg{}, @@ -144,6 +146,19 @@ func (s *session) ViewOf(uri span.URI) source.View { return v } +func (s *session) viewsOf(uri span.URI) []*view { + s.viewMu.Lock() + defer s.viewMu.Unlock() + + var views []*view + for _, view := range s.views { + if strings.HasPrefix(string(uri), string(view.Folder())) { + views = append(views, view) + } + } + return views +} + func (s *session) Views() []source.View { s.viewMu.Lock() defer s.viewMu.Unlock() @@ -219,21 +234,7 @@ func (s *session) DidOpen(ctx context.Context, uri span.URI, kind source.FileKin // A file may be in multiple views. for _, view := range s.views { if strings.HasPrefix(string(uri), string(view.Folder())) { - f, err := view.GetFile(ctx, uri) - if err != nil { - log.Error(ctx, "error getting file", nil, telemetry.File) - return - } - gof, ok := f.(*goFile) - if !ok { - log.Error(ctx, "not a Go file", nil, telemetry.File) - return - } - // Force a reload of the package metadata by clearing the cached data. - gof.mu.Lock() - gof.meta = make(map[packageID]*metadata) - gof.cphs = make(map[packageKey]*checkPackageHandle) - gof.mu.Unlock() + view.invalidateMetadata(uri) } } } @@ -341,15 +342,16 @@ func (s *session) buildOverlay() map[string][]byte { return overlays } -func (s *session) DidChangeOutOfBand(ctx context.Context, f source.GoFile, changeType protocol.FileChangeType) { +func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, changeType protocol.FileChangeType) { if changeType == protocol.Deleted { // After a deletion we must invalidate the package's metadata to - // force a go/packages invocation to refresh the package's file - // list. - f.(*goFile).invalidateMeta(ctx) + // force a go/packages invocation to refresh the package's file list. + views := s.viewsOf(uri) + for _, v := range views { + v.invalidateMetadata(uri) + } } - - s.filesWatchMap.Notify(f.URI()) + s.filesWatchMap.Notify(uri) } func (o *overlay) FileSystem() source.FileSystem { diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go new file mode 100644 index 0000000000..38312ec4f9 --- /dev/null +++ b/internal/lsp/cache/snapshot.go @@ -0,0 +1,309 @@ +package cache + +import ( + "context" + "go/types" + + "golang.org/x/tools/go/packages" + "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/telemetry/log" + "golang.org/x/tools/internal/telemetry/tag" + errors "golang.org/x/xerrors" +) + +type snapshot struct { + id uint64 + + packages map[span.URI]map[packageKey]*checkPackageHandle + ids map[span.URI][]packageID + metadata map[packageID]*metadata +} + +type metadata struct { + id packageID + pkgPath packagePath + name string + files []span.URI + typesSizes types.Sizes + parents map[packageID]bool + children map[packageID]*metadata + errors []packages.Error +} + +func (v *view) getSnapshot(uri span.URI) ([]*metadata, map[packageKey]*checkPackageHandle) { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + var m []*metadata + for _, id := range v.snapshot.ids[uri] { + m = append(m, v.snapshot.metadata[id]) + } + return m, v.snapshot.packages[uri] +} + +func (v *view) getMetadata(uri span.URI) []*metadata { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + var m []*metadata + for _, id := range v.snapshot.ids[uri] { + m = append(m, v.snapshot.metadata[id]) + } + return m +} + +func (v *view) getPackages(uri span.URI) map[packageKey]*checkPackageHandle { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + return v.snapshot.packages[uri] +} + +func (v *view) updateMetadata(ctx context.Context, uri span.URI, pkgs []*packages.Package) ([]*metadata, error) { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + // Clear metadata since we are re-running go/packages. + without := make(map[span.URI]struct{}) + for _, id := range v.snapshot.ids[uri] { + v.remove(id, without, map[packageID]struct{}{}) + } + v.snapshot = v.snapshot.cloneMetadata(without) + + var results []*metadata + for _, pkg := range pkgs { + log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles)) + + // Build the import graph for this package. + if err := v.updateImportGraph(ctx, &importGraph{ + pkgPath: packagePath(pkg.PkgPath), + pkg: pkg, + parent: nil, + }); err != nil { + return nil, err + } + results = append(results, v.snapshot.metadata[packageID(pkg.ID)]) + } + return results, nil +} + +type importGraph struct { + pkgPath packagePath + pkg *packages.Package + parent *metadata +} + +func (v *view) updateImportGraph(ctx context.Context, g *importGraph) error { + // Recreate the metadata rather than reusing it to avoid locking. + m := &metadata{ + id: packageID(g.pkg.ID), + pkgPath: g.pkgPath, + name: g.pkg.Name, + typesSizes: g.pkg.TypesSizes, + errors: g.pkg.Errors, + } + for _, filename := range g.pkg.CompiledGoFiles { + uri := span.FileURI(filename) + v.snapshot.ids[uri] = append(v.snapshot.ids[uri], m.id) + m.files = append(m.files, uri) + } + // Preserve the import graph. + if original, ok := v.snapshot.metadata[m.id]; ok { + m.children = original.children + m.parents = original.parents + } + if m.children == nil { + m.children = make(map[packageID]*metadata) + } + if m.parents == nil { + m.parents = make(map[packageID]bool) + } + + // Add the metadata to the cache. + v.snapshot.metadata[m.id] = m + + // Connect the import graph. + if g.parent != nil { + m.parents[g.parent.id] = true + g.parent.children[m.id] = m + } + for importPath, importPkg := range g.pkg.Imports { + importPkgPath := packagePath(importPath) + if importPkgPath == g.pkgPath { + return errors.Errorf("cycle detected in %s", importPath) + } + // Don't remember any imports with significant errors. + if importPkgPath != "unsafe" && len(importPkg.CompiledGoFiles) == 0 { + continue + } + if _, ok := m.children[packageID(importPkg.ID)]; !ok { + if err := v.updateImportGraph(ctx, &importGraph{ + pkgPath: importPkgPath, + pkg: importPkg, + parent: m, + }); err != nil { + log.Error(ctx, "error in dependency", err) + } + } + } + // Clear out any imports that have been removed since the package was last loaded. + for importID := range m.children { + child, ok := v.snapshot.metadata[importID] + if !ok { + continue + } + importPath := string(child.pkgPath) + if _, ok := g.pkg.Imports[importPath]; ok { + continue + } + delete(m.children, importID) + delete(child.parents, m.id) + } + return nil +} + +func (v *view) updatePackages(cphs []*checkPackageHandle) { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + for _, cph := range cphs { + for _, ph := range cph.files { + uri := ph.File().Identity().URI + if _, ok := v.snapshot.packages[uri]; !ok { + v.snapshot.packages[uri] = make(map[packageKey]*checkPackageHandle) + } + v.snapshot.packages[uri][packageKey{ + id: cph.m.id, + mode: ph.Mode(), + }] = cph + } + } +} + +// invalidateContent invalidates the content of a Go file, +// including any position and type information that depends on it. +func (v *view) invalidateContent(ctx context.Context, f *goFile) { + f.handleMu.Lock() + defer f.handleMu.Unlock() + + without := make(map[span.URI]struct{}) + + // Remove the package and all of its reverse dependencies from the cache. + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + for _, id := range v.snapshot.ids[f.URI()] { + f.view.remove(id, without, map[packageID]struct{}{}) + } + v.snapshot = v.snapshot.clonePackages(without) + f.handle = nil +} + +// invalidateMeta invalidates package metadata for all files in f's +// package. This forces f's package's metadata to be reloaded next +// time the package is checked. +func (v *view) invalidateMetadata(uri span.URI) { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + without := make(map[span.URI]struct{}) + + for _, id := range v.snapshot.ids[uri] { + v.remove(id, without, map[packageID]struct{}{}) + } + v.snapshot = v.snapshot.cloneMetadata(without) +} + +// remove invalidates a package and its reverse dependencies in the view's +// package cache. It is assumed that the caller has locked both the mutexes +// of both the mcache and the pcache. +func (v *view) remove(id packageID, toDelete map[span.URI]struct{}, seen map[packageID]struct{}) { + if _, ok := seen[id]; ok { + return + } + m, ok := v.snapshot.metadata[id] + if !ok { + return + } + seen[id] = struct{}{} + for parentID := range m.parents { + v.remove(parentID, toDelete, seen) + } + for _, uri := range m.files { + toDelete[uri] = struct{}{} + } +} + +func (s *snapshot) clonePackages(without map[span.URI]struct{}) *snapshot { + result := &snapshot{ + id: s.id + 1, + packages: make(map[span.URI]map[packageKey]*checkPackageHandle), + ids: s.ids, + metadata: s.metadata, + } + for k, v := range s.packages { + if _, ok := without[k]; ok { + continue + } + result.packages[k] = v + } + return result +} + +func (s *snapshot) cloneMetadata(without map[span.URI]struct{}) *snapshot { + result := &snapshot{ + id: s.id + 1, + packages: s.packages, + ids: make(map[span.URI][]packageID), + metadata: make(map[packageID]*metadata), + } + withoutIDs := make(map[packageID]struct{}) + for k, ids := range s.ids { + if _, ok := without[k]; ok { + for _, id := range ids { + withoutIDs[id] = struct{}{} + } + continue + } + result.ids[k] = ids + } + for k, v := range s.metadata { + if _, ok := withoutIDs[k]; ok { + continue + } + result.metadata[k] = v + } + return result +} + +func (v *view) reverseDependencies(ctx context.Context, uri span.URI) map[span.URI]struct{} { + seen := make(map[packageID]struct{}) + uris := make(map[span.URI]struct{}) + + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + for _, id := range v.snapshot.ids[uri] { + v.rdeps(id, seen, uris, id) + } + return uris +} + +func (v *view) rdeps(topID packageID, seen map[packageID]struct{}, results map[span.URI]struct{}, id packageID) { + if _, ok := seen[id]; ok { + return + } + seen[id] = struct{}{} + m, ok := v.snapshot.metadata[id] + if !ok { + return + } + if id != topID { + for _, uri := range m.files { + results[uri] = struct{}{} + } + } + for parentID := range m.parents { + v.rdeps(topID, seen, results, parentID) + } +} diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 1b6bccb166..34cebd4d6d 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -10,7 +10,6 @@ import ( "fmt" "go/ast" "go/token" - "go/types" "os" "os/exec" "strings" @@ -21,7 +20,6 @@ import ( "golang.org/x/tools/internal/imports" "golang.org/x/tools/internal/lsp/debug" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/lsp/telemetry" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" errors "golang.org/x/xerrors" @@ -72,8 +70,8 @@ type view struct { filesByURI map[span.URI]viewFile filesByBase map[string][]viewFile - // mcache caches metadata for the packages of the opened files in a view. - mcache *metadataCache + snapshotMu sync.Mutex + snapshot *snapshot // builtin is used to resolve builtin types. builtin *builtinPkg @@ -85,23 +83,6 @@ type view struct { analyzers []*analysis.Analyzer } -type metadataCache struct { - mu sync.Mutex // guards both maps - packages map[packageID]*metadata -} - -type metadata struct { - id packageID - pkgPath packagePath - name string - files []span.URI - key string - typesSizes types.Sizes - parents map[packageID]bool - children map[packageID]*metadata - errors []packages.Error -} - func (v *view) Session() source.Session { return v.session } @@ -324,102 +305,6 @@ func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) (bo return false, nil } -// invalidateContent invalidates the content of a Go file, -// including any position and type information that depends on it. -func (f *goFile) invalidateContent(ctx context.Context) { - // Mutex acquisition order here is important. It must match the order - // in loadParseTypecheck to avoid deadlocks. - f.view.mcache.mu.Lock() - defer f.view.mcache.mu.Unlock() - - toDelete := make(map[packageID]bool) - f.mu.Lock() - for key, cph := range f.cphs { - if cph != nil { - toDelete[key.id] = true - } - } - f.mu.Unlock() - - f.handleMu.Lock() - defer f.handleMu.Unlock() - - // Remove the package and all of its reverse dependencies from the cache. - for id := range toDelete { - f.view.remove(ctx, id, map[packageID]struct{}{}) - } - - f.handle = nil -} - -// invalidateMeta invalidates package metadata for all files in f's -// package. This forces f's package's metadata to be reloaded next -// time the package is checked. -func (f *goFile) invalidateMeta(ctx context.Context) { - cphs, err := f.CheckPackageHandles(ctx) - if err != nil { - log.Error(ctx, "invalidateMeta: GetPackages", err, telemetry.File.Of(f.URI())) - return - } - - for _, pkg := range cphs { - for _, pgh := range pkg.Files() { - uri := pgh.File().Identity().URI - if gof, _ := f.view.FindFile(ctx, uri).(*goFile); gof != nil { - gof.mu.Lock() - gof.meta = nil - gof.mu.Unlock() - } - } - f.view.mcache.mu.Lock() - delete(f.view.mcache.packages, packageID(pkg.ID())) - f.view.mcache.mu.Unlock() - } -} - -// remove invalidates a package and its reverse dependencies in the view's -// package cache. It is assumed that the caller has locked both the mutexes -// of both the mcache and the pcache. -func (v *view) remove(ctx context.Context, id packageID, seen map[packageID]struct{}) { - if _, ok := seen[id]; ok { - return - } - m, ok := v.mcache.packages[id] - if !ok { - return - } - seen[id] = struct{}{} - for parentID := range m.parents { - v.remove(ctx, parentID, seen) - } - // All of the files in the package may also be holding a pointer to the - // invalidated package. - for _, uri := range m.files { - f, err := v.findFile(uri) - if err != nil { - log.Error(ctx, "cannot find file", err, telemetry.File.Of(f.URI())) - continue - } - gof, ok := f.(*goFile) - if !ok { - log.Error(ctx, "non-Go file", nil, telemetry.File.Of(f.URI())) - continue - } - gof.mu.Lock() - for _, mode := range []source.ParseMode{ - source.ParseExported, - source.ParseFull, - } { - delete(gof.cphs, packageKey{ - id: id, - mode: mode, - }) - } - gof.mu.Unlock() - } - return -} - // FindFile returns the file if the given URI is already a part of the view. func (v *view) FindFile(ctx context.Context, uri span.URI) source.File { v.mu.Lock() @@ -481,7 +366,7 @@ func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) if !ok { return } - gof.invalidateContent(ctx) + v.invalidateContent(ctx, gof) }) } v.mapFile(uri, f) @@ -540,6 +425,19 @@ func (v *view) mapFile(uri span.URI, f viewFile) { } } +func (v *view) openFiles(ctx context.Context, uris map[span.URI]struct{}) (results []source.File) { + v.mu.Lock() + defer v.mu.Unlock() + + for uri := range uris { + // Call unlocked version of getFile since we hold the lock on the view. + if f, err := v.getFile(ctx, uri, source.Go); err == nil && v.session.IsOpen(uri) { + results = append(results, f) + } + } + return results +} + type debugView struct{ *view } func (v debugView) ID() string { return v.id } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index c48a2ccf38..1278131082 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -84,13 +84,8 @@ func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[ } } // Updates to the diagnostics for this package may need to be propagated. - revDeps := f.GetActiveReverseDeps(ctx) - for _, f := range revDeps { - cphs, err := f.CheckPackageHandles(ctx) - if err != nil { - return nil, "", err - } - cph := WidestCheckPackageHandle(cphs) + revDeps := view.GetActiveReverseDeps(ctx, f.URI()) + for _, cph := range revDeps { pkg, err := cph.Check(ctx) if err != nil { return nil, warningMsg, err diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index aa8a872313..20191866e6 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -192,7 +192,7 @@ type Session interface { // DidChangeOutOfBand is called when a file under the root folder // changes. The file is not necessarily open in the editor. - DidChangeOutOfBand(ctx context.Context, f GoFile, change protocol.FileChangeType) + DidChangeOutOfBand(ctx context.Context, uri span.URI, change protocol.FileChangeType) // Options returns a copy of the SessionOptions for this session. Options() Options @@ -254,6 +254,10 @@ type View interface { // Analyzers returns the set of Analyzers active for this view. Analyzers() []*analysis.Analyzer + + // GetActiveReverseDeps returns the active files belonging to the reverse + // dependencies of this file's package. + GetActiveReverseDeps(ctx context.Context, uri span.URI) []CheckPackageHandle } // File represents a source file of any type. @@ -270,10 +274,6 @@ type GoFile interface { // GetCheckPackageHandles returns the CheckPackageHandles for the packages // that this file belongs to. CheckPackageHandles(ctx context.Context) ([]CheckPackageHandle, error) - - // GetActiveReverseDeps returns the active files belonging to the reverse - // dependencies of this file's package. - GetActiveReverseDeps(ctx context.Context) []GoFile } type ModFile interface { diff --git a/internal/lsp/watched_files.go b/internal/lsp/watched_files.go index 5a700296c6..281b7ab796 100644 --- a/internal/lsp/watched_files.go +++ b/internal/lsp/watched_files.go @@ -44,7 +44,7 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did case protocol.Changed: log.Print(ctx, "watched file changed", telemetry.File) - s.session.DidChangeOutOfBand(ctx, gof, change.Type) + s.session.DidChangeOutOfBand(ctx, uri, change.Type) // Refresh diagnostics to reflect updated file contents. go s.diagnostics(view, uri) @@ -77,7 +77,7 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did break } } - s.session.DidChangeOutOfBand(ctx, gof, change.Type) + s.session.DidChangeOutOfBand(ctx, uri, change.Type) // If this was the only file in the package, clear its diagnostics. if otherFile == nil {