src-d / go-git

Project has been moved to: https://github.com/go-git/go-git
https://github.com/go-git/go-git
Apache License 2.0
4.9k stars 540 forks source link

feat: improve patch delta performance #1180

Closed orisano closed 5 years ago

orisano commented 5 years ago

ref: #1170

jfontan commented 5 years ago

I've tested with several repositories and time is in the same ballpark as before but somehow uses more memory or is a but slower. This is a similar benchmark as the one provided but downloading a linux repo I have locally and downloading all history:

Before:

BenchmarkBareClone-4           1    202099566609 ns/op  87716304312 B/op    116607058 allocs/op
PASS
ok      gopkg.in/src-d/go-git.v4    202.383s

After:

BenchmarkBareClone-4           1    204590089037 ns/op  95760605528 B/op    105199786 allocs/op
PASS
ok      gopkg.in/src-d/go-git.v4    204.828s

I've also timed the execution of _examples/clone with the linux repository:

Before:

263.33user 11.80system 3:28.19elapsed 132%CPU (0avgtext+0avgdata 3009184maxresident)k
232inputs+3482592outputs (0major+812125minor)pagefaults 0swaps

After:

287.45user 13.48system 3:50.08elapsed 130%CPU (0avgtext+0avgdata 3073260maxresident)k
562240inputs+3482592outputs (0major+828890minor)pagefaults 0swaps

I don't know what can be happening as the changes look seem to improve memory allocation.

orisano commented 5 years ago

Please give me your memory profile.

go test -bench BareClone -benchmem -timeout 10000s -memprofile=mem.pb.gz -cpuprofile=cpu.pb.gz

I can't reproduce ...

jfontan commented 5 years ago

@orisano After your rebase I get similar numbers. These are the results of the tests with linux:

BenchmarkBareClone-4           1    201592414125 ns/op  95829895744 B/op    105199544 allocs/op
PASS
ok      gopkg.in/src-d/go-git.v4    201.815s

271.64user 11.90system 3:28.21elapsed 136%CPU (0avgtext+0avgdata 2936412maxresident)k
0inputs+3482592outputs (0major+794100minor)pagefaults 0swaps

Bear in mind that this is not your benchmark. In your benchmark you are cloning with Depth: 1, this is cloning full history.

Thanks!

@mcuadros I've tested with smaller repos and I don't see any regression. Gains are not spectacular but I believe it's a good base to improve memory allocations.