Merge pull request #698 from jfontan/improvement/use-decoder-cache

plumbing: cache, enforce the use of cache in packfile decoder

2 files changed, 39 insertions, 17 deletions

diff --git a/plumbing/format/packfile/decoder.go b/plumbing/format/packfile/decoder.go
index ad72ea0..cb78701 100644
--- a/plumbing/format/packfile/decoder.go
+++ b/plumbing/format/packfile/decoder.go
@@ -52,7 +52,7 @@ var (
 // is destroyed. The Offsets and CRCs are calculated whether an
 // ObjectStorer was provided or not.
 type Decoder struct {
-	DeltaBaseCache cache.Object
+	deltaBaseCache cache.Object
 
 	s  *Scanner
 	o  storer.EncodedObjectStorer
@@ -80,15 +80,27 @@ type Decoder struct {
 // If the ObjectStorer implements storer.Transactioner, a transaction is created
 // during the Decode execution. If anything fails, Rollback is called
 func NewDecoder(s *Scanner, o storer.EncodedObjectStorer) (*Decoder, error) {
-	return NewDecoderForType(s, o, plumbing.AnyObject)
+	return NewDecoderForType(s, o, plumbing.AnyObject,
+		cache.NewObjectLRUDefault())
+}
+
+// NewDecoderWithCache is a version of NewDecoder where cache can be specified.
+func NewDecoderWithCache(s *Scanner, o storer.EncodedObjectStorer,
+	cacheObject cache.Object) (*Decoder, error) {
+
+	return NewDecoderForType(s, o, plumbing.AnyObject, cacheObject)
 }
 
 // NewDecoderForType returns a new Decoder but in this case for a specific object type.
 // When an object is read using this Decoder instance and it is not of the same type of
 // the specified one, nil will be returned. This is intended to avoid the content
-// deserialization of all the objects
+// deserialization of all the objects.
+//
+// cacheObject is a cache.Object implementation that is used to speed up the
+// process. If cache is not needed you can pass nil. To create an LRU cache
+// object with the default size you can use the helper cache.ObjectLRUDefault().
 func NewDecoderForType(s *Scanner, o storer.EncodedObjectStorer,
-	t plumbing.ObjectType) (*Decoder, error) {
+	t plumbing.ObjectType, cacheObject cache.Object) (*Decoder, error) {
 
 	if t == plumbing.OFSDeltaObject ||
 		t == plumbing.REFDeltaObject ||
@@ -101,8 +113,9 @@ func NewDecoderForType(s *Scanner, o storer.EncodedObjectStorer,
 	}
 
 	return &Decoder{
-		s: s,
-		o: o,
+		s:              s,
+		o:              o,
+		deltaBaseCache: cacheObject,
 
 		idx:          NewIndex(0),
 		offsetToType: make(map[int64]plumbing.ObjectType),
@@ -404,19 +417,19 @@ func (d *Decoder) fillOFSDeltaObjectContent(obj plumbing.EncodedObject, offset i
 }
 
 func (d *Decoder) cacheGet(h plumbing.Hash) (plumbing.EncodedObject, bool) {
-	if d.DeltaBaseCache == nil {
+	if d.deltaBaseCache == nil {
 		return nil, false
 	}
 
-	return d.DeltaBaseCache.Get(h)
+	return d.deltaBaseCache.Get(h)
 }
 
 func (d *Decoder) cachePut(obj plumbing.EncodedObject) {
-	if d.DeltaBaseCache == nil {
+	if d.deltaBaseCache == nil {
 		return
 	}
 
-	d.DeltaBaseCache.Put(obj)
+	d.deltaBaseCache.Put(obj)
 }
 
 func (d *Decoder) recallByOffset(o int64) (plumbing.EncodedObject, error) {
diff --git a/plumbing/format/packfile/decoder_test.go b/plumbing/format/packfile/decoder_test.go
index 1a1a74a..b5bc7b7 100644
--- a/plumbing/format/packfile/decoder_test.go
+++ b/plumbing/format/packfile/decoder_test.go
@@ -4,6 +4,7 @@ import (
 	"io"
 
 	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/plumbing/cache"
 	"gopkg.in/src-d/go-git.v4/plumbing/format/idxfile"
 	"gopkg.in/src-d/go-git.v4/plumbing/format/packfile"
 	"gopkg.in/src-d/go-git.v4/plumbing/storer"
@@ -51,7 +52,8 @@ func (s *ReaderSuite) TestDecodeByTypeRefDelta(c *C) {
 
 	storage := memory.NewStorage()
 	scanner := packfile.NewScanner(f.Packfile())
-	d, err := packfile.NewDecoderForType(scanner, storage, plumbing.CommitObject)
+	d, err := packfile.NewDecoderForType(scanner, storage, plumbing.CommitObject,
+		cache.NewObjectLRUDefault())
 	c.Assert(err, IsNil)
 
 	// Index required to decode by ref-delta.
@@ -77,7 +79,8 @@ func (s *ReaderSuite) TestDecodeByTypeRefDeltaError(c *C) {
 	fixtures.Basic().ByTag("ref-delta").Test(c, func(f *fixtures.Fixture) {
 		storage := memory.NewStorage()
 		scanner := packfile.NewScanner(f.Packfile())
-		d, err := packfile.NewDecoderForType(scanner, storage, plumbing.CommitObject)
+		d, err := packfile.NewDecoderForType(scanner, storage,
+			plumbing.CommitObject, cache.NewObjectLRUDefault())
 		c.Assert(err, IsNil)
 
 		defer d.Close()
@@ -111,7 +114,8 @@ func (s *ReaderSuite) TestDecodeByType(c *C) {
 		for _, t := range ts {
 			storage := memory.NewStorage()
 			scanner := packfile.NewScanner(f.Packfile())
-			d, err := packfile.NewDecoderForType(scanner, storage, t)
+			d, err := packfile.NewDecoderForType(scanner, storage, t,
+				cache.NewObjectLRUDefault())
 			c.Assert(err, IsNil)
 
 			// when the packfile is ref-delta based, the offsets are required
@@ -141,13 +145,17 @@ func (s *ReaderSuite) TestDecodeByTypeConstructor(c *C) {
 	storage := memory.NewStorage()
 	scanner := packfile.NewScanner(f.Packfile())
 
-	_, err := packfile.NewDecoderForType(scanner, storage, plumbing.OFSDeltaObject)
+	_, err := packfile.NewDecoderForType(scanner, storage,
+		plumbing.OFSDeltaObject, cache.NewObjectLRUDefault())
 	c.Assert(err, Equals, plumbing.ErrInvalidType)
 
-	_, err = packfile.NewDecoderForType(scanner, storage, plumbing.REFDeltaObject)
+	_, err = packfile.NewDecoderForType(scanner, storage,
+		plumbing.REFDeltaObject, cache.NewObjectLRUDefault())
+
 	c.Assert(err, Equals, plumbing.ErrInvalidType)
 
-	_, err = packfile.NewDecoderForType(scanner, storage, plumbing.InvalidObject)
+	_, err = packfile.NewDecoderForType(scanner, storage, plumbing.InvalidObject,
+		cache.NewObjectLRUDefault())
 	c.Assert(err, Equals, plumbing.ErrInvalidType)
 }
 
@@ -313,7 +321,8 @@ func (s *ReaderSuite) TestDecodeObjectAt(c *C) {
 func (s *ReaderSuite) TestDecodeObjectAtForType(c *C) {
 	f := fixtures.Basic().One()
 	scanner := packfile.NewScanner(f.Packfile())
-	d, err := packfile.NewDecoderForType(scanner, nil, plumbing.TreeObject)
+	d, err := packfile.NewDecoderForType(scanner, nil, plumbing.TreeObject,
+		cache.NewObjectLRUDefault())
 	c.Assert(err, IsNil)
 
 	// when the packfile is ref-delta based, the offsets are required