From 56c5e91b158bc4569b38bfd5d27d4b4be5e06a27 Mon Sep 17 00:00:00 2001 From: Miguel Molina Date: Thu, 9 Aug 2018 16:53:00 +0200 Subject: plumbing: packfile, open and close packfile on FSObject reads Signed-off-by: Miguel Molina --- plumbing/format/packfile/encoder_advanced_test.go | 7 ++- plumbing/format/packfile/encoder_test.go | 7 ++- plumbing/format/packfile/fsobject.go | 68 +++++++++++++++++----- plumbing/format/packfile/packfile.go | 69 +++++++++++++---------- plumbing/format/packfile/packfile_test.go | 31 +++++++--- plumbing/object/blob_test.go | 7 --- storage/filesystem/dotgit/dotgit.go | 5 ++ storage/filesystem/object.go | 15 ++--- storage/filesystem/object_test.go | 40 ++++++++++++- 9 files changed, 174 insertions(+), 75 deletions(-) diff --git a/plumbing/format/packfile/encoder_advanced_test.go b/plumbing/format/packfile/encoder_advanced_test.go index 78ddc45..fc1419e 100644 --- a/plumbing/format/packfile/encoder_advanced_test.go +++ b/plumbing/format/packfile/encoder_advanced_test.go @@ -6,7 +6,7 @@ import ( "math/rand" "testing" - "gopkg.in/src-d/go-billy.v3/memfs" + "gopkg.in/src-d/go-billy.v4/memfs" "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/format/idxfile" . "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" @@ -84,7 +84,8 @@ func (s *EncoderAdvancedSuite) testEncodeDecode( encodeHash, err := enc.Encode(hashes, packWindow) c.Assert(err, IsNil) - f, err := memfs.New().Create("packfile") + fs := memfs.New() + f, err := fs.Create("packfile") c.Assert(err, IsNil) _, err = f.Write(buf.Bytes()) @@ -105,7 +106,7 @@ func (s *EncoderAdvancedSuite) testEncodeDecode( _, err = f.Seek(0, io.SeekStart) c.Assert(err, IsNil) - p := NewPackfile(index, f) + p := NewPackfile(index, fs, f) decodeHash, err := p.ID() c.Assert(err, IsNil) diff --git a/plumbing/format/packfile/encoder_test.go b/plumbing/format/packfile/encoder_test.go index 24e2082..80b916d 100644 --- a/plumbing/format/packfile/encoder_test.go +++ b/plumbing/format/packfile/encoder_test.go @@ -5,7 +5,7 @@ import ( "io" stdioutil "io/ioutil" - "gopkg.in/src-d/go-billy.v3/memfs" + "gopkg.in/src-d/go-billy.v4/memfs" "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/format/idxfile" "gopkg.in/src-d/go-git.v4/storage/memory" @@ -290,7 +290,8 @@ func objectsEqual(c *C, o1, o2 plumbing.EncodedObject) { } func packfileFromReader(c *C, buf *bytes.Buffer) (*Packfile, func()) { - file, err := memfs.New().Create("packfile") + fs := memfs.New() + file, err := fs.Create("packfile") c.Assert(err, IsNil) _, err = file.Write(buf.Bytes()) @@ -311,7 +312,7 @@ func packfileFromReader(c *C, buf *bytes.Buffer) (*Packfile, func()) { index, err := w.Index() c.Assert(err, IsNil) - return NewPackfile(index, file), func() { + return NewPackfile(index, fs, file), func() { c.Assert(file.Close(), IsNil) } } diff --git a/plumbing/format/packfile/fsobject.go b/plumbing/format/packfile/fsobject.go index d63127e..6fd3ca5 100644 --- a/plumbing/format/packfile/fsobject.go +++ b/plumbing/format/packfile/fsobject.go @@ -3,17 +3,23 @@ package packfile import ( "io" + billy "gopkg.in/src-d/go-billy.v4" "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" ) // FSObject is an object from the packfile on the filesystem. type FSObject struct { - hash plumbing.Hash - h *ObjectHeader - offset int64 - size int64 - typ plumbing.ObjectType - packfile *Packfile + hash plumbing.Hash + h *ObjectHeader + offset int64 + size int64 + typ plumbing.ObjectType + index idxfile.Index + fs billy.Filesystem + path string + cache cache.Object } // NewFSObject creates a new filesystem object. @@ -22,20 +28,42 @@ func NewFSObject( finalType plumbing.ObjectType, offset int64, contentSize int64, - packfile *Packfile, + index idxfile.Index, + fs billy.Filesystem, + path string, + cache cache.Object, ) *FSObject { return &FSObject{ - hash: hash, - offset: offset, - size: contentSize, - typ: finalType, - packfile: packfile, + hash: hash, + offset: offset, + size: contentSize, + typ: finalType, + index: index, + fs: fs, + path: path, + cache: cache, } } // Reader implements the plumbing.EncodedObject interface. func (o *FSObject) Reader() (io.ReadCloser, error) { - return o.packfile.getObjectContent(o.offset) + f, err := o.fs.Open(o.path) + if err != nil { + return nil, err + } + + p := NewPackfileWithCache(o.index, nil, f, o.cache) + r, err := p.getObjectContent(o.offset) + if err != nil { + _ = f.Close() + return nil, err + } + + if err := f.Close(); err != nil { + return nil, err + } + + return r, nil } // SetSize implements the plumbing.EncodedObject interface. This method @@ -62,3 +90,17 @@ func (o *FSObject) Type() plumbing.ObjectType { func (o *FSObject) Writer() (io.WriteCloser, error) { return nil, nil } + +type objectReader struct { + io.ReadCloser + f billy.File +} + +func (r *objectReader) Close() error { + if err := r.ReadCloser.Close(); err != nil { + _ = r.f.Close() + return err + } + + return r.f.Close() +} diff --git a/plumbing/format/packfile/packfile.go b/plumbing/format/packfile/packfile.go index df8a3d4..5feb781 100644 --- a/plumbing/format/packfile/packfile.go +++ b/plumbing/format/packfile/packfile.go @@ -3,9 +3,9 @@ package packfile import ( "bytes" "io" - stdioutil "io/ioutil" "os" + billy "gopkg.in/src-d/go-billy.v4" "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" @@ -24,21 +24,26 @@ var ( // Packfile allows retrieving information from inside a packfile. type Packfile struct { idxfile.Index - file io.ReadSeeker + fs billy.Filesystem + file billy.File s *Scanner deltaBaseCache cache.Object offsetToType map[int64]plumbing.ObjectType } // NewPackfileWithCache creates a new Packfile with the given object cache. +// If the filesystem is provided, the packfile will return FSObjects, otherwise +// it will return MemoryObjects. func NewPackfileWithCache( index idxfile.Index, - file io.ReadSeeker, + fs billy.Filesystem, + file billy.File, cache cache.Object, ) *Packfile { s := NewScanner(file) return &Packfile{ index, + fs, file, s, cache, @@ -48,8 +53,10 @@ func NewPackfileWithCache( // NewPackfile returns a packfile representation for the given packfile file // and packfile idx. -func NewPackfile(index idxfile.Index, file io.ReadSeeker) *Packfile { - return NewPackfileWithCache(index, file, cache.NewObjectLRUDefault()) +// If the filesystem is provided, the packfile will return FSObjects, otherwise +// it will return MemoryObjects. +func NewPackfile(index idxfile.Index, fs billy.Filesystem, file billy.File) *Packfile { + return NewPackfileWithCache(index, fs, file, cache.NewObjectLRUDefault()) } // Get retrieves the encoded object in the packfile with the given hash. @@ -215,6 +222,12 @@ func (p *Packfile) nextObject() (plumbing.EncodedObject, error) { return nil, err } + // If we have no filesystem, we will return a MemoryObject instead + // of an FSObject. + if p.fs == nil { + return p.getNextObject(h) + } + hash, err := p.FindHash(h.Offset) if err != nil { return nil, err @@ -232,7 +245,16 @@ func (p *Packfile) nextObject() (plumbing.EncodedObject, error) { p.offsetToType[h.Offset] = typ - return NewFSObject(hash, typ, h.Offset, size, p), nil + return NewFSObject( + hash, + typ, + h.Offset, + size, + p.Index, + p.fs, + p.file.Name(), + p.deltaBaseCache, + ), nil } func (p *Packfile) getObjectContent(offset int64) (io.ReadCloser, error) { @@ -245,10 +267,20 @@ func (p *Packfile) getObjectContent(offset int64) (io.ReadCloser, error) { return nil, err } + obj, err := p.getNextObject(h) + if err != nil { + return nil, err + } + + return obj.Reader() +} + +func (p *Packfile) getNextObject(h *ObjectHeader) (plumbing.EncodedObject, error) { var obj = new(plumbing.MemoryObject) obj.SetSize(h.Length) obj.SetType(h.Type) + var err error switch h.Type { case plumbing.CommitObject, plumbing.TreeObject, plumbing.BlobObject, plumbing.TagObject: err = p.fillRegularObjectContent(obj) @@ -264,7 +296,7 @@ func (p *Packfile) getObjectContent(offset int64) (io.ReadCloser, error) { return nil, err } - return obj.Reader() + return obj, nil } func (p *Packfile) fillRegularObjectContent(obj plumbing.EncodedObject) error { @@ -410,29 +442,6 @@ func (p *Packfile) Close() error { return closer.Close() } -// MemoryObjectFromDisk converts a FSObject to a MemoryObject. -func MemoryObjectFromDisk(obj plumbing.EncodedObject) (plumbing.EncodedObject, error) { - o2 := new(plumbing.MemoryObject) - o2.SetType(obj.Type()) - o2.SetSize(obj.Size()) - - r, err := obj.Reader() - if err != nil { - return nil, err - } - - data, err := stdioutil.ReadAll(r) - if err != nil { - return nil, err - } - - if _, err := o2.Write(data); err != nil { - return nil, err - } - - return o2, nil -} - type objectIter struct { p *Packfile typ plumbing.ObjectType diff --git a/plumbing/format/packfile/packfile_test.go b/plumbing/format/packfile/packfile_test.go index 3193bed..05dc8a7 100644 --- a/plumbing/format/packfile/packfile_test.go +++ b/plumbing/format/packfile/packfile_test.go @@ -109,13 +109,14 @@ var expectedEntries = map[plumbing.Hash]int64{ func (s *PackfileSuite) SetUpTest(c *C) { s.f = fixtures.Basic().One() - f, err := osfs.New("").Open(s.f.Packfile().Name()) + fs := osfs.New("") + f, err := fs.Open(s.f.Packfile().Name()) c.Assert(err, IsNil) s.idx = idxfile.NewMemoryIndex() c.Assert(idxfile.NewDecoder(s.f.Idx()).Decode(s.idx), IsNil) - s.p = packfile.NewPackfile(s.idx, f) + s.p = packfile.NewPackfile(s.idx, fs, f) } func (s *PackfileSuite) TearDownTest(c *C) { @@ -125,7 +126,11 @@ func (s *PackfileSuite) TearDownTest(c *C) { func (s *PackfileSuite) TestDecode(c *C) { fixtures.Basic().ByTag("packfile").Test(c, func(f *fixtures.Fixture) { index := getIndexFromIdxFile(f.Idx()) - p := packfile.NewPackfile(index, f.Packfile()) + fs := osfs.New("") + pf, err := fs.Open(f.Packfile().Name()) + c.Assert(err, IsNil) + + p := packfile.NewPackfile(index, fs, pf) defer p.Close() for _, h := range expectedHashes { @@ -140,7 +145,11 @@ func (s *PackfileSuite) TestDecodeByTypeRefDelta(c *C) { f := fixtures.Basic().ByTag("ref-delta").One() index := getIndexFromIdxFile(f.Idx()) - packfile := packfile.NewPackfile(index, f.Packfile()) + fs := osfs.New("") + pf, err := fs.Open(f.Packfile().Name()) + c.Assert(err, IsNil) + + packfile := packfile.NewPackfile(index, fs, pf) defer packfile.Close() iter, err := packfile.GetByType(plumbing.CommitObject) @@ -171,7 +180,11 @@ func (s *PackfileSuite) TestDecodeByType(c *C) { fixtures.Basic().ByTag("packfile").Test(c, func(f *fixtures.Fixture) { for _, t := range ts { index := getIndexFromIdxFile(f.Idx()) - packfile := packfile.NewPackfile(index, f.Packfile()) + fs := osfs.New("") + pf, err := fs.Open(f.Packfile().Name()) + c.Assert(err, IsNil) + + packfile := packfile.NewPackfile(index, fs, pf) defer packfile.Close() iter, err := packfile.GetByType(t) @@ -188,10 +201,14 @@ func (s *PackfileSuite) TestDecodeByType(c *C) { func (s *PackfileSuite) TestDecodeByTypeConstructor(c *C) { f := fixtures.Basic().ByTag("packfile").One() index := getIndexFromIdxFile(f.Idx()) - packfile := packfile.NewPackfile(index, f.Packfile()) + fs := osfs.New("") + pf, err := fs.Open(f.Packfile().Name()) + c.Assert(err, IsNil) + + packfile := packfile.NewPackfile(index, fs, pf) defer packfile.Close() - _, err := packfile.GetByType(plumbing.OFSDeltaObject) + _, err = packfile.GetByType(plumbing.OFSDeltaObject) c.Assert(err, Equals, plumbing.ErrInvalidType) _, err = packfile.GetByType(plumbing.REFDeltaObject) diff --git a/plumbing/object/blob_test.go b/plumbing/object/blob_test.go index e08ff25..181436d 100644 --- a/plumbing/object/blob_test.go +++ b/plumbing/object/blob_test.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "gopkg.in/src-d/go-git.v4/plumbing" - "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" . "gopkg.in/check.v1" ) @@ -71,12 +70,6 @@ func (s *BlobsSuite) TestBlobIter(c *C) { blobs := []*Blob{} iter.ForEach(func(b *Blob) error { - var err error - b.obj, err = packfile.MemoryObjectFromDisk(b.obj) - if err != nil { - return err - } - blobs = append(blobs, b) return nil }) diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index dc12f23..af07eb5 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -784,6 +784,11 @@ func (d *DotGit) Alternates() ([]*DotGit, error) { return alternates, nil } +// Fs returns the underlying filesystem of the DotGit folder. +func (d *DotGit) Fs() billy.Filesystem { + return d.fs +} + func isHex(s string) bool { for _, b := range []byte(s) { if isNum(b) { diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go index 86d0da9..6958e32 100644 --- a/storage/filesystem/object.go +++ b/storage/filesystem/object.go @@ -295,12 +295,7 @@ func (s *ObjectStorage) decodeObjectAt( return nil, err } - obj, err := packfile.NewPackfile(idx, f).GetByOffset(offset) - if err != nil { - return nil, err - } - - return packfile.MemoryObjectFromDisk(obj) + return packfile.NewPackfile(idx, s.dir.Fs(), f).GetByOffset(offset) } func (s *ObjectStorage) decodeDeltaObjectAt( @@ -404,7 +399,7 @@ func (s *ObjectStorage) buildPackfileIters(t plumbing.ObjectType, seen map[plumb if err != nil { return nil, err } - return newPackfileIter(pack, t, seen, s.index[h], s.deltaBaseCache) + return newPackfileIter(s.dir.Fs(), pack, t, seen, s.index[h], s.deltaBaseCache) }, }, nil } @@ -466,6 +461,7 @@ type packfileIter struct { // and object type. Packfile and index file will be closed after they're // used. func NewPackfileIter( + fs billy.Filesystem, f billy.File, idxFile billy.File, t plumbing.ObjectType, @@ -479,17 +475,18 @@ func NewPackfileIter( return nil, err } - return newPackfileIter(f, t, make(map[plumbing.Hash]struct{}), idx, nil) + return newPackfileIter(fs, f, t, make(map[plumbing.Hash]struct{}), idx, nil) } func newPackfileIter( + fs billy.Filesystem, f billy.File, t plumbing.ObjectType, seen map[plumbing.Hash]struct{}, index idxfile.Index, cache cache.Object, ) (storer.EncodedObjectIter, error) { - iter, err := packfile.NewPackfile(index, f).GetByType(t) + iter, err := packfile.NewPackfile(index, fs, f).GetByType(t) if err != nil { return nil, err } diff --git a/storage/filesystem/object_test.go b/storage/filesystem/object_test.go index 88f22bf..b1408b7 100644 --- a/storage/filesystem/object_test.go +++ b/storage/filesystem/object_test.go @@ -121,7 +121,7 @@ func (s *FsSuite) TestPackfileIter(c *C) { idxf, err := dg.ObjectPackIdx(h) c.Assert(err, IsNil) - iter, err := NewPackfileIter(f, idxf, t) + iter, err := NewPackfileIter(fs, f, idxf, t) c.Assert(err, IsNil) err = iter.ForEach(func(o plumbing.EncodedObject) error { c.Assert(o.Type(), Equals, t) @@ -169,7 +169,7 @@ func BenchmarkPackfileIter(b *testing.B) { b.Fatal(err) } - iter, err := NewPackfileIter(f, idxf, t) + iter, err := NewPackfileIter(fs, f, idxf, t) if err != nil { b.Fatal(err) } @@ -225,7 +225,7 @@ func BenchmarkPackfileIterReadContent(b *testing.B) { b.Fatal(err) } - iter, err := NewPackfileIter(f, idxf, t) + iter, err := NewPackfileIter(fs, f, idxf, t) if err != nil { b.Fatal(err) } @@ -256,3 +256,37 @@ func BenchmarkPackfileIterReadContent(b *testing.B) { }) } } + +func BenchmarkGetObjectFromPackfile(b *testing.B) { + if err := fixtures.Init(); err != nil { + b.Fatal(err) + } + + defer func() { + if err := fixtures.Clean(); err != nil { + b.Fatal(err) + } + }() + + for _, f := range fixtures.Basic() { + b.Run(f.URL, func(b *testing.B) { + fs := f.DotGit() + o, err := NewObjectStorage(dotgit.New(fs)) + if err != nil { + b.Fatal(err) + } + + for i := 0; i < b.N; i++ { + expected := plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5") + obj, err := o.EncodedObject(plumbing.AnyObject, expected) + if err != nil { + b.Fatal(err) + } + + if obj.Hash() != expected { + b.Errorf("expecting %s, got %s", expected, obj.Hash()) + } + } + }) + } +} -- cgit