@matheusbsbs / matheus-bagliano / commits / 5dfb2bbd8d

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>

Kirill Smelkov committed Feb 24, 2014 at 20:21 UTC 5dfb2bbd8d2e2e48aa3ad6c6a8f437bbe5d2a7fb

1 file changed +14 -8

tree-diff.c

+14 -8

@@ -178,18 +178,24 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,

178 update_tree_entry(t1);

179 continue;

180 }

181 - switch (compare_tree_entry(t1, t2, &base, opt)) {

182 - case -1:

181 +

182 + cmp = compare_tree_entry(t1, t2, &base, opt);

183 +

184 + /* t1 = t2 */

185 + if (cmp == 0) {

186 update_tree_entry(t1);

184 - continue;

185 - case 0:

187 + update_tree_entry(t2);

188 + }

189 +

190 + /* t1 < t2 */

191 + else if (cmp < 0) {

192 update_tree_entry(t1);

187 - /* Fallthrough */

188 - case 1:

193 + }

194 +

195 + /* t1 > t2 */

196 + else {

197 update_tree_entry(t2);

190 - continue;

198 }

192 - die("git diff-tree: internal error");

199 }

200

201 strbuf_release(&base);