Skip to content

Commit

Permalink
tree-diff: rework diff_tree interface to be sha1 based
Browse files Browse the repository at this point in the history
In the next commit this will allow to reduce intermediate calls, when
recursing into subtrees - at that stage we know only subtree sha1, and
it is natural for tree walker to start from that phase. For now we do

    diff_tree
        show_path
            diff_tree_sha1
                diff_tree
                    ...

and the change will allow to reduce it to

    diff_tree
        show_path
            diff_tree

Also, it will allow to omit allocating strbuf for each subtree, and just
reuse the common strbuf via playing with its len.

The above-mentioned improvements go in the next 2 patches.

The downside is that try_to_follow_renames(), if active, we cause
re-reading of 2 initial trees, which was negligible based on my timings,
and which is outweighed cogently by the upsides.

NOTE To keep with the current interface and semantics, I needed to
rename the function from diff_tree() to diff_tree_sha1(). As
diff_tree_sha1() was already used, and the function we are talking here
is its more low-level helper, let's use convention for prefixing
such helpers with "ll_". So the final renaming is

    diff_tree() -> ll_diff_tree_sha1()

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 27, 2014
1 parent ad6f3cc commit 52894e7
Showing 1 changed file with 28 additions and 32 deletions.
60 changes: 28 additions & 32 deletions tree-diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,17 @@ static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
}
}

static int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
const char *base_str, struct diff_options *opt)
static int ll_diff_tree_sha1(const unsigned char *old, const unsigned char *new,
const char *base_str, struct diff_options *opt)
{
struct tree_desc t1, t2;
void *t1tree, *t2tree;
struct strbuf base;
int baselen = strlen(base_str);

t1tree = fill_tree_descriptor(&t1, old);
t2tree = fill_tree_descriptor(&t2, new);

/* Enable recursion indefinitely */
opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);

Expand All @@ -159,39 +164,41 @@ static int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
if (diff_can_quit_early(opt))
break;
if (opt->pathspec.nr) {
skip_uninteresting(t1, &base, opt);
skip_uninteresting(t2, &base, opt);
skip_uninteresting(&t1, &base, opt);
skip_uninteresting(&t2, &base, opt);
}
if (!t1->size && !t2->size)
if (!t1.size && !t2.size)
break;

cmp = tree_entry_pathcmp(t1, t2);
cmp = tree_entry_pathcmp(&t1, &t2);

/* t1 = t2 */
if (cmp == 0) {
if (DIFF_OPT_TST(opt, FIND_COPIES_HARDER) ||
hashcmp(t1->entry.sha1, t2->entry.sha1) ||
(t1->entry.mode != t2->entry.mode))
show_path(&base, opt, t1, t2);
hashcmp(t1.entry.sha1, t2.entry.sha1) ||
(t1.entry.mode != t2.entry.mode))
show_path(&base, opt, &t1, &t2);

update_tree_entry(t1);
update_tree_entry(t2);
update_tree_entry(&t1);
update_tree_entry(&t2);
}

/* t1 < t2 */
else if (cmp < 0) {
show_path(&base, opt, t1, /*t2=*/NULL);
update_tree_entry(t1);
show_path(&base, opt, &t1, /*t2=*/NULL);
update_tree_entry(&t1);
}

/* t1 > t2 */
else {
show_path(&base, opt, /*t1=*/NULL, t2);
update_tree_entry(t2);
show_path(&base, opt, /*t1=*/NULL, &t2);
update_tree_entry(&t2);
}
}

strbuf_release(&base);
free(t2tree);
free(t1tree);
return 0;
}

Expand All @@ -206,7 +213,7 @@ static inline int diff_might_be_rename(void)
!DIFF_FILE_VALID(diff_queued_diff.queue[0]->one);
}

static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt)
static void try_to_follow_renames(const unsigned char *old, const unsigned char *new, const char *base, struct diff_options *opt)
{
struct diff_options diff_opts;
struct diff_queue_struct *q = &diff_queued_diff;
Expand Down Expand Up @@ -244,7 +251,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
diff_opts.break_opt = opt->break_opt;
diff_opts.rename_score = opt->rename_score;
diff_setup_done(&diff_opts);
diff_tree(t1, t2, base, &diff_opts);
ll_diff_tree_sha1(old, new, base, &diff_opts);
diffcore_std(&diff_opts);
free_pathspec(&diff_opts.pathspec);

Expand Down Expand Up @@ -305,23 +312,12 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co

int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const char *base, struct diff_options *opt)
{
void *tree1, *tree2;
struct tree_desc t1, t2;
unsigned long size1, size2;
int retval;

tree1 = fill_tree_descriptor(&t1, old);
tree2 = fill_tree_descriptor(&t2, new);
size1 = t1.size;
size2 = t2.size;
retval = diff_tree(&t1, &t2, base, opt);
if (!*base && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename()) {
init_tree_desc(&t1, tree1, size1);
init_tree_desc(&t2, tree2, size2);
try_to_follow_renames(&t1, &t2, base, opt);
}
free(tree1);
free(tree2);
retval = ll_diff_tree_sha1(old, new, base, opt);
if (!*base && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename())
try_to_follow_renames(old, new, base, opt);

return retval;
}

Expand Down

0 comments on commit 52894e7

Please sign in to comment.