storage/dotgit: add KeepDescriptors option

This option maintains packfile file descriptors opened after reading
objects from them. It improves performance as it does not have to be
opening packfiles each time an object is needed.

Also adds Close to EncodedObjectStorer to close all the files manualy.

Signed-off-by: Javi Fontan <jfontan@gmail.com>

7 files changed, 92 insertions, 2 deletions

diff --git a/plumbing/storer/object.go b/plumbing/storer/object.go
index 92aa629..806b6bb 100644
--- a/plumbing/storer/object.go
+++ b/plumbing/storer/object.go
@@ -40,6 +40,8 @@ type EncodedObjectStorer interface {
 	// HasEncodedObject returns ErrObjNotFound if the object doesn't
 	// exist.  If the object does exist, it returns nil.
 	HasEncodedObject(plumbing.Hash) error
+	// Close any opened files.
+	Close() error
 }
 
 // DeltaObjectStorer is an EncodedObjectStorer that can return delta
diff --git a/plumbing/storer/object_test.go b/plumbing/storer/object_test.go
index 6b4fe0f..adddec4 100644
--- a/plumbing/storer/object_test.go
+++ b/plumbing/storer/object_test.go
@@ -157,3 +157,7 @@ func (o *MockObjectStorage) IterEncodedObjects(t plumbing.ObjectType) (EncodedOb
 func (o *MockObjectStorage) Begin() Transaction {
 	return nil
 }
+
+func (o *MockObjectStorage) Close() error {
+	return nil
+}
diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go
index 00dd2a4..df5cd10 100644
--- a/storage/filesystem/dotgit/dotgit.go
+++ b/storage/filesystem/dotgit/dotgit.go
@@ -62,6 +62,9 @@ type Options struct {
 	// ExclusiveAccess means that the filesystem is not modified externally
 	// while the repo is open.
 	ExclusiveAccess bool
+	// KeepDescriptors makes the file descriptors to be reused but they will
+	// need to be manually closed calling Close().
+	KeepDescriptors bool
 }
 
 // The DotGit type represents a local git repository on disk. This
@@ -78,6 +81,8 @@ type DotGit struct {
 	objectMap  map[plumbing.Hash]struct{}
 	packList   []plumbing.Hash
 	packMap    map[plumbing.Hash]struct{}
+
+	files map[string]billy.File
 }
 
 // New returns a DotGit value ready to be used. The path argument must
@@ -123,6 +128,28 @@ func (d *DotGit) Initialize() error {
 	return nil
 }
 
+// Close closes all opened files.
+func (d *DotGit) Close() error {
+	var firstError error
+	if d.files != nil {
+		for _, f := range d.files {
+			err := f.Close()
+			if err != nil && firstError == nil {
+				firstError = err
+				continue
+			}
+		}
+
+		d.files = nil
+	}
+
+	if firstError != nil {
+		return firstError
+	}
+
+	return nil
+}
+
 // ConfigWriter returns a file pointer for write to the config file
 func (d *DotGit) ConfigWriter() (billy.File, error) {
 	return d.fs.Create(configPath)
@@ -217,12 +244,22 @@ func (d *DotGit) objectPackPath(hash plumbing.Hash, extension string) string {
 }
 
 func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.File, error) {
+	if d.files == nil {
+		d.files = make(map[string]billy.File)
+	}
+
 	err := d.hasPack(hash)
 	if err != nil {
 		return nil, err
 	}
 
-	pack, err := d.fs.Open(d.objectPackPath(hash, extension))
+	path := d.objectPackPath(hash, extension)
+	f, ok := d.files[path]
+	if ok {
+		return f, nil
+	}
+
+	pack, err := d.fs.Open(path)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return nil, ErrPackfileNotFound
@@ -231,6 +268,10 @@ func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.Fil
 		return nil, err
 	}
 
+	if d.options.KeepDescriptors && extension == "pack" {
+		d.files[path] = pack
+	}
+
 	return pack, nil
 }
 
diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go
index c34543e..50f8e64 100644
--- a/storage/filesystem/dotgit/dotgit_test.go
+++ b/storage/filesystem/dotgit/dotgit_test.go
@@ -465,6 +465,34 @@ func (s *SuiteDotGit) TestObjectPack(c *C) {
 	c.Assert(filepath.Ext(pack.Name()), Equals, ".pack")
 }
 
