Re: [PATCH 3/3] Make clear_commit_marks() clean harder

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 3, 2006, 21:12 UTC
Message-ID: <Pine.LNX.4.63.0607032309190.29667@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7vzmfqqxlh.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 3 Jul 2006, Junio C Hamano wrote:

Show 19 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> > Don't care if objects have been parsed or not and don't stop when we
> >> > reach a commit that is already clean -- its parents could be dirty.
> >> 
> >> There is something quite wrong with this patch.
> >
> > I always had the feeling that it was wrong to traverse not-yet-parsed 
> > parents: How could a revision walk possibly come to a certain commit 
> > without at least one continuous history of now-parsed objects?
> >
> > Also, AFAIK the revision walk sets flags for each commit it touched, and 
> > we should not try to be smart-asses about the flags, but just unset these 
> > flags.
> 
> The main points were made by Linus already.
> 
> Traversing is not needed -- not clearing not-yet-parsed is
> obviously wrong.

Traversing is actually wrong. Clearing the marks does not mean to clear them on commits we did not even mark!

But clearing on commits we _have_ -- but not parsed -- is important, obviously.

Show 8 quoted lines

> > BTW some very quick tests showed that the clear_commit_marks() thing that 
> > I sent to the list was much faster than traversing all objects (which was 
> > in my original version).
> 
> I have a crude workaround pushed out last night but will be
> replacing it with something less drastic.  I think the final
> version should be what you had, perhaps minus not looking at the
> parsed flag for unmarking purposes.

Isn't the right way to go about it to just clear the marks if we have a commit that has at least one of the marks set, but traverse further only if _in addition to having at least one mark set_ the commit has been parsed already?

That would not be a crude workaround.

BTW what cases could have a commit, being seen by the revision walk, not have the SEEN mark set?

Ciao, Dscho