gomodfs, store/remotestore: fetch less over the network in WinFsp mode

store/remotestore/remotestore.go

@@ -36,9 +36,17 @@ type Store struct {
 
 	mu      sync.Mutex
 	modmaps map[store.ModuleVersion]*remoteModHandle // cached modmap per MV
+	meta    map[metaKey][]byte                       // cached .info, .mod and .ziphash files
 	tmpDir  string                                   // lazy-created temp dir for large files
 }
 
+// metaKey is the key of a cached metadata file: ext is "info", "mod" or
+// "ziphash".
+type metaKey struct {
+	mv  store.ModuleVersion
+	ext string
+}
+
 // Close cleans up temporary files. It should be called when the store is no longer needed.
 func (s *Store) Close() error {
 	s.mu.Lock()
@@ -274,7 +282,40 @@ func (s *Store) GetZipHash(ctx context.Context, h store.ModHandle) ([]byte, erro
 	return s.getMetaFile(ctx, rmh.mv, "ziphash")
 }
 
+// getMetaFile returns the metadata file with extension ext for mv.
+//
+// Metadata files never change for a module version, and the go command
+// reads hundreds of them per invocation (each stat and open of one is a
+// call here), so they're cached for the life of the Store. Misses aren't
+// cached, as the server may get the module later.
 func (s *Store) getMetaFile(ctx context.Context, mv store.ModuleVersion, ext string) ([]byte, error) {
+	k := metaKey{mv, ext}
+	s.mu.Lock()
+	v, ok := s.meta[k]
+	s.mu.Unlock()
+	if ok {
+		return v, nil
+	}
+	vi, err, _ := s.sf.Do("meta:"+mv.Module+"@"+mv.Version+"."+ext, func() (any, error) {
+		v, err := s.fetchMetaFile(ctx, mv, ext)
+		if err != nil {
+			return nil, err
+		}
+		s.mu.Lock()
+		defer s.mu.Unlock()
+		if s.meta == nil {
+			s.meta = make(map[metaKey][]byte)
+		}
+		s.meta[k] = v
+		return v, nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	return vi.([]byte), nil
+}
+
+func (s *Store) fetchMetaFile(ctx context.Context, mv store.ModuleVersion, ext string) ([]byte, error) {
 	escaped, err := mvToEscapedPath(mv)
 	if err != nil {
 		return nil, err

store/remotestore/remotestore_test.go

@@ -5,11 +5,14 @@ package remotestore
 
 import (
 	"context"
+	"errors"
 	"io/fs"
+	"maps"
 	"net"
 	"net/http"
 	"net/http/httptest"
 	"slices"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -60,6 +63,65 @@ func TestDefaultClientReusesConns(t *testing.T) {
 	}
 }
 
+// TestMetaFileCache tests that a Store fetches each metadata file from
+// the server once, even when asked concurrently, and doesn't cache misses.
+func TestMetaFileCache(t *testing.T) {
+	var mu sync.Mutex
+	reqs := map[string]int{} // URL path -> requests
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		mu.Lock()
+		reqs[r.URL.Path]++
+		mu.Unlock()
+		if strings.Contains(r.URL.Path, "/missing/") {
+			http.NotFound(w, r)
+			return
+		}
+		w.Write([]byte("contents of " + r.URL.Path))
+	}))
+	defer srv.Close()
+
+	s := &Store{BaseURL: srv.URL}
+	ctx := context.Background()
+	mv := store.ModuleVersion{Module: "example.com/m", Version: "v1.0.0"}
+	var wg sync.WaitGroup
+	for range 10 {
+		wg.Go(func() {
+			for range 5 {
+				mod, err := s.GetModFile(ctx, mv)
+				if err != nil {
+					t.Error(err)
+					return
+				}
+				if want := "contents of " + metaPrefix + "example.com/m/@v/v1.0.0.mod"; string(mod) != want {
+					t.Errorf("GetModFile = %q; want %q", mod, want)
+				}
+				if _, err := s.GetInfoFile(ctx, mv); err != nil {
+					t.Error(err)
+				}
+			}
+		})
+	}
+	wg.Wait()
+
+	missing := store.ModuleVersion{Module: "example.com/missing", Version: "v1.0.0"}
+	for range 2 {
+		if _, err := s.GetModFile(ctx, missing); !errors.Is(err, store.ErrCacheMiss) {
+			t.Errorf("GetModFile(missing) error = %v; want ErrCacheMiss", err)
+		}
+	}
+
+	mu.Lock()
+	defer mu.Unlock()
+	want := map[string]int{
+		metaPrefix + "example.com/m/@v/v1.0.0.mod":       1,
+		metaPrefix + "example.com/m/@v/v1.0.0.info":      1,
+		metaPrefix + "example.com/missing/@v/v1.0.0.mod": 2,
+	}
+	if !maps.Equal(reqs, want) {
+		t.Errorf("server requests = %v; want %v", reqs, want)
+	}
+}
+
 func TestBuildDirMap(t *testing.T) {
 	tests := []struct {
 		name  string

winfsp.go

@@ -15,10 +15,10 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"sync"
 	"syscall"
 	"time"
 
-	"github.com/tailscale/gomodfs/store"
 	"github.com/winfsp/go-winfsp"
 	"github.com/winfsp/go-winfsp/gofs"
 	"golang.org/x/sys/windows"
@@ -295,25 +295,30 @@ func (pfs *fspFS) OpenFile(name string, flag int, perm os.FileMode) (retFile gof
 	if err != nil {
 		return nil, err
 	}
-	spanGF := d.fs.Stats.StartSpan("fsp.OpenFile-GetFile")
-	contents, err := d.fs.Store.GetFile(ctx, mh, dp.Path)
+	fi, err := d.fs.Store.Stat(ctx, mh, dp.Path)
 	if err != nil {
-		if errors.Is(err, store.ErrIsDir) {
-			spanRD := d.fs.Stats.StartSpan("fsp.OpenFile-Readdir")
-			ents, err := d.fs.Store.Readdir(ctx, mh, dp.Path)
-			spanRD.End(err)
-			if err != nil {
-				spanGF.End(err)
-				return nil, err
-			}
-			spanGF.End(nil)
-			return wdDir{pathInZip: dp.Path, baseName: base, ents: ents}, nil
-		}
-		spanGF.End(err)
 		return nil, err
 	}
-	spanGF.End(nil)
-	return newFWPFileFromContents(name, contents), nil
+	if fi.IsDir() {
+		spanRD := d.fs.Stats.StartSpan("fsp.OpenFile-Readdir")
+		ents, err := d.fs.Store.Readdir(ctx, mh, dp.Path)
+		spanRD.End(err)
+		if err != nil {
+			return nil, err
+		}
+		return wdDir{pathInZip: dp.Path, baseName: base, ents: ents}, nil
+	}
+	// Get the contents on the first read, not now: Windows opens files for
+	// many things that don't read them, including every os.Stat.
+	return &winFSPRegularFile{
+		fi: regFileInfo{name: name, size: fi.Size()},
+		getContents: func() ([]byte, error) {
+			sp := d.fs.Stats.StartSpan("fsp.ReadAt-GetFile")
+			contents, err := d.fs.Store.GetFile(ctx, mh, dp.Path)
+			sp.End(err)
+			return contents, err
+		},
+	}, nil
 }
 
 func (s *winFSPRunner) Unmount() error {
@@ -350,8 +355,32 @@ func newFWPFileFromContents(baseName string, contents []byte) gofs.File {
 }
 
 type winFSPRegularFile struct {
-	fi       os.FileInfo
-	contents []byte
+	fi os.FileInfo
+
+	// getContents, if non-nil, returns the file's contents. It's called on
+	// the first ReadAt, and again on later ones until it succeeds.
+	getContents func() ([]byte, error)
+
+	mu       sync.Mutex
+	contents []byte // the contents, once known
+	haveData bool   // contents is valid
+}
+
+// data returns the file's contents, getting them first if needed.
+func (f *winFSPRegularFile) data() ([]byte, error) {
+	if f.getContents == nil {
+		return f.contents, nil
+	}
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	if !f.haveData {
+		contents, err := f.getContents()
+		if err != nil {
+			return nil, err
+		}
+		f.contents, f.haveData = contents, true
+	}
+	return f.contents, nil
 }
 
 func (f *winFSPRegularFile) Close() error { return nil }
@@ -374,7 +403,11 @@ func (f *winFSPRegularFile) Read(p []byte) (n int, err error) {
 }
 
 func (f *winFSPRegularFile) ReadAt(p []byte, off int64) (n int, err error) {
-	n = copy(p, f.contents[min(off, int64(len(f.contents))):])
+	contents, err := f.data()
+	if err != nil {
+		return 0, err
+	}
+	n = copy(p, contents[min(off, int64(len(contents))):])
 	if n == 0 && len(p) > 0 {
 		return 0, io.EOF
 	}