On Sun, May 12, 2013 at 05:28:24PM +0100, John Keeping wrote: > However, this doesn't seem to make a difference to the time taken when I > add in --cherry-mark (which is why I was partially correct in the > parallel thread - it doesn't have the effect on cherry-mark that I want > it to): > > $ time git rev-list --ancestry-path --left-right --count --cherry-mark \ > origin/master...git-gui/master > 2056 5 0 > > real 0m32.266s > user 0m31.522s > sys 0m0.749s > > $ time git rev-list --left-right --count --cherry-mark \ > origin/master...git-gui/master > 31959 5 0 > > real 0m32.140s > user 0m31.337s > sys 0m0.807s > > This seems to be caused by the code in revision.c::limit_list() which > does the cherry detection then limits to left/right and only then > applies the ancestry path. I haven't looked further than that, but is > there any reason not to apply the ancestry path restriction before > looking for patch-identical commits? With the patch below, the --ancestry-path version drops to under 2 seconds. I'm not sure if this is a good idea though. It helps me say "I know nothing that isn't on the ancestry path can be patch-identical, so don't bother checking if it is" but it regresses users who want the full cherry-pick check while only limiting the output. Perhaps we need --cherry-no-uninteresting to apply the first 3 hunks of the patch at runtime :-S -- >8 -- diff --git a/revision.c b/revision.c index de3b058..d721d83 100644 --- a/revision.c +++ b/revision.c @@ -837,7 +837,7 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs) for (p = list; p; p = p->next) { struct commit *commit = p->item; unsigned flags = commit->object.flags; - if (flags & BOUNDARY) + if (flags & (BOUNDARY | UNINTERESTING)) ; else if (flags & SYMMETRIC_LEFT) left_count++; @@ -858,7 +858,7 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs) struct commit *commit = p->item; unsigned flags = commit->object.flags; - if (flags & BOUNDARY) + if (flags & (BOUNDARY | UNINTERESTING)) continue; /* * If we have fewer left, left_first is set and we omit @@ -879,7 +879,7 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs) struct patch_id *id; unsigned flags = commit->object.flags; - if (flags & BOUNDARY) + if (flags & (BOUNDARY | UNINTERESTING)) continue; /* * If we have fewer left, left_first is set and we omit @@ -1103,17 +1103,18 @@ static int limit_list(struct rev_info *revs) show(revs, newlist); show_early_output = NULL; } - if (revs->cherry_pick || revs->cherry_mark) - cherry_pick_list(newlist, revs); - - if (revs->left_only || revs->right_only) - limit_left_right(newlist, revs); if (bottom) { limit_to_ancestry(bottom, newlist); free_commit_list(bottom); } + if (revs->cherry_pick || revs->cherry_mark) + cherry_pick_list(newlist, revs); + + if (revs->left_only || revs->right_only) + limit_left_right(newlist, revs); + /* * Check if any commits have become TREESAME by some of their parents * becoming UNINTERESTING. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html