Skip to content

Commit c3edf5a

Browse files
committed
gopls/internal/lsp/cache: actually preload files in parallel
snapshot.ReadFile holds the snapshot lock, so calling it concurrently was pointless. Instead, add a new preloadFiles helper that delegates to to the underlying FileSource. For golang/go#57987 Change-Id: If3259ca45260b8a24542d40f7558b5d6bf5018d1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/477975 gopls-CI: kokoro <[email protected]> Run-TryBot: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent acaae98 commit c3edf5a

File tree

2 files changed

+38
-12
lines changed

2 files changed

+38
-12
lines changed

gopls/internal/lsp/cache/load.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"path/filepath"
1313
"sort"
1414
"strings"
15-
"sync"
1615
"sync/atomic"
1716
"time"
1817

@@ -255,7 +254,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
255254
s.dumpWorkspace("load")
256255
s.mu.Unlock()
257256

258-
// Opt: pre-fetch loaded files in parallel.
257+
// Opt: preLoad files in parallel.
259258
//
260259
// Requesting files in batch optimizes the underlying filesystem reads.
261260
// However, this is also currently necessary for correctness: populating all
@@ -266,16 +265,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
266265
// TODO(rfindley, golang/go#57558): determine the set of directories based on
267266
// loaded packages, so that reading files here is not necessary for
268267
// correctness.
269-
var wg sync.WaitGroup
270-
for _, uri := range files {
271-
uri := uri
272-
wg.Add(1)
273-
go func() {
274-
s.ReadFile(ctx, uri) // ignore result
275-
wg.Done()
276-
}()
277-
}
278-
wg.Wait()
268+
s.preloadFiles(ctx, files)
279269

280270
if len(moduleErrs) > 0 {
281271
return &moduleErrorMap{moduleErrs}

gopls/internal/lsp/cache/snapshot.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,6 +1240,42 @@ func (s *snapshot) ReadFile(ctx context.Context, uri span.URI) (source.FileHandl
12401240
return lockedSnapshot{s}.ReadFile(ctx, uri)
12411241
}
12421242

1243+
// preloadFiles delegates to the view FileSource to read the requested uris in
1244+
// parallel, without holding the snapshot lock.
1245+
func (s *snapshot) preloadFiles(ctx context.Context, uris []span.URI) {
1246+
files := make([]source.FileHandle, len(uris))
1247+
var wg sync.WaitGroup
1248+
iolimit := make(chan struct{}, 20) // I/O concurrency limiting semaphore
1249+
for i, uri := range uris {
1250+
wg.Add(1)
1251+
iolimit <- struct{}{}
1252+
go func(i int, uri span.URI) {
1253+
defer wg.Done()
1254+
fh, err := s.view.fs.ReadFile(ctx, uri)
1255+
<-iolimit
1256+
if err != nil && ctx.Err() == nil {
1257+
event.Error(ctx, fmt.Sprintf("reading %s", uri), err)
1258+
return
1259+
}
1260+
files[i] = fh
1261+
}(i, uri)
1262+
}
1263+
wg.Wait()
1264+
1265+
s.mu.Lock()
1266+
defer s.mu.Unlock()
1267+
1268+
for i, fh := range files {
1269+
if fh == nil {
1270+
continue // error logged above
1271+
}
1272+
uri := uris[i]
1273+
if _, ok := s.files.Get(uri); !ok {
1274+
s.files.Set(uri, fh)
1275+
}
1276+
}
1277+
}
1278+
12431279
// A lockedSnapshot implements the source.FileSource interface while holding
12441280
// the lock for the wrapped snapshot.
12451281
type lockedSnapshot struct{ wrapped *snapshot }

0 commit comments

Comments
 (0)