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
}
