Skip to content

Commit 4a8620f

Browse files
findleyrfindleyr
committed
internal/lsp/cache: move metadata fields to a new metadataGraph type
In preparation for making metadata immutable, move metadata-related fields to a new MetadataGraph type. Other than instantiating this type when cloning, this CL contains no functional changes. For golang/go#45686 Change-Id: I7ad29d1f331ba7e53dad3f012ad7ecdae4f7d4b7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/340730 Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent a3d129c commit 4a8620f

File tree

4 files changed

+77
-56
lines changed

4 files changed

+77
-56
lines changed

‎internal/lsp/cache/graph.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright 2022 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package cache
6+
7+
import "golang.org/x/tools/internal/span"
8+
9+
// A metadataGraph holds information about a transtively closed import graph of
10+
// Go packages, as obtained from go/packages.
11+
//
12+
// Currently a new metadata graph is created for each snapshot.
13+
// TODO(rfindley): make this type immutable, so that it may be shared across
14+
// snapshots.
15+
type metadataGraph struct {
16+
// ids maps file URIs to package IDs. A single file may belong to multiple
17+
// packages due to tests packages.
18+
ids map[span.URI][]PackageID
19+
20+
// metadata maps package IDs to their associated metadata.
21+
metadata map[PackageID]*KnownMetadata
22+
23+
// importedBy maps package IDs to the list of packages that import them.
24+
importedBy map[PackageID][]PackageID
25+
}
26+
27+
func NewMetadataGraph() *metadataGraph {
28+
return &metadataGraph{
29+
ids: make(map[span.URI][]PackageID),
30+
metadata: make(map[PackageID]*KnownMetadata),
31+
importedBy: make(map[PackageID][]PackageID),
32+
}
33+
}

