On 12/1/2019 11:22 AM, Junio C Hamano wrote: > Derrick Stolee <stolee@xxxxxxxxx> writes: > >> In general I like this change. I'm happy that this was split into a >> method instead of crammed into the block of the "if" below. >> >>> + clear_author_date_slab(&info->author_date); >> >> The only issue I have is that the author_date slab should not be >> cleared. That is used by more than the topo-walk AND the values for >> author dates will not change between subsequent revision walks. Just >> drop that line and we should be good to go! > > Hmph, isn't this merely a performance thing, or would a slab that > was once cleared never repopulate upon its second use (i.e. > affecting correctness)? Yes, this is only a performance thing. If you think it is safest to clear it here, then it can stay. -Stolee