Merge pull request #587 from keybase/strib/skip-compression-gh-master
config: support a configurable, and turn-off-able, pack.window
diff --git a/config/config.go b/config/config.go
index 475045e..477eb35 100644
--- a/config/config.go
+++ b/config/config.go
@@ -6,6 +6,7 @@
"errors"
"fmt"
"sort"
+ "strconv"
format "gopkg.in/src-d/go-git.v4/plumbing/format/config"
)
@@ -40,6 +41,14 @@
// Worktree is the path to the root of the working tree.
Worktree string
}
+
+ Pack struct {
+ // Window controls the size of the sliding window for delta
+ // compression. The default is 10. A value of 0 turns off
+ // delta compression entirely.
+ Window uint
+ }
+
// Remotes list of repository remotes, the key of the map is the name
// of the remote, should equal to RemoteConfig.Name.
Remotes map[string]*RemoteConfig
@@ -81,10 +90,14 @@
remoteSection = "remote"
submoduleSection = "submodule"
coreSection = "core"
+ packSection = "pack"
fetchKey = "fetch"
urlKey = "url"
bareKey = "bare"
worktreeKey = "worktree"
+ windowKey = "window"
+
+ defaultPackWindow = uint(10)
)
// Unmarshal parses a git-config file and stores it.
@@ -98,6 +111,9 @@
}
c.unmarshalCore()
+ if err := c.unmarshalPack(); err != nil {
+ return err
+ }
c.unmarshalSubmodules()
return c.unmarshalRemotes()
}
@@ -111,6 +127,21 @@
c.Core.Worktree = s.Options.Get(worktreeKey)
}
+func (c *Config) unmarshalPack() error {
+ s := c.Raw.Section(packSection)
+ window := s.Options.Get(windowKey)
+ if window == "" {
+ c.Pack.Window = defaultPackWindow
+ } else {
+ winUint, err := strconv.ParseUint(window, 10, 32)
+ if err != nil {
+ return err
+ }
+ c.Pack.Window = uint(winUint)
+ }
+ return nil
+}
+
func (c *Config) unmarshalRemotes() error {
s := c.Raw.Section(remoteSection)
for _, sub := range s.Subsections {
@@ -138,6 +169,7 @@
// Marshal returns Config encoded as a git-config file.
func (c *Config) Marshal() ([]byte, error) {
c.marshalCore()
+ c.marshalPack()
c.marshalRemotes()
c.marshalSubmodules()
@@ -158,6 +190,13 @@
}
}
+func (c *Config) marshalPack() {
+ s := c.Raw.Section(packSection)
+ if c.Pack.Window != defaultPackWindow {
+ s.SetOption(windowKey, fmt.Sprintf("%d", c.Pack.Window))
+ }
+}
+
func (c *Config) marshalRemotes() {
s := c.Raw.Section(remoteSection)
newSubsections := make(format.Subsections, 0, len(c.Remotes))
diff --git a/config/config_test.go b/config/config_test.go
index c27ee26..019cee6 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -10,6 +10,8 @@
input := []byte(`[core]
bare = true
worktree = foo
+[pack]
+ window = 20
[remote "origin"]
url = git@github.com:mcuadros/go-git.git
fetch = +refs/heads/*:refs/remotes/origin/*
@@ -33,6 +35,7 @@
c.Assert(cfg.Core.IsBare, Equals, true)
c.Assert(cfg.Core.Worktree, Equals, "foo")
+ c.Assert(cfg.Pack.Window, Equals, uint(20))
c.Assert(cfg.Remotes, HasLen, 2)
c.Assert(cfg.Remotes["origin"].Name, Equals, "origin")
c.Assert(cfg.Remotes["origin"].URLs, DeepEquals, []string{"git@github.com:mcuadros/go-git.git"})
@@ -51,6 +54,8 @@
output := []byte(`[core]
bare = true
worktree = bar
+[pack]
+ window = 20
[remote "alt"]
url = git@github.com:mcuadros/go-git.git
url = git@github.com:src-d/go-git.git
@@ -65,6 +70,7 @@
cfg := NewConfig()
cfg.Core.IsBare = true
cfg.Core.Worktree = "bar"
+ cfg.Pack.Window = 20
cfg.Remotes["origin"] = &RemoteConfig{
Name: "origin",
URLs: []string{"git@github.com:mcuadros/go-git.git"},
@@ -92,6 +98,8 @@
bare = true
worktree = foo
custom = ignored
+[pack]
+ window = 20
[remote "origin"]
url = git@github.com:mcuadros/go-git.git
fetch = +refs/heads/*:refs/remotes/origin/*
diff --git a/plumbing/format/packfile/delta_selector.go b/plumbing/format/packfile/delta_selector.go
index 0b3539d..77573ac 100644
--- a/plumbing/format/packfile/delta_selector.go
+++ b/plumbing/format/packfile/delta_selector.go
@@ -9,9 +9,6 @@
)
const (
- // How far back in the sorted list to search for deltas. 10 is
- // the default in command line git.
- deltaWindowSize = 10
// deltas based on deltas, how many steps we can do.
// 50 is the default value used in JGit
maxDepth = int64(50)
@@ -31,14 +28,24 @@
return &deltaSelector{s}
}
-// ObjectsToPack creates a list of ObjectToPack from the hashes provided,
-// creating deltas if it's suitable, using an specific internal logic
-func (dw *deltaSelector) ObjectsToPack(hashes []plumbing.Hash) ([]*ObjectToPack, error) {
- otp, err := dw.objectsToPack(hashes)
+// ObjectsToPack creates a list of ObjectToPack from the hashes
+// provided, creating deltas if it's suitable, using an specific
+// internal logic. `packWindow` specifies the size of the sliding
+// window used to compare objects for delta compression; 0 turns off
+// delta compression entirely.
+func (dw *deltaSelector) ObjectsToPack(
+ hashes []plumbing.Hash,
+ packWindow uint,
+) ([]*ObjectToPack, error) {
+ otp, err := dw.objectsToPack(hashes, packWindow)
if err != nil {
return nil, err
}
+ if packWindow == 0 {
+ return otp, nil
+ }
+
dw.sort(otp)
var objectGroups [][]*ObjectToPack
@@ -60,7 +67,7 @@
objs := objs
wg.Add(1)
go func() {
- if walkErr := dw.walk(objs); walkErr != nil {
+ if walkErr := dw.walk(objs, packWindow); walkErr != nil {
once.Do(func() {
err = walkErr
})
@@ -77,10 +84,19 @@
return otp, nil
}
-func (dw *deltaSelector) objectsToPack(hashes []plumbing.Hash) ([]*ObjectToPack, error) {
+func (dw *deltaSelector) objectsToPack(
+ hashes []plumbing.Hash,
+ packWindow uint,
+) ([]*ObjectToPack, error) {
var objectsToPack []*ObjectToPack
for _, h := range hashes {
- o, err := dw.encodedDeltaObject(h)
+ var o plumbing.EncodedObject
+ var err error
+ if packWindow == 0 {
+ o, err = dw.encodedObject(h)
+ } else {
+ o, err = dw.encodedDeltaObject(h)
+ }
if err != nil {
return nil, err
}
@@ -93,6 +109,10 @@
objectsToPack = append(objectsToPack, otp)
}
+ if packWindow == 0 {
+ return objectsToPack, nil
+ }
+
if err := dw.fixAndBreakChains(objectsToPack); err != nil {
return nil, err
}
@@ -201,7 +221,10 @@
sort.Sort(byTypeAndSize(objectsToPack))
}
-func (dw *deltaSelector) walk(objectsToPack []*ObjectToPack) error {
+func (dw *deltaSelector) walk(
+ objectsToPack []*ObjectToPack,
+ packWindow uint,
+) error {
indexMap := make(map[plumbing.Hash]*deltaIndex)
for i := 0; i < len(objectsToPack); i++ {
target := objectsToPack[i]
@@ -218,7 +241,7 @@
continue
}
- for j := i - 1; j >= 0 && i-j < deltaWindowSize; j-- {
+ for j := i - 1; j >= 0 && i-j < int(packWindow); j-- {
base := objectsToPack[j]
// Objects must use only the same type as their delta base.
// Since objectsToPack is sorted by type and size, once we find
diff --git a/plumbing/format/packfile/delta_selector_test.go b/plumbing/format/packfile/delta_selector_test.go
index ca4a96b..7d7fd0c 100644
--- a/plumbing/format/packfile/delta_selector_test.go
+++ b/plumbing/format/packfile/delta_selector_test.go
@@ -146,7 +146,8 @@
func (s *DeltaSelectorSuite) TestObjectsToPack(c *C) {
// Different type
hashes := []plumbing.Hash{s.hashes["base"], s.hashes["treeType"]}
- otp, err := s.ds.ObjectsToPack(hashes)
+ deltaWindowSize := uint(10)
+ otp, err := s.ds.ObjectsToPack(hashes, deltaWindowSize)
c.Assert(err, IsNil)
c.Assert(len(otp), Equals, 2)
c.Assert(otp[0].Object, Equals, s.store.Objects[s.hashes["base"]])
@@ -154,7 +155,7 @@
// Size radically different
hashes = []plumbing.Hash{s.hashes["bigBase"], s.hashes["target"]}
- otp, err = s.ds.ObjectsToPack(hashes)
+ otp, err = s.ds.ObjectsToPack(hashes, deltaWindowSize)
c.Assert(err, IsNil)
c.Assert(len(otp), Equals, 2)
c.Assert(otp[0].Object, Equals, s.store.Objects[s.hashes["bigBase"]])
@@ -162,7 +163,7 @@
// Delta Size Limit with no best delta yet
hashes = []plumbing.Hash{s.hashes["smallBase"], s.hashes["smallTarget"]}
- otp, err = s.ds.ObjectsToPack(hashes)
+ otp, err = s.ds.ObjectsToPack(hashes, deltaWindowSize)
c.Assert(err, IsNil)
c.Assert(len(otp), Equals, 2)
c.Assert(otp[0].Object, Equals, s.store.Objects[s.hashes["smallBase"]])
@@ -170,7 +171,7 @@
// It will create the delta
hashes = []plumbing.Hash{s.hashes["base"], s.hashes["target"]}
- otp, err = s.ds.ObjectsToPack(hashes)
+ otp, err = s.ds.ObjectsToPack(hashes, deltaWindowSize)
c.Assert(err, IsNil)
c.Assert(len(otp), Equals, 2)
c.Assert(otp[0].Object, Equals, s.store.Objects[s.hashes["target"]])
@@ -185,7 +186,7 @@
s.hashes["o2"],
s.hashes["o3"],
}
- otp, err = s.ds.ObjectsToPack(hashes)
+ otp, err = s.ds.ObjectsToPack(hashes, deltaWindowSize)
c.Assert(err, IsNil)
c.Assert(len(otp), Equals, 3)
c.Assert(otp[0].Object, Equals, s.store.Objects[s.hashes["o1"]])
@@ -201,20 +202,32 @@
// a delta.
hashes = make([]plumbing.Hash, 0, deltaWindowSize+2)
hashes = append(hashes, s.hashes["base"])
- for i := 0; i < deltaWindowSize; i++ {
+ for i := uint(0); i < deltaWindowSize; i++ {
hashes = append(hashes, s.hashes["smallTarget"])
}
hashes = append(hashes, s.hashes["target"])
// Don't sort so we can easily check the sliding window without
// creating a bunch of new objects.
- otp, err = s.ds.objectsToPack(hashes)
+ otp, err = s.ds.objectsToPack(hashes, deltaWindowSize)
c.Assert(err, IsNil)
- err = s.ds.walk(otp)
+ err = s.ds.walk(otp, deltaWindowSize)
c.Assert(err, IsNil)
- c.Assert(len(otp), Equals, deltaWindowSize+2)
+ c.Assert(len(otp), Equals, int(deltaWindowSize)+2)
targetIdx := len(otp) - 1
c.Assert(otp[targetIdx].IsDelta(), Equals, false)
+
+ // Check that no deltas are created, and the objects are unsorted,
+ // if compression is off.
+ hashes = []plumbing.Hash{s.hashes["base"], s.hashes["target"]}
+ otp, err = s.ds.ObjectsToPack(hashes, 0)
+ c.Assert(err, IsNil)
+ c.Assert(len(otp), Equals, 2)
+ c.Assert(otp[0].Object, Equals, s.store.Objects[s.hashes["base"]])
+ c.Assert(otp[0].IsDelta(), Equals, false)
+ c.Assert(otp[1].Original, Equals, s.store.Objects[s.hashes["target"]])
+ c.Assert(otp[1].IsDelta(), Equals, false)
+ c.Assert(otp[1].Depth, Equals, 0)
}
func (s *DeltaSelectorSuite) TestMaxDepth(c *C) {
diff --git a/plumbing/format/packfile/encoder.go b/plumbing/format/packfile/encoder.go
index 1426559..7ee6546 100644
--- a/plumbing/format/packfile/encoder.go
+++ b/plumbing/format/packfile/encoder.go
@@ -14,10 +14,10 @@
// Encoder gets the data from the storage and write it into the writer in PACK
// format
type Encoder struct {
- selector *deltaSelector
- w *offsetWriter
- zw *zlib.Writer
- hasher plumbing.Hasher
+ selector *deltaSelector
+ w *offsetWriter
+ zw *zlib.Writer
+ hasher plumbing.Hasher
// offsets is a map of object hashes to corresponding offsets in the packfile.
// It is used to determine offset of the base of a delta when a OFS_DELTA is
// used.
@@ -45,10 +45,15 @@
}
}
-// Encode creates a packfile containing all the objects referenced in hashes
-// and writes it to the writer in the Encoder.
-func (e *Encoder) Encode(hashes []plumbing.Hash) (plumbing.Hash, error) {
- objects, err := e.selector.ObjectsToPack(hashes)
+// Encode creates a packfile containing all the objects referenced in
+// hashes and writes it to the writer in the Encoder. `packWindow`
+// specifies the size of the sliding window used to compare objects
+// for delta compression; 0 turns off delta compression entirely.
+func (e *Encoder) Encode(
+ hashes []plumbing.Hash,
+ packWindow uint,
+) (plumbing.Hash, error) {
+ objects, err := e.selector.ObjectsToPack(hashes, packWindow)
if err != nil {
return plumbing.ZeroHash, err
}
@@ -137,7 +142,7 @@
// for OFS_DELTA, offset of the base is interpreted as negative offset
// relative to the type-byte of the header of the ofs-delta entry.
- relativeOffset := deltaOffset-baseOffset
+ relativeOffset := deltaOffset - baseOffset
if relativeOffset <= 0 {
return fmt.Errorf("bad offset for OFS_DELTA entry: %d", relativeOffset)
}
diff --git a/plumbing/format/packfile/encoder_advanced_test.go b/plumbing/format/packfile/encoder_advanced_test.go
index d92e2c4..39c0700 100644
--- a/plumbing/format/packfile/encoder_advanced_test.go
+++ b/plumbing/format/packfile/encoder_advanced_test.go
@@ -27,12 +27,23 @@
fixs.Test(c, func(f *fixtures.Fixture) {
storage, err := filesystem.NewStorage(f.DotGit())
c.Assert(err, IsNil)
- s.testEncodeDecode(c, storage)
+ s.testEncodeDecode(c, storage, 10)
})
}
-func (s *EncoderAdvancedSuite) testEncodeDecode(c *C, storage storer.Storer) {
+func (s *EncoderAdvancedSuite) TestEncodeDecodeNoDeltaCompression(c *C) {
+ fixs := fixtures.Basic().ByTag("packfile").ByTag(".git")
+ fixs = append(fixs, fixtures.ByURL("https://github.com/src-d/go-git.git").
+ ByTag("packfile").ByTag(".git").One())
+ fixs.Test(c, func(f *fixtures.Fixture) {
+ storage, err := filesystem.NewStorage(f.DotGit())
+ c.Assert(err, IsNil)
+ s.testEncodeDecode(c, storage, 0)
+ })
+}
+
+func (s *EncoderAdvancedSuite) testEncodeDecode(c *C, storage storer.Storer, packWindow uint) {
objIter, err := storage.IterEncodedObjects(plumbing.AnyObject)
c.Assert(err, IsNil)
@@ -57,7 +68,7 @@
buf := bytes.NewBuffer(nil)
enc := NewEncoder(buf, storage, false)
- _, err = enc.Encode(hashes)
+ _, err = enc.Encode(hashes, packWindow)
c.Assert(err, IsNil)
scanner := NewScanner(buf)
diff --git a/plumbing/format/packfile/encoder_test.go b/plumbing/format/packfile/encoder_test.go
index b5b0c42..2cb9094 100644
--- a/plumbing/format/packfile/encoder_test.go
+++ b/plumbing/format/packfile/encoder_test.go
@@ -26,7 +26,7 @@
}
func (s *EncoderSuite) TestCorrectPackHeader(c *C) {
- hash, err := s.enc.Encode([]plumbing.Hash{})
+ hash, err := s.enc.Encode([]plumbing.Hash{}, 10)
c.Assert(err, IsNil)
hb := [20]byte(hash)
@@ -47,7 +47,7 @@
_, err := s.store.SetEncodedObject(o)
c.Assert(err, IsNil)
- hash, err := s.enc.Encode([]plumbing.Hash{o.Hash()})
+ hash, err := s.enc.Encode([]plumbing.Hash{o.Hash()}, 10)
c.Assert(err, IsNil)
// PACK + VERSION(2) + OBJECT NUMBER(1)
@@ -74,13 +74,13 @@
o.SetType(plumbing.CommitObject)
_, err := s.store.SetEncodedObject(o)
c.Assert(err, IsNil)
- hash, err := s.enc.Encode([]plumbing.Hash{o.Hash()})
+ hash, err := s.enc.Encode([]plumbing.Hash{o.Hash()}, 10)
c.Assert(err, IsNil)
c.Assert(hash.IsZero(), Not(Equals), true)
}
func (s *EncoderSuite) TestHashNotFound(c *C) {
- h, err := s.enc.Encode([]plumbing.Hash{plumbing.NewHash("BAD")})
+ h, err := s.enc.Encode([]plumbing.Hash{plumbing.NewHash("BAD")}, 10)
c.Assert(h, Equals, plumbing.ZeroHash)
c.Assert(err, NotNil)
c.Assert(err, Equals, plumbing.ErrObjectNotFound)
diff --git a/plumbing/transport/server/server.go b/plumbing/transport/server/server.go
index be36de5..f896f7a 100644
--- a/plumbing/transport/server/server.go
+++ b/plumbing/transport/server/server.go
@@ -165,7 +165,8 @@
pr, pw := io.Pipe()
e := packfile.NewEncoder(pw, s.storer, false)
go func() {
- _, err := e.Encode(objs)
+ // TODO: plumb through a pack window.
+ _, err := e.Encode(objs, 10)
pw.CloseWithError(err)
}()
diff --git a/plumbing/transport/test/receive_pack.go b/plumbing/transport/test/receive_pack.go
index d29d9ca..ed0f517 100644
--- a/plumbing/transport/test/receive_pack.go
+++ b/plumbing/transport/test/receive_pack.go
@@ -348,7 +348,7 @@
func (s *ReceivePackSuite) emptyPackfile() io.ReadCloser {
var buf bytes.Buffer
e := packfile.NewEncoder(&buf, memory.NewStorage(), false)
- _, err := e.Encode(nil)
+ _, err := e.Encode(nil, 10)
if err != nil {
panic(err)
}
diff --git a/remote.go b/remote.go
index 34ea7f5..fca539d 100644
--- a/remote.go
+++ b/remote.go
@@ -797,17 +797,21 @@
func pushHashes(
ctx context.Context,
sess transport.ReceivePackSession,
- sto storer.EncodedObjectStorer,
+ s storage.Storer,
req *packp.ReferenceUpdateRequest,
hs []plumbing.Hash,
) (*packp.ReportStatus, error) {
rd, wr := io.Pipe()
req.Packfile = rd
+ config, err := s.Config()
+ if err != nil {
+ return nil, err
+ }
done := make(chan error)
go func() {
- e := packfile.NewEncoder(wr, sto, false)
- if _, err := e.Encode(hs); err != nil {
+ e := packfile.NewEncoder(wr, s, false)
+ if _, err := e.Encode(hs, config.Pack.Window); err != nil {
done <- wr.CloseWithError(err)
return
}
