From 1e1a7d0623459807d6f1e871492147f971f7540c Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 30 Aug 2018 15:29:51 +0200 Subject: git: add Static option to PlainOpen Also adds Static configuration to Storage and DotGit. This option means that the git repository is not expected to be modified while open and enables some optimizations. Each time a file is accessed the storer tries to open an object file for the requested hash. When this is done for a lot of objects it is expensive. With Static option a list of object files is generated the first time an object is accessed and used to check if exists instead of using system calls. A similar optimization is done for packfiles. Signed-off-by: Javi Fontan --- options.go | 2 + repository.go | 11 ++- repository_test.go | 19 ++++ storage/filesystem/dotgit/dotgit.go | 182 +++++++++++++++++++++++++++++++++++- storage/filesystem/storage.go | 27 +++++- storage/filesystem/storage_test.go | 18 ++++ 6 files changed, 249 insertions(+), 10 deletions(-) diff --git a/options.go b/options.go index 7b1570f..7b55146 100644 --- a/options.go +++ b/options.go @@ -431,6 +431,8 @@ type PlainOpenOptions struct { // DetectDotGit defines whether parent directories should be // walked until a .git directory or file is found. DetectDotGit bool + // Static means that the repository won't be modified while open. + Static bool } // Validate validates the fields and sets the default values. diff --git a/repository.go b/repository.go index 818cfb3..d99d6eb 100644 --- a/repository.go +++ b/repository.go @@ -235,9 +235,8 @@ func PlainOpen(path string) (*Repository, error) { return PlainOpenWithOptions(path, &PlainOpenOptions{}) } -// PlainOpen opens a git repository from the given path. It detects if the -// repository is bare or a normal one. If the path doesn't contain a valid -// repository ErrRepositoryNotExists is returned +// PlainOpenWithOptions opens a git repository from the given path with specific +// options. See PlainOpen for more info. func PlainOpenWithOptions(path string, o *PlainOpenOptions) (*Repository, error) { dot, wt, err := dotGitToOSFilesystems(path, o.DetectDotGit) if err != nil { @@ -252,7 +251,11 @@ func PlainOpenWithOptions(path string, o *PlainOpenOptions) (*Repository, error) return nil, err } - s, err := filesystem.NewStorage(dot) + so := filesystem.StorageOptions{ + Static: o.Static, + } + + s, err := filesystem.NewStorageWithOptions(dot, so) if err != nil { return nil, err } diff --git a/repository_test.go b/repository_test.go index 261af7a..b891413 100644 --- a/repository_test.go +++ b/repository_test.go @@ -550,6 +550,25 @@ func (s *RepositorySuite) TestPlainOpenNotExistsDetectDotGit(c *C) { c.Assert(r, IsNil) } +func (s *RepositorySuite) TestPlainOpenStatic(c *C) { + dir, err := ioutil.TempDir("", "plain-open") + c.Assert(err, IsNil) + defer os.RemoveAll(dir) + + r, err := PlainInit(dir, true) + c.Assert(err, IsNil) + c.Assert(r, NotNil) + + op := &PlainOpenOptions{Static: true} + r, err = PlainOpenWithOptions(dir, op) + c.Assert(err, IsNil) + c.Assert(r, NotNil) + + sto, ok := r.Storer.(*filesystem.Storage) + c.Assert(ok, Equals, true) + c.Assert(sto.StorageOptions.Static, Equals, true) +} + func (s *RepositorySuite) TestPlainClone(c *C) { r, err := PlainClone(c.MkDir(), false, &CloneOptions{ URL: s.GetBasicLocalRepositoryURL(), diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index df4f756..2048ddc 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -60,18 +60,39 @@ var ( // The DotGit type represents a local git repository on disk. This // type is not zero-value-safe, use the New function to initialize it. type DotGit struct { + DotGitOptions fs billy.Filesystem // incoming object directory information incomingChecked bool incomingDirName string + + objectList []plumbing.Hash + objectMap map[plumbing.Hash]struct{} + packList []plumbing.Hash + packMap map[plumbing.Hash]struct{} +} + +// DotGitOptions holds configuration options for new DotGit objects. +type DotGitOptions struct { + // Static means that the filesystem won't be changed while the repo is open. + Static bool } // New returns a DotGit value ready to be used. The path argument must // be the absolute path of a git repository directory (e.g. // "/foo/bar/.git"). func New(fs billy.Filesystem) *DotGit { - return &DotGit{fs: fs} + return NewWithOptions(fs, DotGitOptions{}) +} + +// NewWithOptions creates a new DotGit and sets non default configuration +// options. See New for complete help. +func NewWithOptions(fs billy.Filesystem, o DotGitOptions) *DotGit { + return &DotGit{ + DotGitOptions: o, + fs: fs, + } } // Initialize creates all the folder scaffolding. @@ -143,11 +164,25 @@ func (d *DotGit) Shallow() (billy.File, error) { // NewObjectPack return a writer for a new packfile, it saves the packfile to // disk and also generates and save the index for the given packfile. func (d *DotGit) NewObjectPack() (*PackWriter, error) { + d.cleanPackList() return newPackWrite(d.fs) } // ObjectPacks returns the list of availables packfiles func (d *DotGit) ObjectPacks() ([]plumbing.Hash, error) { + if !d.Static { + return d.objectPacks() + } + + err := d.genPackList() + if err != nil { + return nil, err + } + + return d.packList, nil +} + +func (d *DotGit) objectPacks() ([]plumbing.Hash, error) { packDir := d.fs.Join(objectsPath, packPath) files, err := d.fs.ReadDir(packDir) if err != nil { @@ -181,6 +216,11 @@ func (d *DotGit) objectPackPath(hash plumbing.Hash, extension string) string { } func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.File, error) { + err := d.hasPack(hash) + if err != nil { + return nil, err + } + pack, err := d.fs.Open(d.objectPackPath(hash, extension)) if err != nil { if os.IsNotExist(err) { @@ -195,15 +235,27 @@ func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.Fil // ObjectPack returns a fs.File of the given packfile func (d *DotGit) ObjectPack(hash plumbing.Hash) (billy.File, error) { + err := d.hasPack(hash) + if err != nil { + return nil, err + } + return d.objectPackOpen(hash, `pack`) } // ObjectPackIdx returns a fs.File of the index file for a given packfile func (d *DotGit) ObjectPackIdx(hash plumbing.Hash) (billy.File, error) { + err := d.hasPack(hash) + if err != nil { + return nil, err + } + return d.objectPackOpen(hash, `idx`) } func (d *DotGit) DeleteOldObjectPackAndIndex(hash plumbing.Hash, t time.Time) error { + d.cleanPackList() + path := d.objectPackPath(hash, `pack`) if !t.IsZero() { fi, err := d.fs.Stat(path) @@ -224,12 +276,23 @@ func (d *DotGit) DeleteOldObjectPackAndIndex(hash plumbing.Hash, t time.Time) er // NewObject return a writer for a new object file. func (d *DotGit) NewObject() (*ObjectWriter, error) { + d.cleanObjectList() + return newObjectWriter(d.fs) } // Objects returns a slice with the hashes of objects found under the // .git/objects/ directory. func (d *DotGit) Objects() ([]plumbing.Hash, error) { + if d.Static { + err := d.genObjectList() + if err != nil { + return nil, err + } + + return d.objectList, nil + } + var objects []plumbing.Hash err := d.ForEachObjectHash(func(hash plumbing.Hash) error { objects = append(objects, hash) @@ -241,9 +304,29 @@ func (d *DotGit) Objects() ([]plumbing.Hash, error) { return objects, nil } -// Objects returns a slice with the hashes of objects found under the -// .git/objects/ directory. +// ForEachObjectHash iterates over the hashes of objects found under the +// .git/objects/ directory and executes the provided . func (d *DotGit) ForEachObjectHash(fun func(plumbing.Hash) error) error { + if !d.Static { + return d.forEachObjectHash(fun) + } + + err := d.genObjectList() + if err != nil { + return err + } + + for _, h := range d.objectList { + err := fun(h) + if err != nil { + return err + } + } + + return nil +} + +func (d *DotGit) forEachObjectHash(fun func(plumbing.Hash) error) error { files, err := d.fs.ReadDir(objectsPath) if err != nil { if os.IsNotExist(err) { @@ -278,6 +361,87 @@ func (d *DotGit) ForEachObjectHash(fun func(plumbing.Hash) error) error { return nil } +func (d *DotGit) cleanObjectList() { + d.objectMap = nil + d.objectList = nil +} + +func (d *DotGit) genObjectList() error { + if d.objectMap != nil { + return nil + } + + d.objectMap = make(map[plumbing.Hash]struct{}) + return d.forEachObjectHash(func(h plumbing.Hash) error { + d.objectList = append(d.objectList, h) + d.objectMap[h] = struct{}{} + + return nil + }) +} + +func (d *DotGit) hasObject(h plumbing.Hash) error { + if !d.Static { + return nil + } + + err := d.genObjectList() + if err != nil { + return err + } + + _, ok := d.objectMap[h] + if !ok { + return plumbing.ErrObjectNotFound + } + + return nil +} + +func (d *DotGit) cleanPackList() { + d.packMap = nil + d.packList = nil +} + +func (d *DotGit) genPackList() error { + if d.packMap != nil { + return nil + } + + op, err := d.objectPacks() + if err != nil { + return err + } + + d.packMap = make(map[plumbing.Hash]struct{}) + d.packList = nil + + for _, h := range op { + d.packList = append(d.packList, h) + d.packMap[h] = struct{}{} + } + + return nil +} + +func (d *DotGit) hasPack(h plumbing.Hash) error { + if !d.Static { + return nil + } + + err := d.genPackList() + if err != nil { + return err + } + + _, ok := d.packMap[h] + if !ok { + return ErrPackfileNotFound + } + + return nil +} + func (d *DotGit) objectPath(h plumbing.Hash) string { hash := h.String() return d.fs.Join(objectsPath, hash[0:2], hash[2:40]) @@ -322,6 +486,11 @@ func (d *DotGit) hasIncomingObjects() bool { // Object returns a fs.File pointing the object file, if exists func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { + err := d.hasObject(h) + if err != nil { + return nil, err + } + obj1, err1 := d.fs.Open(d.objectPath(h)) if os.IsNotExist(err1) && d.hasIncomingObjects() { obj2, err2 := d.fs.Open(d.incomingObjectPath(h)) @@ -335,6 +504,11 @@ func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { // ObjectStat returns a os.FileInfo pointing the object file, if exists func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { + err := d.hasObject(h) + if err != nil { + return nil, err + } + obj1, err1 := d.fs.Stat(d.objectPath(h)) if os.IsNotExist(err1) && d.hasIncomingObjects() { obj2, err2 := d.fs.Stat(d.incomingObjectPath(h)) @@ -348,6 +522,8 @@ func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { // ObjectDelete removes the object file, if exists func (d *DotGit) ObjectDelete(h plumbing.Hash) error { + d.cleanObjectList() + err1 := d.fs.Remove(d.objectPath(h)) if os.IsNotExist(err1) && d.hasIncomingObjects() { err2 := d.fs.Remove(d.incomingObjectPath(h)) diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go index 622bb4a..a969a1f 100644 --- a/storage/filesystem/storage.go +++ b/storage/filesystem/storage.go @@ -11,6 +11,8 @@ import ( // standard git format (this is, the .git directory). Zero values of this type // are not safe to use, see the NewStorage function below. type Storage struct { + StorageOptions + fs billy.Filesystem dir *dotgit.DotGit @@ -22,17 +24,36 @@ type Storage struct { ModuleStorage } +// StorageOptions holds configuration for the storage. +type StorageOptions struct { + // Static means that the filesystem is not modified while the repo is open. + Static bool +} + // NewStorage returns a new Storage backed by a given `fs.Filesystem` func NewStorage(fs billy.Filesystem) (*Storage, error) { - dir := dotgit.New(fs) + return NewStorageWithOptions(fs, StorageOptions{}) +} + +// NewStorageWithOptions returns a new Storage backed by a given `fs.Filesystem` +func NewStorageWithOptions( + fs billy.Filesystem, + ops StorageOptions, +) (*Storage, error) { + dOps := dotgit.DotGitOptions{ + Static: ops.Static, + } + + dir := dotgit.NewWithOptions(fs, dOps) o, err := NewObjectStorage(dir) if err != nil { return nil, err } return &Storage{ - fs: fs, - dir: dir, + StorageOptions: ops, + fs: fs, + dir: dir, ObjectStorage: o, ReferenceStorage: ReferenceStorage{dir: dir}, diff --git a/storage/filesystem/storage_test.go b/storage/filesystem/storage_test.go index 4d9ba6f..d7ebf71 100644 --- a/storage/filesystem/storage_test.go +++ b/storage/filesystem/storage_test.go @@ -26,6 +26,10 @@ func (s *StorageSuite) SetUpTest(c *C) { storage, err := NewStorage(osfs.New(s.dir)) c.Assert(err, IsNil) + setUpTest(s, c, storage) +} + +func setUpTest(s *StorageSuite, c *C, storage *Storage) { // ensure that right interfaces are implemented var _ storer.EncodedObjectStorer = storage var _ storer.IndexStorer = storage @@ -51,3 +55,17 @@ func (s *StorageSuite) TestNewStorageShouldNotAddAnyContentsToDir(c *C) { c.Assert(err, IsNil) c.Assert(fis, HasLen, 0) } + +type StorageStaticSuite struct { + StorageSuite +} + +var _ = Suite(&StorageStaticSuite{}) + +func (s *StorageStaticSuite) SetUpTest(c *C) { + s.dir = c.MkDir() + storage, err := NewStorageWithOptions(osfs.New(s.dir), StorageOptions{Static: true}) + c.Assert(err, IsNil) + + setUpTest(&s.StorageSuite, c, storage) +} -- cgit