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

Hi,

On Mon, 5 Mar 2007, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

Just marking the parents uninteresting unless they are prerequisites 
would be better, probably.

quoted

 
 	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

I explicitly wanted to avoid that.  If the generated bundle had
prereqs that are not independent of each other, doing that would
hide some prereqs.

Yes. I wanted to say SHOWN instead of UNINTERESTING, but reading the 
source again, it seems that add_parents_to_list() is called in 
get_revision_1() even if the commit is marked SHOWN.

So forget about that idea of mine.

quoted

This should help performance, as not all reachable commits are traversed 
any more.

You prevented that with "while (i &&" part already, didn't you?

Well, yes. I also wanted to prevent going down all paths, though.

quoted

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?

Yes, but does the patch change that?

I _think_ that yes, it could. If the timestamp of HEAD is older than of 
HEAD^, and you say "git log HEAD^ HEAD", then HEAD^ will be actually shown 
first, right? If you then say "--skip 1", HEAD will be shown first, and 
then HEAD^...

quoted

quoted

@@ -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?

Absolutely.  But I have further updates on this series.

Sorry for interrupting your patch spree :-)

Ciao,
Dscho