Avoid fetching the same blob twice. Change-Id: Ie447d531b24a46256086caf05509da86b9c996c2
diff --git a/fs/gitilesfs.go b/fs/gitilesfs.go index b192c02..72462c3 100644 --- a/fs/gitilesfs.go +++ b/fs/gitilesfs.go
@@ -15,6 +15,7 @@ package fs import ( + "fmt" "io" "log" "os" @@ -51,6 +52,9 @@ shaMap map[git.Oid]string lazyRepo *cache.LazyRepo + + fetchingCond *sync.Cond + fetching map[git.Oid]bool } type linkNode struct { @@ -180,42 +184,78 @@ // given, we may try a clone of the git repository func (r *gitilesRoot) openFile(id git.Oid, clone bool) (*os.File, error) { f, ok := r.cache.Blob.Open(id) - if !ok { - repo := r.lazyRepo.Repository() - if clone && repo == nil { - r.lazyRepo.Clone() - } + if ok { + return f, nil + } - var content []byte - if repo != nil { - blob, err := repo.LookupBlob(&id) - if err != nil { - log.Printf("LookupBlob: %v", err) - return nil, syscall.ESPIPE - } - defer blob.Free() - content = blob.Contents() - } else { - path := r.shaMap[id] + f, err := r.fetchFile(id, clone) + if err != nil { + log.Printf("fetchFile(%s): %v", id, err) + return nil, syscall.ESPIPE + } - var err error - content, err = r.service.GetBlob(r.opts.Revision, path) - if err != nil { - log.Printf("GetBlob(%s, %s): %v", r.opts.Revision, path, err) - return nil, syscall.EDOM - } - } + return f, nil +} - if err := r.cache.Blob.Write(id, content); err != nil { - return nil, err - } +func (r *gitilesRoot) fetchFile(id git.Oid, clone bool) (*os.File, error) { + r.fetchingCond.L.Lock() + defer r.fetchingCond.L.Unlock() + for r.fetching[id] { + r.fetchingCond.Wait() + } + + f, ok := r.cache.Blob.Open(id) + if ok { + return f, nil + } + + r.fetching[id] = true + defer func() { delete(r.fetching, id) }() + r.fetchingCond.L.Unlock() + err := r.fetchFileExpensive(id, clone) + r.fetchingCond.L.Lock() + r.fetchingCond.Broadcast() + + if err == nil { f, ok = r.cache.Blob.Open(id) if !ok { - return nil, syscall.EROFS + return nil, fmt.Errorf("fetch succeeded, but blob %s not there", id.String()) + } + return f, nil + } + + return nil, err +} + +func (r *gitilesRoot) fetchFileExpensive(id git.Oid, clone bool) error { + repo := r.lazyRepo.Repository() + if clone && repo == nil { + r.lazyRepo.Clone() + } + + var content []byte + if repo != nil { + blob, err := repo.LookupBlob(&id) + if err != nil { + return err + } + defer blob.Free() + content = blob.Contents() + } else { + path := r.shaMap[id] + + var err error + content, err = r.service.GetBlob(r.opts.Revision, path) + if err != nil { + return fmt.Errorf("GetBlob(%s, %s): %v", r.opts.Revision, path, err) } } - return f, nil + + if err := r.cache.Blob.Write(id, content); err != nil { + return err + } + return nil } // dataNode makes arbitrary data available as a file. @@ -250,14 +290,16 @@ // NewGitilesRoot returns the root node for a file system. func NewGitilesRoot(c *cache.Cache, tree *gitiles.Tree, service *gitiles.RepoService, options GitilesOptions) nodefs.Node { r := &gitilesRoot{ - Node: newDirNode(), - service: service, - nodeCache: newNodeCache(), - cache: c, - shaMap: map[git.Oid]string{}, - tree: tree, - opts: options, - lazyRepo: cache.NewLazyRepo(options.CloneURL, c), + Node: newDirNode(), + service: service, + nodeCache: newNodeCache(), + cache: c, + shaMap: map[git.Oid]string{}, + tree: tree, + opts: options, + lazyRepo: cache.NewLazyRepo(options.CloneURL, c), + fetchingCond: sync.NewCond(&sync.Mutex{}), + fetching: map[git.Oid]bool{}, } return r
diff --git a/fs/gitilesfs_test.go b/fs/gitilesfs_test.go index 728b31e..35f5865 100644 --- a/fs/gitilesfs_test.go +++ b/fs/gitilesfs_test.go
@@ -27,6 +27,7 @@ "regexp" "sort" "strings" + "sync" "syscall" "testing" "time" @@ -157,11 +158,23 @@ `, } -func handleStatic(w http.ResponseWriter, r *http.Request) { +type testServer struct { + listener net.Listener + mu sync.Mutex + requests map[string]int +} + +func (s *testServer) handleStatic(w http.ResponseWriter, r *http.Request) { log.Println("handling", r.URL.String()) + + s.mu.Lock() + s.requests[r.URL.Path]++ + s.mu.Unlock() + resp, ok := testGitiles[r.URL.String()] if !ok { http.Error(w, "not found", 404) + return } out := []byte(resp) @@ -175,20 +188,27 @@ // TODO(hanwen): set content type? } -func newTestServer() (net.Listener, error) { +func newTestServer() (*testServer, error) { + listener, err := net.Listen("tcp", ":0") + if err != nil { + return nil, err + } + + ts := &testServer{ + listener: listener, + requests: map[string]int{}, + } + mux := http.NewServeMux() - mux.HandleFunc("/", handleStatic) + mux.HandleFunc("/", ts.handleStatic) s := &http.Server{ Handler: mux, } - listener, err := net.Listen("tcp", ":0") - if err != nil { - return nil, err - } - go s.Serve(listener) - return listener, err + go s.Serve(ts.listener) + + return ts, err } func TestGitilesFSSharedNodes(t *testing.T) { @@ -336,6 +356,47 @@ } } +func TestGitilesFSMultiFetch(t *testing.T) { + fix, err := newTestFixture() + if err != nil { + t.Fatal("newTestFixture", err) + } + defer fix.cleanup() + + repoService := fix.service.NewRepoService("platform/build/kati") + treeResp, err := repoService.GetTree("ce34badf691d36e8048b63f89d1a86ee5fa4325c", "", true) + if err != nil { + t.Fatal("Tree:", err) + } + + options := GitilesOptions{ + Revision: "ce34badf691d36e8048b63f89d1a86ee5fa4325c", + } + + fs := NewGitilesRoot(fix.cache, treeResp, repoService, options) + if err := fix.mount(fs); err != nil { + t.Fatal("mount", err) + } + + fn := filepath.Join(fix.mntDir, "AUTHORS") + + var wg sync.WaitGroup + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + ioutil.ReadFile(fn) + wg.Done() + }() + } + wg.Wait() + + for key, got := range fix.testServer.requests { + if got != 1 { + t.Errorf("got request count %d for %s, want 1", got, key) + } + } +} + func newManifestTestFixture(mf *manifest.Manifest) (*testFixture, error) { fix, err := newTestFixture() if err != nil { @@ -476,18 +537,18 @@ } type testFixture struct { - dir string - mntDir string - server *fuse.Server - cache *cache.Cache - listener net.Listener - service *gitiles.Service - root nodefs.Node + dir string + mntDir string + server *fuse.Server + cache *cache.Cache + testServer *testServer + service *gitiles.Service + root nodefs.Node } func (f *testFixture) cleanup() { - if f.listener != nil { - f.listener.Close() + if f.testServer != nil { + f.testServer.listener.Close() } if f.server != nil { f.server.Unmount() @@ -508,13 +569,13 @@ return nil, err } - fixture.listener, err = newTestServer() + fixture.testServer, err = newTestServer() if err != nil { return nil, err } fixture.service, err = gitiles.NewService( - fmt.Sprintf("http://%s", fixture.listener.Addr().String())) + fmt.Sprintf("http://%s", fixture.testServer.listener.Addr().String())) if err != nil { return nil, err }