Subject: Re: Git is not scalable with too many refs/*

From: René Scharfe <[email-masked]>
Date: Fri, 30 Sep 2011 20:17:26 +0200

Am 30.09.2011 18:52, schrieb Junio C Hamano:
> René Scharfe <[email-masked]> writes:
> 
>> Hi Martin,
>>
>> Am 29.09.2011 22:11, schrieb Martin Fick:
>>> Your patch works well for me.  It achieves about the same 
>>> gains as Julian's patch. Thanks!
>>
>> OK, and what happens if you apply the following patch on top of my first
>> one?  It avoids going through all the refs a second time during cleanup,
>> at the cost of going through the list of all known objects.  I wonder if
>> that's any faster in your case.
>> ...
>>  static void describe_one_orphan(struct strbuf *sb, struct commit *commit)
>> @@ -690,8 +689,7 @@ static void orphaned_commit_warning(struct commit *commit)
>>  	else
>>  		describe_detached_head(_("Previous HEAD position was"), commit);
>>  
>> -	clear_commit_marks(commit, -1);
>> -	for_each_ref(clear_commit_marks_from_one_ref, NULL);
>> +	clear_commit_marks_for_all(ALL_REV_FLAGS);
>>  }
> 
> The function already clears all the flag bits from commits near the tip of
> all the refs (i.e. whatever commit it traverses until it gets to the fork
> point), so it cannot be reused in other contexts where the caller
> 
>  - first marks commit objects with some flag bits for its own purpose,
>    unrelated to the "orphaned"-ness check;
>  - calls this function to issue a warning; and then
>  - use the flag it earlier set to do something useful.
> 
> which requires "cleaning after yourself, by clearing only the bits you
> used without disturbing other bits that you do not use" pattern.

Yes, clear_commit_marks_for_all is a bit brutal.  Callers could clear
specfic bits (e.g. SEEN|UNINTERESTING) instead of ALL_REV_FLAGS, though.

> It might be a better solution to not bother to clear the marks at all;
> would it break anything in this codepath?

Unfortunately, yes; the cleanup part was added by 5c08dc48 later, when
it become apparent that it's really needed.

However, since the patch only buys us a 5% speedup I'm not sure it's
worth it in its current form.

René