From 791303284cb3ede61729e33112d6923df406161f Mon Sep 17 00:00:00 2001 From: Kirill Smelkov Date: Wed, 5 Feb 2014 20:57:09 +0400 Subject: [PATCH 1/5] tree-diff: allow diff_tree_sha1 to accept NULL sha1 which would mean that corresponding tree - old or new - is empty. As followup patches will show, that functionality was already needed in several places of Git codebase, but there, we were preparing empty tree_desc objects by hand, with some code duplication. For handling sha1 = NULL case, let's reuse fill_tree_descriptor() which returns just empty tree_desc in that case. Signed-off-by: Kirill Smelkov Signed-off-by: Junio C Hamano --- tree-diff.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tree-diff.c b/tree-diff.c index 456660c7a2..b919983e96 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -294,14 +294,10 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha unsigned long size1, size2; int retval; - tree1 = read_object_with_reference(old, tree_type, &size1, NULL); - if (!tree1) - die("unable to read source tree (%s)", sha1_to_hex(old)); - tree2 = read_object_with_reference(new, tree_type, &size2, NULL); - if (!tree2) - die("unable to read destination tree (%s)", sha1_to_hex(new)); - init_tree_desc(&t1, tree1, size1); - init_tree_desc(&t2, tree2, size2); + 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); From 0b707c3319f37f2ec3700638d62f3199af40c138 Mon Sep 17 00:00:00 2001 From: Kirill Smelkov Date: Wed, 5 Feb 2014 20:57:10 +0400 Subject: [PATCH 2/5] tree-diff: convert diff_root_tree_sha1() to just call diff_tree_sha1 with old=NULL Now since diff_tree_sha1 understands NULL for both old and new, we could indicate an empty tree for root commit by providing just NULL for old sha1. Signed-off-by: Kirill Smelkov Signed-off-by: Junio C Hamano --- tree-diff.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/tree-diff.c b/tree-diff.c index b919983e96..11c3550177 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -311,18 +311,5 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_options *opt) { - int retval; - void *tree; - unsigned long size; - struct tree_desc empty, real; - - tree = read_object_with_reference(new, tree_type, &size, NULL); - if (!tree) - die("unable to read root tree (%s)", sha1_to_hex(new)); - init_tree_desc(&real, tree, size); - - init_tree_desc(&empty, "", 0); - retval = diff_tree(&empty, &real, base, opt); - free(tree); - return retval; + return diff_tree_sha1(NULL, new, base, opt); } From 7bc4ec01dde22be0156d64ef77db7364a11cb859 Mon Sep 17 00:00:00 2001 From: Kirill Smelkov Date: Wed, 5 Feb 2014 20:57:11 +0400 Subject: [PATCH 3/5] line-log: convert to using diff_tree_sha1() Since diff_tree_sha1() can now accept empty trees via NULL sha1, we could just call it without manually reading trees into tree_desc and duplicating code. Cc: Thomas Rast Signed-off-by: Kirill Smelkov Signed-off-by: Junio C Hamano --- line-log.c | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/line-log.c b/line-log.c index 717638b333..1500101058 100644 --- a/line-log.c +++ b/line-log.c @@ -766,16 +766,6 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list } } -static void load_tree_desc(struct tree_desc *desc, void **tree, - const unsigned char *sha1) -{ - unsigned long size; - *tree = read_object_with_reference(sha1, tree_type, &size, NULL); - if (!*tree) - die("Unable to read tree (%s)", sha1_to_hex(sha1)); - init_tree_desc(desc, *tree, size); -} - static int count_parents(struct commit *commit) { struct commit_list *parents = commit->parents; @@ -842,18 +832,11 @@ static void queue_diffs(struct line_log_data *range, struct diff_queue_struct *queue, struct commit *commit, struct commit *parent) { - void *tree1 = NULL, *tree2 = NULL; - struct tree_desc desc1, desc2; - assert(commit); - load_tree_desc(&desc2, &tree2, commit->tree->object.sha1); - if (parent) - load_tree_desc(&desc1, &tree1, parent->tree->object.sha1); - else - init_tree_desc(&desc1, "", 0); DIFF_QUEUE_CLEAR(&diff_queued_diff); - diff_tree(&desc1, &desc2, "", opt); + diff_tree_sha1(parent ? parent->tree->object.sha1 : NULL, + commit->tree->object.sha1, "", opt); if (opt->detect_rename) { filter_diffs_for_paths(range, 1); if (diff_might_be_rename()) @@ -861,11 +844,6 @@ static void queue_diffs(struct line_log_data *range, filter_diffs_for_paths(range, 0); } move_diff_queue(queue, &diff_queued_diff); - - if (tree1) - free(tree1); - if (tree2) - free(tree2); } static char *get_nth_line(long line, unsigned long *ends, void *data) From 6275c91c08f16f9d9aefea39ab7dab8560f50512 Mon Sep 17 00:00:00 2001 From: Kirill Smelkov Date: Wed, 5 Feb 2014 20:57:12 +0400 Subject: [PATCH 4/5] revision: convert to using diff_tree_sha1() Since diff_tree_sha1() can now accept empty trees via NULL sha1, we could just call it without manually reading trees into tree_desc and duplicating code. Besides, that if (!tree) return 0; looked suspect - we were saying an invalid tree != empty tree, but maybe it is better to just say the tree is invalid here, which is what diff_tree_sha1() does for such case. Signed-off-by: Kirill Smelkov Signed-off-by: Junio C Hamano --- revision.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/revision.c b/revision.c index a0df72f32c..7f61ecde8c 100644 --- a/revision.c +++ b/revision.c @@ -496,24 +496,14 @@ static int rev_compare_tree(struct rev_info *revs, static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit) { int retval; - void *tree; - unsigned long size; - struct tree_desc empty, real; struct tree *t1 = commit->tree; if (!t1) return 0; - tree = read_object_with_reference(t1->object.sha1, tree_type, &size, NULL); - if (!tree) - return 0; - init_tree_desc(&real, tree, size); - init_tree_desc(&empty, "", 0); - tree_difference = REV_TREE_SAME; DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES); - retval = diff_tree(&empty, &real, "", &revs->pruning); - free(tree); + retval = diff_tree_sha1(NULL, t1->object.sha1, "", &revs->pruning); return retval >= 0 && (tree_difference == REV_TREE_SAME); } From 7146e66f0861c720f9b32dc9d80ab80495a33e43 Mon Sep 17 00:00:00 2001 From: Kirill Smelkov Date: Thu, 6 Feb 2014 15:36:31 +0400 Subject: [PATCH 5/5] tree-walk: finally switch over tree descriptors to contain a pre-parsed entry This continues 4651ece8 (Switch over tree descriptors to contain a pre-parsed entry) and moves the only rest computational part mode = canon_mode(mode) from tree_entry_extract() to tree entry decode phase - to decode_tree_entry(). The reason to do it, is that canon_mode() is at least 2 conditional jumps for regular files, and that could be noticeable should canon_mode() be invoked several times. That does not matter for current Git codebase, where typical tree traversal is while (t->size) { sha1 = tree_entry_extract(t, &path, &mode); ... update_tree_entry(t); } i.e. we do t -> sha1,path.mode "extraction" only once per entry. In such cases, it does not matter performance-wise, where that mode canonicalization is done - either once in tree_entry_extract(), or once in decode_tree_entry() called by update_tree_entry() - it is approximately the same. But for future code, which could need to work with several tree_desc's in parallel, it could be handy to operate on tree_desc descriptors, and do "extracts" only when needed, or at all, access only relevant part of it through structure fields directly. And for such situations, having canon_mode() be done once in decode phase is better - we won't need to pay the performance price of 2 extra conditional jumps on every t->mode access. So let's move mode canonicalization to decode_tree_entry(). That was the final bit. Now after tree entry is decoded, it is fully ready and could be accessed either directly via field, or through tree_entry_extract() which this time got really "totally trivial". Signed-off-by: Kirill Smelkov Signed-off-by: Junio C Hamano --- tree-walk.c | 2 +- tree-walk.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tree-walk.c b/tree-walk.c index 79dba1d0f4..4dc86c7fe5 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -37,7 +37,7 @@ static void decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned /* Initialize the descriptor entry */ desc->entry.path = path; - desc->entry.mode = mode; + desc->entry.mode = canon_mode(mode); desc->entry.sha1 = (const unsigned char *)(path + len); } diff --git a/tree-walk.h b/tree-walk.h index ae04b6417d..ae7fb3a824 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -16,7 +16,7 @@ struct tree_desc { static inline const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pathp, unsigned int *modep) { *pathp = desc->entry.path; - *modep = canon_mode(desc->entry.mode); + *modep = desc->entry.mode; return desc->entry.sha1; }