Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

storage/filesystem: keep packs open in PackfileIter #962

Merged
merged 2 commits into from
Sep 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions storage/filesystem/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,10 @@ func (s *ObjectStorage) IterEncodedObjects(t plumbing.ObjectType) (storer.Encode
return storer.NewMultiEncodedObjectIter(iters), nil
}

func (s *ObjectStorage) buildPackfileIters(t plumbing.ObjectType, seen map[plumbing.Hash]struct{}) (storer.EncodedObjectIter, error) {
func (s *ObjectStorage) buildPackfileIters(
t plumbing.ObjectType,
seen map[plumbing.Hash]struct{},
) (storer.EncodedObjectIter, error) {
if err := s.requireIndex(); err != nil {
return nil, err
}
Expand All @@ -412,7 +415,10 @@ func (s *ObjectStorage) buildPackfileIters(t plumbing.ObjectType, seen map[plumb
if err != nil {
return nil, err
}
return newPackfileIter(s.dir.Fs(), pack, t, seen, s.index[h], s.deltaBaseCache)
return newPackfileIter(
s.dir.Fs(), pack, t, seen, s.index[h],
s.deltaBaseCache, s.options.KeepDescriptors,
)
},
}, nil
}
Expand Down Expand Up @@ -473,16 +479,21 @@ type packfileIter struct {
pack billy.File
iter storer.EncodedObjectIter
seen map[plumbing.Hash]struct{}

// tells whether the pack file should be left open after iteration or not
keepPack bool
}

// NewPackfileIter returns a new EncodedObjectIter for the provided packfile
// and object type. Packfile and index file will be closed after they're
// used.
// used. If keepPack is true the packfile won't be closed after the iteration
// finished.
func NewPackfileIter(
fs billy.Filesystem,
f billy.File,
idxFile billy.File,
t plumbing.ObjectType,
keepPack bool,
) (storer.EncodedObjectIter, error) {
idx := idxfile.NewMemoryIndex()
if err := idxfile.NewDecoder(idxFile).Decode(idx); err != nil {
Expand All @@ -493,7 +504,8 @@ func NewPackfileIter(
return nil, err
}

return newPackfileIter(fs, f, t, make(map[plumbing.Hash]struct{}), idx, nil)
seen := make(map[plumbing.Hash]struct{})
return newPackfileIter(fs, f, t, seen, idx, nil, keepPack)
}

func newPackfileIter(
Expand All @@ -503,6 +515,7 @@ func newPackfileIter(
seen map[plumbing.Hash]struct{},
index idxfile.Index,
cache cache.Object,
keepPack bool,
) (storer.EncodedObjectIter, error) {
var p *packfile.Packfile
if cache != nil {
Expand All @@ -517,9 +530,10 @@ func newPackfileIter(
}

return &packfileIter{
pack: f,
iter: iter,
seen: seen,
pack: f,
iter: iter,
seen: seen,
keepPack: keepPack,
}, nil
}

Expand Down Expand Up @@ -557,7 +571,9 @@ func (iter *packfileIter) ForEach(cb func(plumbing.EncodedObject) error) error {

func (iter *packfileIter) Close() {
iter.iter.Close()
_ = iter.pack.Close()
if !iter.keepPack {
_ = iter.pack.Close()
}
}

type objectsIter struct {
Expand Down
44 changes: 40 additions & 4 deletions storage/filesystem/object_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,18 +153,54 @@ func (s *FsSuite) TestPackfileIter(c *C) {
idxf, err := dg.ObjectPackIdx(h)
c.Assert(err, IsNil)

iter, err := NewPackfileIter(fs, f, idxf, t)
iter, err := NewPackfileIter(fs, f, idxf, t, false)
c.Assert(err, IsNil)

err = iter.ForEach(func(o plumbing.EncodedObject) error {
c.Assert(o.Type(), Equals, t)
return nil
})

c.Assert(err, IsNil)
}
}
})
}

func (s *FsSuite) TestPackfileIterKeepDescriptors(c *C) {
fixtures.ByTag(".git").Test(c, func(f *fixtures.Fixture) {
fs := f.DotGit()
ops := dotgit.Options{KeepDescriptors: true}
dg := dotgit.NewWithOptions(fs, ops)

for _, t := range objectTypes {
ph, err := dg.ObjectPacks()
c.Assert(err, IsNil)

for _, h := range ph {
f, err := dg.ObjectPack(h)
c.Assert(err, IsNil)

idxf, err := dg.ObjectPackIdx(h)
c.Assert(err, IsNil)

iter, err := NewPackfileIter(fs, f, idxf, t, true)
c.Assert(err, IsNil)

err = iter.ForEach(func(o plumbing.EncodedObject) error {
c.Assert(o.Type(), Equals, t)
return nil
})
c.Assert(err, IsNil)

// test twice to check that packfiles are not closed
err = iter.ForEach(func(o plumbing.EncodedObject) error {
c.Assert(o.Type(), Equals, t)
return nil
})
c.Assert(err, IsNil)
}
}
})
}

func BenchmarkPackfileIter(b *testing.B) {
Expand Down Expand Up @@ -201,7 +237,7 @@ func BenchmarkPackfileIter(b *testing.B) {
b.Fatal(err)
}

iter, err := NewPackfileIter(fs, f, idxf, t)
iter, err := NewPackfileIter(fs, f, idxf, t, false)
if err != nil {
b.Fatal(err)
}
Expand Down Expand Up @@ -257,7 +293,7 @@ func BenchmarkPackfileIterReadContent(b *testing.B) {
b.Fatal(err)
}

iter, err := NewPackfileIter(fs, f, idxf, t)
iter, err := NewPackfileIter(fs, f, idxf, t, false)
if err != nil {
b.Fatal(err)
}
Expand Down