gopls/internal/cache: share goimports state for GOMODCACHE

When using the gopls daemon, or with the multi-View workspaces that will
be increasingly common following golang/go#57979, there is a lot of
redundant work performed scanning the module cache. This CL eliminates
that redundancy, by moving module cache information into the cache.Cache
shared by all Sessions and Views.

There should be effectively no change in behavior for gopls resulting
from this CL. In ModuleResolver.scan, we still require that module cache
roots are scanned. However, we no longer invalidate this scan in
ModuleResolver.ClearForNewScan: re-scanning the module cache is the
responsibility of a new ScanModuleCache function, which is independently
scheduled. To enable this separation of refresh logic, a new
refreshTimer type is extracted to encapsulate the refresh logic.

For golang/go#44863

Change-Id: I333d55fca009be7984a514ed4abdc9a9fcafc08a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/559636
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

gopls/internal/cache/cache.go

@@ -10,6 +10,7 @@ import (
 	"sync/atomic"
 
 	"golang.org/x/tools/gopls/internal/protocol/command"
+	"golang.org/x/tools/internal/imports"
 	"golang.org/x/tools/internal/memoize"
 )
 
@@ -30,20 +31,36 @@ func New(store *memoize.Store) *Cache {
 		id:         strconv.FormatInt(index, 10),
 		store:      store,
 		memoizedFS: newMemoizedFS(),
+		modCache: &sharedModCache{
+			caches: make(map[string]*imports.DirInfoCache),
+			timers: make(map[string]*refreshTimer),
+		},
 	}
 	return c
 }
 
-// A Cache holds caching stores that are bundled together for consistency.
-//
-// TODO(rfindley): once fset and store need not be bundled together, the Cache
-// type can be eliminated.
+// A Cache holds content that is shared across multiple gopls sessions.
 type Cache struct {
 	id string
 
+	// store holds cached calculations.
+	//
+	// TODO(rfindley): at this point, these are not important, as we've moved our
+	// content-addressable cache to the file system (the filecache package). It
+	// is unlikely that this shared cache provides any shared value. We should
+	// consider removing it, replacing current uses with a simpler futures cache,
+	// as we've done for e.g. type-checked packages.
 	store *memoize.Store
 
-	*memoizedFS // implements file.Source
+	// memoizedFS holds a shared file.Source that caches reads.
+	//
+	// Reads are invalidated when *any* session gets a didChangeWatchedFile
+	// notification. This is fine: it is the responsibility of memoizedFS to hold
+	// our best knowledge of the current file system state.
+	*memoizedFS
+
+	// modCache holds the
+	modCache *sharedModCache
 }
 
 var cacheIndex, sessionIndex, viewIndex int64

gopls/internal/cache/imports.go

@@ -13,19 +13,129 @@ import (
 	"golang.org/x/tools/gopls/internal/file"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/event/keys"
+	"golang.org/x/tools/internal/event/tag"
 	"golang.org/x/tools/internal/imports"
 )
 
