Skip to content

Commit f10e7d5

Browse files
findleyrfindleyr
committed
gopls/internal/lsp/cache: remove package dependence on packages.Config
The cache.pkg type was a mix of metadata-related information and type checking information, resulting in unnecessary relationships between type-checking results (which are shared) and loading results (which are not shared). As a result, the experimentalPackageCacheKey was more or less a hope that these relationships were valid. Avoid this relationship altogether by separating the shared type-checking result from other derived calculations. This makes the experimentalPackageCacheKey obsolete and lays the groundwork for type-checking from export data. Additionally: - revisit the package cache key to ensure it covers all inputs into type-checking, and make it more similar to the analysis key - remove methods from the source.Package API that return source.Package: we can't have edges between packages if they are going to be standalone - remove the experimentalPackageCacheKey setting - add a test for go list errors - use the proper types.Sizes when type-checking - address a comment from an earlier CL in completion_test.go Fixes golang/go#57853 Change-Id: I238913c7c8305cb534db77ebec5f062e96ed2503 Reviewed-on: https://go-review.googlesource.com/c/tools/+/461944 Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
1 parent 8270757 commit f10e7d5

38 files changed

+711
-511
lines changed

‎gopls/doc/settings.md

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -116,20 +116,6 @@ a go.mod file, narrowing the scope to that directory if it exists.
116116

117117
Default: `true`.
118118

119-
#### **experimentalPackageCacheKey** *bool*
120-
121-
**This setting is experimental and may be deleted.**
122-
123-
experimentalPackageCacheKey controls whether to use a coarser cache key
124-
for package type information to increase cache hits. This setting removes
125-
the user's environment, build flags, and working directory from the cache
126-
key, which should be a safe change as all relevant inputs into the type
127-
checking pass are already hashed into the key. This is temporarily guarded
128-
by an experiment because caching behavior is subtle and difficult to
129-
comprehensively test.
130-
131-
Default: `true`.
132-
133119
#### **allowModfileModifications** *bool*
134120

135121
**This setting is experimental and may be deleted.**

‎gopls/internal/lsp/cache/analysis.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -520,10 +520,8 @@ func analysisCacheKey(analyzers []*analysis.Analyzer, m *source.Metadata, compil
520520
sz := m.TypesSizes.(*types.StdSizes)
521521
fmt.Fprintf(hasher, "sizes: %d %d\n", sz.WordSize, sz.MaxAlign)
522522

523-
// metadata errors
524-
for _, err := range m.Errors {
525-
fmt.Fprintf(hasher, "error: %q", err)
526-
}
523+
// metadata errors: used for 'compiles' field
524+
fmt.Fprintf(hasher, "errors: %d", len(m.Errors))
527525

528526
// module Go version
529527
if m.Module != nil && m.Module.GoVersion != "" {
@@ -542,8 +540,8 @@ func analysisCacheKey(analyzers []*analysis.Analyzer, m *source.Metadata, compil
542540
depIDs = append(depIDs, string(depID))
543541
}
544542
sort.Strings(depIDs)
545-
for _, id := range depIDs {
546-
vdep := vdeps[PackageID(id)]
543+
for _, depID := range depIDs {
544+
vdep := vdeps[PackageID(depID)]
547545
fmt.Fprintf(hasher, "dep: %s\n", vdep.PkgPath)
548546
fmt.Fprintf(hasher, "export: %s\n", vdep.DeepExportHash)
549547

@@ -766,6 +764,7 @@ func typeCheckForAnalysis(fset *token.FileSet, parsed []*source.ParsedGoFile, m
766764
}
767765

768766
cfg := &types.Config{
767+
Sizes: m.TypesSizes,
769768
Error: func(e error) {
770769
pkg.compiles = false // type error
771770
pkg.typeErrors = append(pkg.typeErrors, e.(types.Error))

‎gopls/internal/lsp/cache/cache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func (c *Cache) PackageStats(withNames bool) template.HTML {
209209
typsCost := typesCost(v.pkg.types.Scope())
210210
typInfoCost := typesInfoCost(v.pkg.typesInfo)
211211
stat := packageStat{
212-
id: v.pkg.m.ID,
212+
id: v.pkg.id,
213213
mode: v.pkg.mode,
214214
types: typsCost,
215215
typesInfo: typInfoCost,

0 commit comments

Comments
 (0)