Re: [PATCH 2/4] revision traversal: retire BOUNDARY_SHOW

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.

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?

Is this really relevant in practice?

Ciao,
Dscho