Hi,
On Mon, 5 Mar 2007, Junio C Hamano wrote:
> This removes the flag internally used by revision traversal to
> decide which commits are indeed boundaries and renames it to
> CHILD_SHOWN. builtin-bundle uses the symbol for its
> verification, but I think the logic it uses it is wrong. The
> flag is still useful but it is local to the git-bundle, so it is
> renamed to PREREQ_MARK.
The idea was to bail out of revision walking when that prerequisite was
found, _even_ if it happened to be uninteresting.
Just marking the parents uninteresting unless they are prerequisites would
be better, probably.
>
> i = req_nr;
> while (i && (commit = get_revision(&revs)))
> - if (commit->object.flags & BOUNDARY_SHOW)
> + if (commit->object.flags & PREREQ_MARK)
> i--;
The above-mentioned idea, then would be: instead of just i--; do
{
struct commit_list *parent = commit->parents;
i--;
for (; parent; parent = parent->next)
if (!(parent->item->object.flag &
PREREQ_MARK))
parent->item->object.flag |=
UNINTERESTING;
}
This should help performance, as not all reachable commits are traversed
any more.
>
> for (i = 0; i < req_nr; i++)
> diff --git a/revision.c b/revision.c
> index 5d137ea..2d27ccf 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1285,17 +1285,21 @@ struct commit *get_revision(struct rev_info *revs)
> commit_list_insert(c, &l);
> revs->commits = l;
> revs->reverse = 0;
> + c = NULL;
Why? It gets assigned here anyway:
> }
>
> /*
> * Now pick up what they want to give us
> */
> - c = get_revision_1(revs);
> + if (!(c = get_revision_1(revs)))
> + return NULL;
> while (0 < revs->skip_count) {
> revs->skip_count--;
> c = get_revision_1(revs);
> if (!c)
> break;
> + /* Although we grabbed it, it is not shown. */
> + c->object.flags &= ~SHOWN;
Wasn't --skip meant for something like gitweb, where you just want to skip
the first <n> commits from the list, but do not want to change the list
otherwise?
> @@ -1305,6 +1309,9 @@ struct commit *get_revision(struct rev_info *revs)
> case -1:
> break;
> case 0:
> + /* Although we grabbed it, it is not shown. */
> + if (c)
> + c->object.flags &= ~SHOWN;
> c = NULL;
Is this really relevant in practice?
Ciao,
Dscho
-
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