plumbing/storer: add ExclusiveAccess option to Storer #941
Conversation
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 <[email protected]>
storage/filesystem/dotgit/dotgit.go
Outdated
| // 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 . |
|
I'm also going to move |
options.go
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
I thought about read only but what it really means is that it is not modified from the outside. I've added packfile and hash cache invalidation when new objects or packfiles are created. Mainly because I wanted to pass all tests in this mode. So it is in fact ReadWrite but assumes that only go-git is changing things.
Anyway, I agree that Static may not be the best name but could not find any other that reflected that is not modified from the outside.
storage/filesystem/dotgit/dotgit.go
Outdated
| // 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 { |
Signed-off-by: Javi Fontan <[email protected]>
Signed-off-by: Javi Fontan <[email protected]>
|
Maybe 👍 for this PR speed up merging ;) So we can start extending Storage's Options by adding configurable cache size (src-d/gitbase#440) |
|
@jfontan Just a doubt about naming and behaviour: Does this mean that it will not be modified externally or that it won't be modified even by go-git itself while open? If it's the later, maybe it should be named |
|
@smola The first case. We don't expect it to be modified externally. We can write with go-git (create new packfiles or objects). Maybe |
Signed-off-by: Javi Fontan <[email protected]>
|
@jfontan |
Signed-off-by: Javi Fontan <[email protected]>
jfontan
changed the title
storage/filesystem/storage.go
Outdated
| @@ -11,6 +12,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 { | |||
| storer.Options | |||
There was a problem hiding this comment.
Why is an embedded struct? and not a private field?
There was a problem hiding this comment.
You're right. There's no reason why it should be embedded or public. It's now changed.
Signed-off-by: Javi Fontan <[email protected]>
Signed-off-by: Javi Fontan <[email protected]>
jfontan
force-pushed
the
improvement/static-mode
branch
from
f894cd2
to
e25ac61
Compare
jfontan
changed the title
This functionality was already tested in storage/filesystem. The coverage tool only takes into account files from the same package of the test. Signed-off-by: Javi Fontan <[email protected]>
jfontan
force-pushed
the
improvement/static-mode
branch
from
e25ac61
to
659ec44
Compare
jfontan
changed the title
Adds
ExclusiveAccessconfiguration tofilesystem.Storageandfilesystem.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
ExclusiveAccessoption 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.