+// refreshTimer implements delayed asynchronous refreshing of state.
+//
+// See the [refreshTimer.schedule] documentation for more details.
+type refreshTimer struct {
+	mu        sync.Mutex
+	duration  time.Duration
+	timer     *time.Timer
+	refreshFn func()
+}
+
+// newRefreshTimer constructs a new refresh timer which schedules refreshes
+// using the given function.
+func newRefreshTimer(refresh func()) *refreshTimer {
+	return &refreshTimer{
+		refreshFn: refresh,
+	}
+}
+
+// schedule schedules the refresh function to run at some point in the future,
+// if no existing refresh is already scheduled.
+//
+// At a minimum, scheduled refreshes are delayed by 30s, but they may be
+// delayed longer to keep their expected execution time under 2% of wall clock
+// time.
+func (t *refreshTimer) schedule() {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+
+	if t.timer == nil {
+		// Don't refresh more than twice per minute.
+		delay := 30 * time.Second
+		// Don't spend more than ~2% of the time refreshing.
+		if adaptive := 50 * t.duration; adaptive > delay {
+			delay = adaptive
+		}
+		t.timer = time.AfterFunc(delay, func() {
+			start := time.Now()
+			t.refreshFn()
+			t.mu.Lock()
+			t.duration = time.Since(start)
+			t.timer = nil
+			t.mu.Unlock()
+		})
+	}
+}
+
+// A sharedModCache tracks goimports state for GOMODCACHE directories
+// (each session may have its own GOMODCACHE).
+//
+// This state is refreshed independently of view-specific imports state.
+type sharedModCache struct {
+	mu     sync.Mutex
+	caches map[string]*imports.DirInfoCache // GOMODCACHE -> cache content; never invalidated
+	timers map[string]*refreshTimer         // GOMODCACHE -> timer
+}
+
+func (c *sharedModCache) dirCache(dir string) *imports.DirInfoCache {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	cache, ok := c.caches[dir]
+	if !ok {
+		cache = imports.NewDirInfoCache()
+		c.caches[dir] = cache
+	}
+	return cache
+}
+
+// refreshDir schedules a refresh of the given directory, which must be a
+// module cache.
+func (c *sharedModCache) refreshDir(ctx context.Context, dir string, logf func(string, ...any)) {
+	cache := c.dirCache(dir)
+
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	timer, ok := c.timers[dir]
+	if !ok {
+		timer = newRefreshTimer(func() {
+			_, done := event.Start(ctx, "cache.sharedModCache.refreshDir", tag.Directory.Of(dir))
+			defer done()
+			imports.ScanModuleCache(dir, cache, logf)
+		})
+		c.timers[dir] = timer
+	}
+
+	timer.schedule()
+}
+
+// importsState tracks view-specific imports state.
 type importsState struct {
-	ctx context.Context
+	ctx          context.Context
+	modCache     *sharedModCache
+	refreshTimer *refreshTimer
+
+	mu                sync.Mutex
+	processEnv        *imports.ProcessEnv
+	cachedModFileHash file.Hash
+}
 
-	mu                   sync.Mutex
-	processEnv           *imports.ProcessEnv
-	cacheRefreshDuration time.Duration
-	cacheRefreshTimer    *time.Timer
-	cachedModFileHash    file.Hash
+// newImportsState constructs a new imports state for running goimports
+// functions via [runProcessEnvFunc].
+//
+// The returned state will automatically refresh itself following a call to
+// runProcessEnvFunc.
+func newImportsState(backgroundCtx context.Context, modCache *sharedModCache, env *imports.ProcessEnv) *importsState {
+	s := &importsState{
+		ctx:        backgroundCtx,
+		modCache:   modCache,
+		processEnv: env,
+	}
+	s.refreshTimer = newRefreshTimer(s.refreshProcessEnv)
+	return s
 }
 
+// runProcessEnvFunc runs goimports.
+//
+// Any call to runProcessEnvFunc will schedule a refresh of the imports state
+// at some point in the future, if such a refresh is not already scheduled. See
+// [refreshTimer] for more details.
 func (s *importsState) runProcessEnvFunc(ctx context.Context, snapshot *Snapshot, fn func(context.Context, *imports.Options) error) error {
 	ctx, done := event.Start(ctx, "cache.importsState.runProcessEnvFunc")
 	defer done()
@@ -72,15 +182,20 @@ func (s *importsState) runProcessEnvFunc(ctx context.Context, snapshot *Snapshot
 		return err
 	}
 
-	if s.cacheRefreshTimer == nil {
-		// Don't refresh more than twice per minute.
-		delay := 30 * time.Second
-		// Don't spend more than a couple percent of the time refreshing.
-		if adaptive := 50 * s.cacheRefreshDuration; adaptive > delay {
-			delay = adaptive
-		}
-		s.cacheRefreshTimer = time.AfterFunc(delay, s.refreshProcessEnv)
-	}
+	// Refresh the imports resolver after usage. This may seem counterintuitive,
+	// since it means the first ProcessEnvFunc after a long period of inactivity
+	// may be stale, but in practice we run ProcessEnvFuncs frequently during
+	// active development (e.g. during completion), and so this mechanism will be
+	// active while gopls is in use, and inactive when gopls is idle.
+	s.refreshTimer.schedule()
+
+	// TODO(rfindley): the GOMODCACHE value used here isn't directly tied to the
+	// ProcessEnv.Env["GOMODCACHE"], though they should theoretically always
+	// agree. It would be better if we guaranteed this, possibly by setting all
+	// required environment variables in ProcessEnv.Env, to avoid the redundant
+	// Go command invocation.
+	gomodcache := snapshot.view.folder.Env.GOMODCACHE
+	s.modCache.refreshDir(s.ctx, gomodcache, s.processEnv.Logf)
 
 	return nil
 }