‎internal/lsp/cache/load.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,12 +495,12 @@ func (s *snapshot) setMetadataLocked(ctx context.Context, pkgPath PackagePath, p
495495
// Add the metadata to the cache.
496496

497497
// If we've already set the metadata for this snapshot, reuse it.
498-
if original, ok := s.metadata[m.ID]; ok && original.Valid {
498+
if original, ok := s.meta.metadata[m.ID]; ok && original.Valid {
499499
// Since we've just reloaded, clear out shouldLoad.
500500
original.ShouldLoad = false
501501
m = original.Metadata
502502
} else {
503-
s.metadata[m.ID] = &KnownMetadata{
503+
s.meta.metadata[m.ID] = &KnownMetadata{
504504
Metadata: m,
505505
Valid: true,
506506
}

‎internal/lsp/cache/session.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,12 +232,10 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
232232
initializeOnce: &sync.Once{},
233233
generation: s.cache.store.Generation(generationName(v, 0)),
234234
packages: make(map[packageKey]*packageHandle),
235-
ids: make(map[span.URI][]PackageID),
236-
metadata: make(map[PackageID]*KnownMetadata),
235+
meta: NewMetadataGraph(),
237236
files: make(map[span.URI]source.VersionedFileHandle),
238237
goFiles: make(map[parseKey]*parseGoHandle),
239238
symbols: make(map[span.URI]*symbolHandle),
240-
importedBy: make(map[PackageID][]PackageID),
241239
actions: make(map[actionKey]*actionHandle),
242240
workspacePackages: make(map[PackageID]PackagePath),
243241
unloadableFiles: make(map[span.URI]struct{}),

‎internal/lsp/cache/snapshot.go‎

Lines changed: 41 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,8 @@ type snapshot struct {
6868
// builtin pins the AST and package for builtin.go in memory.
6969
builtin span.URI
7070

71-
// ids maps file URIs to package IDs.
72-
// It may be invalidated on calls to go/packages.
73-
ids map[span.URI][]PackageID
74-
75-
// metadata maps file IDs to their associated metadata.
76-
// It may invalidated on calls to go/packages.
77-
metadata map[PackageID]*KnownMetadata
78-
79-
// importedBy maps package IDs to the list of packages that import them.
80-
importedBy map[PackageID][]PackageID
71+
// meta holds loaded metadata.
72+
meta *metadataGraph
8173

8274
// files maps file URIs to their corresponding FileHandles.
8375
// It may invalidated when a file's content changes.
@@ -716,25 +708,25 @@ func (s *snapshot) getImportedBy(id PackageID) []PackageID {
716708

717709
func (s *snapshot) getImportedByLocked(id PackageID) []PackageID {
718710
// If we haven't rebuilt the import graph since creating the snapshot.
719-
if len(s.importedBy) == 0 {
711+
if len(s.meta.importedBy) == 0 {
720712
s.rebuildImportGraph()
721713
}
722-
return s.importedBy[id]
714+
return s.meta.importedBy[id]
723715
}
724716

725717
func (s *snapshot) clearAndRebuildImportGraph() {
726718
s.mu.Lock()
727719
defer s.mu.Unlock()
728720

729721
// Completely invalidate the original map.
730-
s.importedBy = make(map[PackageID][]PackageID)
722+
s.meta.importedBy = make(map[PackageID][]PackageID)
731723
s.rebuildImportGraph()
732724
}
733725

734726
func (s *snapshot) rebuildImportGraph() {
735-
for id, m := range s.metadata {
727+
for id, m := range s.meta.metadata {
736728
for _, importID := range m.Deps {
737-
s.importedBy[importID] = append(s.importedBy[importID], id)
729+
s.meta.importedBy[importID] = append(s.meta.importedBy[importID], id)
738730
}
739731
}
740732
}
@@ -789,7 +781,7 @@ func (s *snapshot) isActiveLocked(id PackageID, seen map[PackageID]bool) (active
789781
defer func() {
790782
seen[id] = active
791783
}()
792-
m, ok := s.metadata[id]
784+
m, ok := s.meta.metadata[id]
793785
if !ok {
794786
return false
795787
}
@@ -1045,7 +1037,7 @@ func (s *snapshot) KnownPackages(ctx context.Context) ([]source.Package, error)
10451037
// workspace packages first.
10461038
ids := s.workspacePackageIDs()
10471039
s.mu.Lock()
1048-
for id := range s.metadata {
1040+
for id := range s.meta.metadata {
10491041
if _, ok := s.workspacePackages[id]; ok {
10501042
continue
10511043
}
@@ -1177,14 +1169,14 @@ func (s *snapshot) getIDsForURI(uri span.URI) []PackageID {
11771169
s.mu.Lock()
11781170
defer s.mu.Unlock()
11791171

1180-
return s.ids[uri]
1172+
return s.meta.ids[uri]
11811173
}
11821174

11831175
func (s *snapshot) getMetadata(id PackageID) *KnownMetadata {
11841176
s.mu.Lock()
11851177
defer s.mu.Unlock()
11861178

1187-
return s.metadata[id]
1179+
return s.meta.metadata[id]
11881180
}
11891181

11901182
func (s *snapshot) shouldLoad(scope interface{}) bool {
@@ -1194,7 +1186,7 @@ func (s *snapshot) shouldLoad(scope interface{}) bool {
11941186
switch scope := scope.(type) {
11951187
case PackagePath:
11961188
var meta *KnownMetadata
1197-
for _, m := range s.metadata {
1189+
for _, m := range s.meta.metadata {
11981190
if m.PkgPath != scope {
11991191
continue
12001192
}
@@ -1206,12 +1198,12 @@ func (s *snapshot) shouldLoad(scope interface{}) bool {
12061198
return false
12071199
case fileURI:
12081200
uri := span.URI(scope)
1209-
ids := s.ids[uri]
1201+
ids := s.meta.ids[uri]
12101202
if len(ids) == 0 {
12111203
return true
12121204
}
12131205
for _, id := range ids {
1214-
m, ok := s.metadata[id]
1206+
m, ok := s.meta.metadata[id]
12151207
if !ok || m.ShouldLoad {
12161208
return true
12171209
}
@@ -1229,7 +1221,7 @@ func (s *snapshot) clearShouldLoad(scope interface{}) {
12291221
switch scope := scope.(type) {
12301222
case PackagePath:
12311223
var meta *KnownMetadata
1232-
for _, m := range s.metadata {
1224+
for _, m := range s.meta.metadata {
12331225
if m.PkgPath == scope {
12341226
meta = m
12351227
}
@@ -1240,12 +1232,12 @@ func (s *snapshot) clearShouldLoad(scope interface{}) {
12401232
meta.ShouldLoad = false
12411233
case fileURI:
12421234
uri := span.URI(scope)
1243-
ids := s.ids[uri]
1235+
ids := s.meta.ids[uri]
12441236
if len(ids) == 0 {
12451237
return
12461238
}
12471239
for _, id := range ids {
1248-
if m, ok := s.metadata[id]; ok {
1240+
if m, ok := s.meta.metadata[id]; ok {
12491241
m.ShouldLoad = false
12501242
}
12511243
}
@@ -1255,12 +1247,12 @@ func (s *snapshot) clearShouldLoad(scope interface{}) {
12551247
// noValidMetadataForURILocked reports whether there is any valid metadata for
12561248
// the given URI.
12571249
func (s *snapshot) noValidMetadataForURILocked(uri span.URI) bool {
1258-
ids, ok := s.ids[uri]
1250+
ids, ok := s.meta.ids[uri]
12591251
if !ok {
12601252
return true
12611253
}
12621254
for _, id := range ids {
1263-
if m, ok := s.metadata[id]; ok && m.Valid {
1255+
if m, ok := s.meta.metadata[id]; ok && m.Valid {
12641256
return false
12651257
}
12661258
}
@@ -1276,7 +1268,7 @@ func (s *snapshot) noValidMetadataForID(id PackageID) bool {
12761268
}
12771269

12781270
func (s *snapshot) noValidMetadataForIDLocked(id PackageID) bool {
1279-
m := s.metadata[id]
1271+
m := s.meta.metadata[id]
12801272
return m == nil || !m.Valid
12811273
}
12821274

@@ -1289,7 +1281,7 @@ func (s *snapshot) updateIDForURIsLocked(id PackageID, uris map[span.URI]struct{
12891281
for uri := range uris {
12901282
// Collect the new set of IDs, preserving any valid existing IDs.
12911283
newIDs := []PackageID{id}
1292-
for _, existingID := range s.ids[uri] {
1284+
for _, existingID := range s.meta.ids[uri] {
12931285
// Don't set duplicates of the same ID.
12941286
if existingID == id {
12951287
continue
@@ -1302,15 +1294,15 @@ func (s *snapshot) updateIDForURIsLocked(id PackageID, uris map[span.URI]struct{
13021294
}
13031295
// If the metadata for an existing ID is invalid, and we are
13041296
// setting metadata for a new, valid ID--don't preserve the old ID.
1305-
if m, ok := s.metadata[existingID]; !ok || !m.Valid {
1297+
if m, ok := s.meta.metadata[existingID]; !ok || !m.Valid {
13061298
continue
13071299
}
13081300
newIDs = append(newIDs, existingID)
13091301
}
13101302
sort.Slice(newIDs, func(i, j int) bool {
13111303
return newIDs[i] < newIDs[j]
13121304
})
1313-
s.ids[uri] = newIDs
1305+
s.meta.ids[uri] = newIDs
13141306
}
13151307
}
13161308

@@ -1396,10 +1388,10 @@ func (s *snapshot) awaitLoaded(ctx context.Context) error {
13961388

13971389
// If we still have absolutely no metadata, check if the view failed to
13981390
// initialize and return any errors.
1399-
if s.useInvalidMetadata() && len(s.metadata) > 0 {
1391+
if s.useInvalidMetadata() && len(s.meta.metadata) > 0 {
14001392
return nil
14011393
}
1402-
for _, m := range s.metadata {
1394+
for _, m := range s.meta.metadata {
14031395
if m.Valid {
14041396
return nil
14051397
}
@@ -1523,10 +1515,10 @@ func (s *snapshot) AwaitInitialized(ctx context.Context) {
15231515
func (s *snapshot) reloadWorkspace(ctx context.Context) error {
15241516
// See which of the workspace packages are missing metadata.
15251517
s.mu.Lock()
1526-
missingMetadata := len(s.workspacePackages) == 0 || len(s.metadata) == 0
1518+
missingMetadata := len(s.workspacePackages) == 0 || len(s.meta.metadata) == 0
15271519
pkgPathSet := map[PackagePath]struct{}{}
15281520
for id, pkgPath := range s.workspacePackages {
1529-
if m, ok := s.metadata[id]; ok && m.Valid {
1521+
if m, ok := s.meta.metadata[id]; ok && m.Valid {
15301522
continue
15311523
}
15321524
missingMetadata = true
@@ -1670,10 +1662,10 @@ func checkSnapshotLocked(ctx context.Context, s *snapshot) {
16701662
// Check that every go file for a workspace package is identified as
16711663
// belonging to that workspace package.
16721664
for wsID := range s.workspacePackages {
1673-
if m, ok := s.metadata[wsID]; ok {
1665+
if m, ok := s.meta.metadata[wsID]; ok {
16741666
for _, uri := range m.GoFiles {
16751667
found := false
1676-
for _, id := range s.ids[uri] {
1668+
for _, id := range s.meta.ids[uri] {
16771669
if id == wsID {
16781670
found = true
16791671
break
@@ -1723,9 +1715,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
17231715
builtin: s.builtin,
17241716
initializeOnce: s.initializeOnce,
17251717
initializedErr: s.initializedErr,
1726-
ids: make(map[span.URI][]PackageID, len(s.ids)),
1727-
importedBy: make(map[PackageID][]PackageID, len(s.importedBy)),
1728-
metadata: make(map[PackageID]*KnownMetadata, len(s.metadata)),
1718+
meta: NewMetadataGraph(),
17291719
packages: make(map[packageKey]*packageHandle, len(s.packages)),
17301720
actions: make(map[actionKey]*actionHandle, len(s.actions)),
17311721
files: make(map[span.URI]source.VersionedFileHandle, len(s.files)),
@@ -1813,7 +1803,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
18131803

18141804
// Invalidate all package metadata if the workspace module has changed.
18151805
if workspaceReload {
1816-
for k := range s.metadata {
1806+
for k := range s.meta.metadata {
18171807
directIDs[k] = true
18181808
}
18191809
}
@@ -1843,7 +1833,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
18431833
anyImportDeleted = anyImportDeleted || importDeleted
18441834

18451835
// Mark all of the package IDs containing the given file.
1846-
filePackageIDs := invalidatedPackageIDs(uri, s.ids, pkgFileChanged)
1836+
filePackageIDs := invalidatedPackageIDs(uri, s.meta.ids, pkgFileChanged)
18471837
if pkgFileChanged {
18481838
for id := range filePackageIDs {
18491839
changedPkgFiles[id] = struct{}{}
@@ -1892,7 +1882,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
18921882
// from an unparseable state to a parseable state, as we don't have a
18931883
// starting point to compare with.
18941884
if anyImportDeleted {
1895-
for id, metadata := range s.metadata {
1885+
for id, metadata := range s.meta.metadata {
18961886
if len(metadata.Errors) > 0 {
18971887
directIDs[id] = true
18981888
}
@@ -1952,7 +1942,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
19521942
continue
19531943
}
19541944
// The file has been deleted.
1955-
if ids, ok := s.ids[c.fileHandle.URI()]; ok {
1945+
if ids, ok := s.meta.ids[c.fileHandle.URI()]; ok {
19561946
for _, id := range ids {
19571947
skipID[id] = true
19581948
}
@@ -1968,7 +1958,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
19681958
return
19691959
}
19701960
reachableID[id] = true
1971-
m, ok := s.metadata[id]
1961+
m, ok := s.meta.metadata[id]
19721962
if !ok {
19731963
return
19741964
}
@@ -1984,7 +1974,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
19841974
// metadata will be reloaded in future calls to load.
19851975
deleteInvalidMetadata := forceReloadMetadata || workspaceModeChanged
19861976
idsInSnapshot := map[PackageID]bool{} // track all known IDs
1987-
for uri, ids := range s.ids {
1977+
for uri, ids := range s.meta.ids {
19881978
var resultIDs []PackageID
19891979
for _, id := range ids {
19901980
if skipID[id] || deleteInvalidMetadata && idsToInvalidate[id] {
@@ -1998,20 +1988,20 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
19981988
idsInSnapshot[id] = true
19991989
resultIDs = append(resultIDs, id)
20001990
}
2001-
result.ids[uri] = resultIDs
1991+
result.meta.ids[uri] = resultIDs
20021992
}
20031993

20041994
// Copy the package metadata. We only need to invalidate packages directly
20051995
// containing the affected file, and only if it changed in a relevant way.
2006-
for k, v := range s.metadata {
1996+
for k, v := range s.meta.metadata {
20071997
if !idsInSnapshot[k] {
20081998
// Delete metadata for IDs that are no longer reachable from files
20091999
// in the snapshot.
20102000
continue
20112001
}
20122002
invalidateMetadata := idsToInvalidate[k]
20132003
// Mark invalidated metadata rather than deleting it outright.
2014-
result.metadata[k] = &KnownMetadata{
2004+
result.meta.metadata[k] = &KnownMetadata{
20152005
Metadata: v.Metadata,
20162006
Valid: v.Valid && !invalidateMetadata,
20172007
ShouldLoad: v.ShouldLoad || invalidateMetadata,
@@ -2032,9 +2022,9 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
20322022

20332023
// If all the files we know about in a package have been deleted,
20342024
// the package is gone and we should no longer try to load it.
2035-
if m := s.metadata[id]; m != nil {
2025+
if m := s.meta.metadata[id]; m != nil {
20362026
hasFiles := false
2037-
for _, uri := range s.metadata[id].GoFiles {
2027+
for _, uri := range s.meta.metadata[id].GoFiles {
20382028
// For internal tests, we need _test files, not just the normal
20392029
// ones. External tests only have _test files, but we can check
20402030
// them anyway.

0 commit comments

Comments
 (0)