Skip to content

Commit

Permalink
tree-diff: don't assume compare_tree_entry() returns -1,0,1
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Kirill Smelkov authored and Junio C Hamano committed Mar 20, 2014
1 parent d00e980 commit 5dfb2bb
Showing 1 changed file with 14 additions and 8 deletions.
22 changes: 14 additions & 8 deletions tree-diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 5dfb2bb

Please sign in to comment.