@@ -96,16 +211,17 @@ func (s *importsState) refreshProcessEnv() {
 	if resolver, err := s.processEnv.GetResolver(); err == nil {
 		resolver.ClearForNewScan()
 	}
+	// TODO(rfindley): it's not clear why we're unlocking here. Shouldn't we
+	// guard the use of env below? In any case, we can prime a separate resolver.
 	s.mu.Unlock()
 
 	event.Log(s.ctx, "background imports cache refresh starting")
+
+	// TODO(rfindley, golang/go#59216): do this priming with a separate resolver,
+	// and then replace, so that we never have to wait on an unprimed cache.
 	if err := imports.PrimeCache(context.Background(), env); err == nil {
 		event.Log(ctx, fmt.Sprintf("background refresh finished after %v", time.Since(start)))
 	} else {
 		event.Log(ctx, fmt.Sprintf("background refresh finished after %v", time.Since(start)), keys.Err.Of(err))
 	}
-	s.mu.Lock()
-	s.cacheRefreshDuration = time.Since(start)
-	s.cacheRefreshTimer = nil
-	s.mu.Unlock()
 }

gopls/internal/cache/session.go

@@ -209,6 +209,7 @@ func (s *Session) createView(ctx context.Context, def *viewDefinition) (*View, *
 			SkipPathInScan: skipPath,
 			Env:            env,
 			WorkingDir:     def.root.Path(),
+			ModCache:       s.cache.modCache.dirCache(def.folder.Env.GOMODCACHE),
 		}
 		if def.folder.Options.VerboseOutput {
 			pe.Logf = func(format string, args ...interface{}) {
@@ -227,10 +228,7 @@ func (s *Session) createView(ctx context.Context, def *viewDefinition) (*View, *
 		ignoreFilter:         ignoreFilter,
 		fs:                   s.overlayFS,
 		viewDefinition:       def,
-		importsState: &importsState{
-			ctx:        backgroundCtx,
-			processEnv: pe,
-		},
+		importsState:         newImportsState(backgroundCtx, s.cache.modCache, pe),
 	}
 
 	s.snapshotWG.Add(1)

internal/imports/fix.go

@@ -884,6 +884,10 @@ type ProcessEnv struct {
 	// If Logf is non-nil, debug logging is enabled through this function.
 	Logf func(format string, args ...interface{})
 
+	// If set, ModCache holds a shared cache of directory info to use across
+	// multiple ProcessEnvs.
+	ModCache *DirInfoCache
+
 	initialized bool // see TODO above
 
 	// resolver and resolverErr are lazily evaluated (see GetResolver).
@@ -984,7 +988,7 @@ func (e *ProcessEnv) GetResolver() (Resolver, error) {
 		if len(e.Env["GOMOD"]) == 0 && len(e.Env["GOWORK"]) == 0 {
 			e.resolver = newGopathResolver(e)
 		} else {
-			e.resolver, e.resolverErr = newModuleResolver(e)
+			e.resolver, e.resolverErr = newModuleResolver(e, e.ModCache)
 		}
 	}
 
@@ -1252,17 +1256,14 @@ func ImportPathToAssumedName(importPath string) string {
 type gopathResolver struct {
 	env      *ProcessEnv
 	walked   bool
-	cache    *dirInfoCache
+	cache    *DirInfoCache
 	scanSema chan struct{} // scanSema prevents concurrent scans.
 }
 
 func newGopathResolver(env *ProcessEnv) *gopathResolver {
 	r := &gopathResolver{
-		env: env,
-		cache: &dirInfoCache{
-			dirs:      map[string]*directoryPackageInfo{},
-			listeners: map[*int]cacheListener{},
-		},
+		env:      env,
+		cache:    NewDirInfoCache(),
 		scanSema: make(chan struct{}, 1),
 	}
 	r.scanSema <- struct{}{}
@@ -1271,10 +1272,7 @@ func newGopathResolver(env *ProcessEnv) *gopathResolver {
 
 func (r *gopathResolver) ClearForNewScan() {
 	<-r.scanSema
-	r.cache = &dirInfoCache{
-		dirs:      map[string]*directoryPackageInfo{},
-		listeners: map[*int]cacheListener{},
-	}
+	r.cache = NewDirInfoCache()
 	r.walked = false
 	r.scanSema <- struct{}{}
 }