+func (s *SuiteDotGit) TestObjectPackWithKeepDescriptors(c *C) {
+	f := fixtures.Basic().ByTag(".git").One()
+	fs := f.DotGit()
+	dir := NewWithOptions(fs, Options{KeepDescriptors: true})
+
+	pack, err := dir.ObjectPack(f.PackfileHash)
+	c.Assert(err, IsNil)
+	c.Assert(filepath.Ext(pack.Name()), Equals, ".pack")
+
+	pack2, err := dir.ObjectPack(f.PackfileHash)
+	c.Assert(err, IsNil)
+	c.Assert(pack, Equals, pack2)
+
+	err = dir.Close()
+	c.Assert(err, IsNil)
+
+	pack2, err = dir.ObjectPack(f.PackfileHash)
+	c.Assert(err, IsNil)
+	c.Assert(pack, Not(Equals), pack2)
+
+	err = pack2.Close()
+	c.Assert(err, IsNil)
+
+	err = dir.Close()
+	c.Assert(err, NotNil)
+
+}
+
 func (s *SuiteDotGit) TestObjectPackIdx(c *C) {
 	f := fixtures.Basic().ByTag(".git").One()
 	fs := f.DotGit()
diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go
index 3519385..3545e27 100644
--- a/storage/filesystem/object.go
+++ b/storage/filesystem/object.go
@@ -74,6 +74,7 @@ func (s *ObjectStorage) loadIdxFile(h plumbing.Hash) (err error) {
 	}
 
 	defer ioutil.CheckClose(f, &err)
+
 	idxf := idxfile.NewMemoryIndex()
 	d := idxfile.NewDecoder(f)
 	if err = d.Decode(idxf); err != nil {
@@ -280,7 +281,9 @@ func (s *ObjectStorage) getFromPackfile(h plumbing.Hash, canBeDelta bool) (
 		return nil, err
 	}
 
-	defer ioutil.CheckClose(f, &err)
+	if !s.options.KeepDescriptors {
+		defer ioutil.CheckClose(f, &err)
+	}
 
 	idx := s.index[pack]
 	if canBeDelta {
@@ -423,6 +426,11 @@ func (s *ObjectStorage) buildPackfileIters(t plumbing.ObjectType, seen map[plumb
 	}, nil
 }
 
+// Close closes all opened files.
+func (s *ObjectStorage) Close() error {
+	return s.dir.Close()
+}
+
 type lazyPackfilesIter struct {
 	hashes []plumbing.Hash
 	open   func(h plumbing.Hash) (storer.EncodedObjectIter, error)
diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go
index 25b3653..7fae789 100644
--- a/storage/filesystem/storage.go
+++ b/storage/filesystem/storage.go
@@ -27,6 +27,9 @@ type Options struct {
 	// ExclusiveAccess means that the filesystem is not modified externally
 	// while the repo is open.
 	ExclusiveAccess bool
+	// KeepDescriptors makes the file descriptors to be reused but they will
+	// need to be manually closed calling Close().
+	KeepDescriptors bool
 }
 
 // NewStorage returns a new Storage backed by a given `fs.Filesystem`
@@ -41,6 +44,7 @@ func NewStorageWithOptions(
 ) (*Storage, error) {
 	dirOps := dotgit.Options{
 		ExclusiveAccess: ops.ExclusiveAccess,
+		KeepDescriptors: ops.KeepDescriptors,
 	}
 
 	dir := dotgit.NewWithOptions(fs, dirOps)
diff --git a/storage/memory/storage.go b/storage/memory/storage.go
index 2e32509..3d3b348 100644
--- a/storage/memory/storage.go
+++ b/storage/memory/storage.go
@@ -183,6 +183,9 @@ func (o *ObjectStorage) ObjectPacks() ([]plumbing.Hash, error) {
 func (o *ObjectStorage) DeleteOldObjectPackAndIndex(plumbing.Hash, time.Time) error {
 	return nil
 }
+func (s *ObjectStorage) Close() error {
+	return nil
+}
 
 var errNotSupported = fmt.Errorf("Not supported")