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

Clean reconstructed objects outside pack window #716

Merged
merged 1 commit into from
Jan 15, 2018
Merged

Clean reconstructed objects outside pack window #716

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 19 additions & 13 deletions plumbing/format/packfile/delta_selector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,16 @@ func (dw *deltaSelector) walk(
) error {
indexMap := make(map[plumbing.Hash]*deltaIndex)
for i := 0; i < len(objectsToPack); i++ {
// Clean up the index map for anything outside our pack
// window, to save memory.
// Clean up the index map and reconstructed delta objects for anything
// outside our pack window, to save memory.
if i > int(packWindow) {
delete(indexMap, objectsToPack[i-int(packWindow)].Hash())
obj := objectsToPack[i-int(packWindow)]

delete(indexMap, obj.Hash())

if obj.IsDelta() {
obj.Original = nil
}
}

target := objectsToPack[i]
Expand Down Expand Up @@ -261,6 +267,16 @@ func (dw *deltaSelector) walk(
}

func (dw *deltaSelector) tryToDeltify(indexMap map[plumbing.Hash]*deltaIndex, base, target *ObjectToPack) error {
// Original object might not be present if we're reusing a delta, so we
// ensure it is restored.
if err := dw.restoreOriginal(target); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this making it actually slower than before (even if it uses way less memory)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just run borges pack with repository https://github.com/numpy/numpy (downloaded locally) and the results are quite close.

  • Original version:
    • 203.83user 12.34system 2:28.80elapsed 2.4 Gb RAM
    • 204.40user 12.27system 2:29.30elapsed 2.4 Gb RAM
  • With proposed changes:
    • 217.82user 11.97system 2:25.72elapsed 1 Gb RAM
    • 220.06user 11.81system 2:26.28elapsed 1 Gb RAM

It seems that it's a bit slower (user time is bigger) but it spends more system time. Maybe heap allocation.

return err
}

if err := dw.restoreOriginal(base); err != nil {
return err
}

// If the sizes are radically different, this is a bad pairing.
if target.Size() < base.Size()>>4 {
return nil
Expand All @@ -283,16 +299,6 @@ func (dw *deltaSelector) tryToDeltify(indexMap map[plumbing.Hash]*deltaIndex, ba
return nil
}

// Original object might not be present if we're reusing a delta, so we
// ensure it is restored.
if err := dw.restoreOriginal(target); err != nil {
return err
}

if err := dw.restoreOriginal(base); err != nil {
return err
}

if _, ok := indexMap[base.Hash()]; !ok {
indexMap[base.Hash()] = new(deltaIndex)
}
Expand Down