On 10/6/2017 10:11 AM, Jeff King wrote:
On Thu, Oct 05, 2017 at 08:39:42AM -0400, Derrick Stolee wrote:
I'll run some perf numbers for these commands you recommend, and also see if
I can replicate some of the pain points that triggered this change using the
Linux repo.
Thanks!
-Peff
In my local copy, I added a test to p4211-line-log.sh that runs "git log
--raw -r" and tested it on three copies of the Linux repo. In order,
they have 1 packfile (0 loose), 24 packfiles (0 loose), and 23 packfiles
(~324,000 loose).
4211.6: git log --raw -r 43.34(42.62+0.65) 40.47(40.16+0.27) -6.6%
4211.6: git log --raw -r 88.77(86.54+2.12) 82.44(81.87+0.52) -7.1%
4211.6: git log --raw -r 108.86(103.97+4.81) 103.92(100.63+3.19) -4.5%
We have moderate performance gains for this command, despite the command
doing many more things than just checking abbreviations.
I plan to re-roll my patch on Monday including the following feedback items:
* Remove test-list-objects and test-abbrev in favor of a new "git log"
performance test.
* Fix binary search overflow error.
* Check response from open_pack_index(p) in find_abbrev_len_for_pack().
I plan to return without failure on non-zero result, which results in
no failure on a bad pack and the abbreviation length will be the
minimum required among all valid packs. (Thanks Stefan!)
I made note of a few things, but will not include them in my re-roll.
I'll revisit them later if they are valuable:
- nth_packed_object_sha1() could be simplified in
find_abbrev_len_for_pack().
- Teach 'cat-file' to --batch-check='%(objectsize:short)'. (Peff already
included a patch, perhaps that could be reviewed separately.)
- Ramsay caught my lack of "static" in test-list-objects.c, but that
file will be removed in the next patch. I'll make sure to use "static"
in the future.
I'm not re-rolling immediately to allow for some extra review over the
weekend, if anyone is so inclined.
Thanks,
-Stolee
