Am 30.09.2011 18:52, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@xxxxxxxxxxxxxx> 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é
--
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