Clean reconstructed objects outside pack window #716
Conversation
Object walk reconstructs delta objects but these are not cleaned up after they got out the pack window. Without this change all reconstructed objects reside in memory. restoreOriginal call is moved before calling Size(). Now we can not guarantee that the object is already undeltified. Signed-off-by: Javi Fontan <[email protected]>
| @@ -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 { | |||
There was a problem hiding this comment.
isn't this making it actually slower than before (even if it uses way less memory)?
There was a problem hiding this comment.
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.
|
Then lgtm. That little slowdown is a nice tradeoff for half the memory
|
|
Can we test this with more diverse repository? Not only with the |
|
I've did new tests with the following repos:
The times and memory are only from the push action from local to a new repository. Each test is executed twice and the smaller value for time and memory is selected.
The code to do the benchmark is here: https://gist.github.com/jfontan/42fbfe0761e5280012d285c1a4290f5d |
|
So it's actually faster? And uses half the memory? Nice, great job! |
|
Networking is envolved, so speed number aren't reliable |
|
I've just updated the table with two small repos. |
Object walk reconstructs delta objects but these are not cleaned up
after they got out the pack window. Without this change all
reconstructed objects reside in memory.
restoreOriginal call is moved before calling Size(). Now we can not
guarantee that the object is already undeltified.