diff options
author | Kirill Smelkov <kirr@mns.spb.ru> | 2014-02-24 20:21:39 +0400 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2014-03-20 15:04:31 -0700 |
commit | 5dfb2bbd8d2e2e48aa3ad6c6a8f437bbe5d2a7fb (patch) | |
tree | 79ccc58260790a12b98becf9c8f8eb3617a6d674 | |
parent | d00e980c224d4b65972dda4474fbd9294bdb185f (diff) | |
download | git-5dfb2bbd8d2e2e48aa3ad6c6a8f437bbe5d2a7fb.tar.gz |
tree-diff: don't assume compare_tree_entry() returns -1,0,1
It does, but we'll be reworking it in the next patch after it won't, and
besides it is better to stick to standard
strcmp/memcmp/base_name_compare/etc... convention, where comparison
function returns <0, =0, >0
Regarding performance, comparing for <0, =0, >0 should be a little bit
faster, than switch, because it is just 1 test-without-immediate
instruction and then up to 3 conditional branches, and in switch you
have up to 3 tests with immediate and up to 3 conditional branches.
No worry, that update_tree_entry(t2) is duplicated for =0 and >0 - it
will be good after we'll be adding support for multiparent walker and
will stay that way.
=0 case goes first, because it happens more often in real diffs - i.e.
paths are the same.
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r-- | tree-diff.c | 22 |
1 files changed, 14 insertions, 8 deletions
diff --git a/tree-diff.c b/tree-diff.c index 1af8219033..6a677daa7a 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -178,18 +178,24 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, update_tree_entry(t1); continue; } - switch (compare_tree_entry(t1, t2, &base, opt)) { - case -1: + + cmp = compare_tree_entry(t1, t2, &base, opt); + + /* t1 = t2 */ + if (cmp == 0) { update_tree_entry(t1); - continue; - case 0: + update_tree_entry(t2); + } + + /* t1 < t2 */ + else if (cmp < 0) { update_tree_entry(t1); - /* Fallthrough */ - case 1: + } + + /* t1 > t2 */ + else { update_tree_entry(t2); - continue; } - die("git diff-tree: internal error"); } strbuf_release(&base); |