From 42835aa6b83c0c2f694cbe078b24243fe27486f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 8 Oct 2014 10:24:06 +0200 Subject: [PATCH 1/5] revwalk: clear the flags on reset These store merge-base information which is only valid for a single run. --- src/revwalk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/revwalk.c b/src/revwalk.c index bd07d02cb..84034f580 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -540,6 +540,7 @@ void git_revwalk_reset(git_revwalk *walk) commit->in_degree = 0; commit->topo_delay = 0; commit->uninteresting = 0; + commit->flags = 0; }); git_pqueue_clear(&walk->iterator_time); From ad66bf88df087bee34bb0dd47a9eacfdbef89c57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 8 Oct 2014 10:45:47 +0200 Subject: [PATCH 2/5] revwalk: keep a single list of user inputs The old separation was due to the old merge-base finding, so it's no longer necessary. --- src/revwalk.c | 46 +++++++++++++++++++++------------------------- src/revwalk.h | 5 ++--- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index 84034f580..8a5a8464e 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -116,6 +116,7 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting, int error; git_object *obj, *oobj; git_commit_list_node *commit; + git_commit_list *list; if ((error = git_object_lookup(&oobj, walk->repo, oid, GIT_OBJ_ANY)) < 0) return error; @@ -142,13 +143,14 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting, return -1; /* error already reported by failed lookup */ commit->uninteresting = uninteresting; - if (walk->one == NULL && !uninteresting) { - walk->one = commit; - } else { - if (git_vector_insert(&walk->twos, commit) < 0) - return -1; + list = walk->user_input; + if (git_commit_list_insert(commit, &list) == NULL) { + giterr_set_oom(); + return -1; } + walk->user_input = list; + return 0; } @@ -369,25 +371,23 @@ static int revwalk_next_reverse(git_commit_list_node **object_out, git_revwalk * static int prepare_walk(git_revwalk *walk) { - int error; - unsigned int i; - git_commit_list_node *next, *two; + int error, interesting = 0; + git_commit_list *list; + git_commit_list_node *next; - /* - * If walk->one is NULL, there were no positive references, - * so we know that the walk is already over. - */ - if (walk->one == NULL) { - giterr_clear(); - return GIT_ITEROVER; + for (list = walk->user_input; list; list = list->next) { + interesting += !list->item->uninteresting; + if (process_commit(walk, list->item, list->item->uninteresting) < 0) + return -1; } - if (process_commit(walk, walk->one, walk->one->uninteresting) < 0) - return -1; - git_vector_foreach(&walk->twos, i, two) { - if (process_commit(walk, two, two->uninteresting) < 0) - return -1; + /* + * If there were no pushes, we know that the walk is already over. + */ + if (!interesting) { + giterr_clear(); + return GIT_ITEROVER; } if (walk->sorting & GIT_SORT_TOPOLOGICAL) { @@ -440,7 +440,6 @@ int git_revwalk_new(git_revwalk **revwalk_out, git_repository *repo) if (git_pqueue_init( &walk->iterator_time, 0, 8, git_commit_list_time_cmp) < 0 || - git_vector_init(&walk->twos, 4, NULL) < 0 || git_pool_init(&walk->commit_pool, 1, git_pool__suggest_items_per_page(COMMIT_ALLOC) * COMMIT_ALLOC) < 0) return -1; @@ -470,7 +469,6 @@ void git_revwalk_free(git_revwalk *walk) git_oidmap_free(walk->commits); git_pool_clear(&walk->commit_pool); git_pqueue_free(&walk->iterator_time); - git_vector_free(&walk->twos); git__free(walk); } @@ -547,10 +545,8 @@ void git_revwalk_reset(git_revwalk *walk) git_commit_list_free(&walk->iterator_topo); git_commit_list_free(&walk->iterator_rand); git_commit_list_free(&walk->iterator_reverse); + git_commit_list_free(&walk->user_input); walk->walking = 0; - - walk->one = NULL; - git_vector_clear(&walk->twos); } int git_revwalk_add_hide_cb( diff --git a/src/revwalk.h b/src/revwalk.h index d81f97c01..05a7a4223 100644 --- a/src/revwalk.h +++ b/src/revwalk.h @@ -35,9 +35,8 @@ struct git_revwalk { first_parent: 1; unsigned int sorting; - /* merge base calculation */ - git_commit_list_node *one; - git_vector twos; + /* the pushes and hides */ + git_commit_list *user_input; /* hide callback */ git_revwalk_hide_cb hide_cb; From e7970576f1bcef1561813dbef339a31efabdc9b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 8 Oct 2014 15:52:11 +0200 Subject: [PATCH 3/5] revwalk: mark uninteresting only up to the common ancestors This introduces a phase at the start of preparing a walk which pre-marks uninteresting commits, but only up to the common ancestors. We do this in a similar way to git, by walking down the history and marking (which is what we used to do), but we keep a time-sorted priority queue of commits and stop marking as soon as there are only uninteresting commits in this queue. This is a similar rule to the one used to find the merge-base. As we keep inserting commits regardless of the uninteresting bit, if there are only uninteresting commits in the queue, it means we've run out of interesting commits in our walk, so we can stop. The old mark_unintesting() logic is still in place, but that stops walking if it finds an already-uninteresting commit, so it will stop on the ones we've pre-marked; but keeping it allows us to also hide those that are hidden via the callback. --- src/revwalk.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/src/revwalk.c b/src/revwalk.c index 8a5a8464e..a1d761f1b 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -369,19 +369,91 @@ static int revwalk_next_reverse(git_commit_list_node **object_out, git_revwalk * } +static int interesting(git_pqueue *list) +{ + size_t i; + + for (i = 0; i < git_pqueue_size(list); i++) { + git_commit_list_node *commit = git_pqueue_get(list, i); + if (!commit->uninteresting) + return 1; + } + + return 0; +} + +static int contains(git_pqueue *list, git_commit_list_node *node) +{ + size_t i; + + for (i = 0; i < git_pqueue_size(list); i++) { + git_commit_list_node *commit = git_pqueue_get(list, i); + if (commit == node) + return 1; + } + + return 0; +} + +static int premark_uninteresting(git_revwalk *walk) +{ + int error = 0; + unsigned short i; + git_pqueue q; + git_commit_list *list; + git_commit_list_node *commit, *parent; + + if ((error = git_pqueue_init(&q, 0, 8, git_commit_list_time_cmp)) < 0) + return error; + + for (list = walk->user_input; list; list = list->next) { + if ((error = git_commit_list_parse(walk, list->item)) < 0) + goto cleanup; + + if ((error = git_pqueue_insert(&q, list->item)) < 0) + goto cleanup; + } + + while (interesting(&q)) { + commit = git_pqueue_pop(&q); + + for (i = 0; i < commit->out_degree; i++) { + parent = commit->parents[i]; + + if ((error = git_commit_list_parse(walk, parent)) < 0) + goto cleanup; + + if (commit->uninteresting) + parent->uninteresting = 1; + + if (contains(&q, parent)) + continue; + + if ((error = git_pqueue_insert(&q, parent)) < 0) + goto cleanup; + } + } + +cleanup: + git_pqueue_free(&q); + return error; +} + static int prepare_walk(git_revwalk *walk) { int error, interesting = 0; git_commit_list *list; git_commit_list_node *next; + if ((error = premark_uninteresting(walk)) < 0) + return error; + for (list = walk->user_input; list; list = list->next) { interesting += !list->item->uninteresting; if (process_commit(walk, list->item, list->item->uninteresting) < 0) return -1; } - /* * If there were no pushes, we know that the walk is already over. */ From 9b5d6cea4aba38a58233a93fde2d0ffd6945f171 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 8 Oct 2014 17:14:48 +0200 Subject: [PATCH 4/5] revwalk: catch no-push and no-hide cases If there have been no pushes, we can immediately return ITEROVER. If there have been no hides, we must not run the uninteresting pre-mark phase, as we do not want to hide anything and this would simply cause us to spend time loading objects. --- src/revwalk.c | 24 ++++++++++++++---------- src/revwalk.h | 4 +++- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index a1d761f1b..1bf9fbe5c 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -142,6 +142,11 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting, if (commit == NULL) return -1; /* error already reported by failed lookup */ + if (uninteresting) + walk->did_hide = 1; + else + walk->did_push = 1; + commit->uninteresting = uninteresting; list = walk->user_input; if (git_commit_list_insert(commit, &list) == NULL) { @@ -441,26 +446,24 @@ cleanup: static int prepare_walk(git_revwalk *walk) { - int error, interesting = 0; + int error; git_commit_list *list; git_commit_list_node *next; - if ((error = premark_uninteresting(walk)) < 0) + /* If there were no pushes, we know that the walk is already over */ + if (!walk->did_push) { + giterr_clear(); + return GIT_ITEROVER; + } + + if (walk->did_hide && (error = premark_uninteresting(walk)) < 0) return error; for (list = walk->user_input; list; list = list->next) { - interesting += !list->item->uninteresting; if (process_commit(walk, list->item, list->item->uninteresting) < 0) return -1; } - /* - * If there were no pushes, we know that the walk is already over. - */ - if (!interesting) { - giterr_clear(); - return GIT_ITEROVER; - } if (walk->sorting & GIT_SORT_TOPOLOGICAL) { unsigned short i; @@ -619,6 +622,7 @@ void git_revwalk_reset(git_revwalk *walk) git_commit_list_free(&walk->iterator_reverse); git_commit_list_free(&walk->user_input); walk->walking = 0; + walk->did_push = walk->did_hide = 0; } int git_revwalk_add_hide_cb( diff --git a/src/revwalk.h b/src/revwalk.h index 05a7a4223..72ddedc75 100644 --- a/src/revwalk.h +++ b/src/revwalk.h @@ -32,7 +32,9 @@ struct git_revwalk { int (*enqueue)(git_revwalk *, git_commit_list_node *); unsigned walking:1, - first_parent: 1; + first_parent: 1, + did_hide: 1, + did_push: 1; unsigned int sorting; /* the pushes and hides */ From d6afda62d94d7bb29d864cf1c86648839ef60b2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 8 Oct 2014 17:17:31 +0200 Subject: [PATCH 5/5] revwalk: clear first-parent flag on reset This should have been included when implementing the feature but was missed. --- src/revwalk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/revwalk.c b/src/revwalk.c index 1bf9fbe5c..4dca7599a 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -621,6 +621,7 @@ void git_revwalk_reset(git_revwalk *walk) git_commit_list_free(&walk->iterator_rand); git_commit_list_free(&walk->iterator_reverse); git_commit_list_free(&walk->user_input); + walk->first_parent = 0; walk->walking = 0; walk->did_push = walk->did_hide = 0; }