From ed276111079290e7e1b77db2343972dac95c4bc9 Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Mon, 13 Jun 2022 19:52:45 +0000 Subject: [PATCH] internal/lsp/cache: cache known subdirs pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Known subdirs change rarely and it's quite expensive to compute a glob pattern derived from them, so cache computation result and inherit it across generations. Computation cost is divided ≈evenly between `sort.Sort` and `span.URI.Filename` calls, and there is no trivial way to optimize them away besides caching. Benchmark (didChange in kubernetes): ~37ms->30ms Change-Id: Idb1691c76b8ff163dc61f637f07229498888606c GitHub-Last-Rev: cd99a9ce5c797afb5aaa9b478fcf433edd0dc03c GitHub-Pull-Request: golang/tools#383 Reviewed-on: https://go-review.googlesource.com/c/tools/+/411636 Reviewed-by: Alan Donovan Reviewed-by: Hyang-Ah Hana Kim --- internal/lsp/cache/snapshot.go | 60 ++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 6edd1dbe65..7b73f4b279 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -114,7 +114,8 @@ type snapshot struct { // knownSubdirs is the set of subdirectories in the workspace, used to // create glob patterns for file watching. - knownSubdirs map[span.URI]struct{} + knownSubdirs map[span.URI]struct{} + knownSubdirsPatternCache string // unprocessedSubdirChanges are any changes that might affect the set of // subdirectories in the workspace. They are not reflected to knownSubdirs // during the snapshot cloning step as it can slow down cloning. @@ -834,21 +835,38 @@ func (s *snapshot) fileWatchingGlobPatterns(ctx context.Context) map[string]stru // of the directories in the workspace. We find them by adding the // directories of every file in the snapshot's workspace directories. // There may be thousands. - knownSubdirs := s.getKnownSubdirs(dirs) - if n := len(knownSubdirs); n > 0 { - dirNames := make([]string, 0, n) - for _, uri := range knownSubdirs { - dirNames = append(dirNames, uri.Filename()) - } - sort.Strings(dirNames) - // The double allocation of Sprintf(Join()) accounts for 8% - // of DidChange, but specializing doesn't appear to help. :( - patterns[fmt.Sprintf("{%s}", strings.Join(dirNames, ","))] = struct{}{} + if pattern := s.getKnownSubdirsPattern(dirs); pattern != "" { + patterns[pattern] = struct{}{} } return patterns } +func (s *snapshot) getKnownSubdirsPattern(wsDirs []span.URI) string { + s.mu.Lock() + defer s.mu.Unlock() + + // First, process any pending changes and update the set of known + // subdirectories. + // It may change list of known subdirs and therefore invalidate the cache. + s.applyKnownSubdirsChangesLocked(wsDirs) + + if len(s.knownSubdirs) == 0 { + return "" + } + + if s.knownSubdirsPatternCache == "" { + dirNames := make([]string, 0, len(s.knownSubdirs)) + for uri := range s.knownSubdirs { + dirNames = append(dirNames, uri.Filename()) + } + sort.Strings(dirNames) + s.knownSubdirsPatternCache = fmt.Sprintf("{%s}", strings.Join(dirNames, ",")) + } + + return s.knownSubdirsPatternCache +} + // collectAllKnownSubdirs collects all of the subdirectories within the // snapshot's workspace directories. None of the workspace directories are // included. @@ -859,6 +877,7 @@ func (s *snapshot) collectAllKnownSubdirs(ctx context.Context) { defer s.mu.Unlock() s.knownSubdirs = map[span.URI]struct{}{} + s.knownSubdirsPatternCache = "" for uri := range s.files { s.addKnownSubdirLocked(uri, dirs) } @@ -870,6 +889,16 @@ func (s *snapshot) getKnownSubdirs(wsDirs []span.URI) []span.URI { // First, process any pending changes and update the set of known // subdirectories. + s.applyKnownSubdirsChangesLocked(wsDirs) + + result := make([]span.URI, 0, len(s.knownSubdirs)) + for uri := range s.knownSubdirs { + result = append(result, uri) + } + return result +} + +func (s *snapshot) applyKnownSubdirsChangesLocked(wsDirs []span.URI) { for _, c := range s.unprocessedSubdirChanges { if c.isUnchanged { continue @@ -881,12 +910,6 @@ func (s *snapshot) getKnownSubdirs(wsDirs []span.URI) []span.URI { } } s.unprocessedSubdirChanges = nil - - result := make([]span.URI, 0, len(s.knownSubdirs)) - for uri := range s.knownSubdirs { - result = append(result, uri) - } - return result } func (s *snapshot) addKnownSubdirLocked(uri span.URI, dirs []span.URI) { @@ -917,6 +940,7 @@ func (s *snapshot) addKnownSubdirLocked(uri span.URI, dirs []span.URI) { } s.knownSubdirs[uri] = struct{}{} dir = filepath.Dir(dir) + s.knownSubdirsPatternCache = "" } } @@ -929,6 +953,7 @@ func (s *snapshot) removeKnownSubdirLocked(uri span.URI) { } if info, _ := os.Stat(dir); info == nil { delete(s.knownSubdirs, uri) + s.knownSubdirsPatternCache = "" } dir = filepath.Dir(dir) } @@ -1816,6 +1841,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC for k, v := range s.knownSubdirs { result.knownSubdirs[k] = v } + result.knownSubdirsPatternCache = s.knownSubdirsPatternCache for _, c := range changes { result.unprocessedSubdirChanges = append(result.unprocessedSubdirChanges, c) }