Re: [PATCH 1/3] rev-list: allow bisect and first-parent flags
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 28, 2020, 21:53 UTC
- Message-ID
- <xmqqa6zjcnni.fsf@gitster.c.googlers.com>
- In-Reply-To
- <xmqq4kpre6cl.fsf@gitster.c.googlers.com>
Taking all the suggestions and speculations I made in the message I am responding to, I think squashing the following changes into the [PATCH 1/3] would make the code simpler and much easier to follow.
The first two hunks revert the changes made to count_distance(); the third hunk makes sure that count_distance() is not called under the first-parent mode, since the "single-strand-of-pearls" optimization is the only counting method needed in the first-parent mode. The last hunk is to revert the changes to the single-strand-of-pearls optimized counting and tweak the loop control to take only the first parent into account.
Your new tests in 6000 and 6002 of course still pass with this clean-up.
bisect.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/bisect.c b/bisect.c index 762f68c8e9..a11fdb1473 100644 --- a/bisect.c +++ b/bisect.c @@ -37,7 +37,7 @@ static const char *term_good; * We care just barely enough to avoid recursing for * non-merge entries. */ -static int count_distance(struct commit_list *entry, int first_parent_only) +static int count_distance(struct commit_list *entry) { int nr = 0; @@ -52,10 +52,10 @@ static int count_distance(struct commit_list *entry, int first_parent_only) commit->object.flags |= COUNTED; p = commit->parents; entry = p; - if (p && !first_parent_only) { + if (p) { p = p->next; while (p) { - nr += count_distance(p, first_parent_only); + nr += count_distance(p); p = p->next; } } @@ -315,7 +315,9 @@ static struct commit_list *do_find_bisection(struct commit_list *list, continue; if (weight(p) != -2) continue; - weight_set(p, count_distance(p, first_parent_only)); + if (first_parent_only) + BUG("shouldn't be calling count-distance in fp mode"); + weight_set(p, count_distance(p)); clear_distance(list); /* Does it happen to be at exactly half-way? */ @@ -331,21 +333,16 @@ static struct commit_list *do_find_bisection(struct commit_list *list, struct commit_list *q; unsigned flags = p->item->object.flags; - /* if commit p has already been weighted, continue. */ if (0 <= weight(p)) continue; - /* - * otherwise, find the first interesting - * already-weighted parent of p and store as q. - */ - for (q = p->item->parents; q; q = q->next) { - if (!(q->item->object.flags & UNINTERESTING) && 0 <= weight(q)) { - break; - } else if (first_parent_only) { - q = NULL; + for (q = p->item->parents; + q; + q = first_parent_only ? NULL : q->next) { + if (q->item->object.flags & UNINTERESTING) + continue; + if (0 <= weight(q)) break; - } } if (!q) continue;