Thread (1 message) 1 message, 1 author, 2019-12-04

Re: [PATCH 2/2] revision: free topo_walk_info before creating a new one in init_topo_walk

From: Junio C Hamano <hidden>
Date: 2019-12-04 19:47:30

Derrick Stolee [off-list ref] writes:
On 12/1/2019 11:22 AM, Junio C Hamano wrote:
quoted
Derrick Stolee [off-list ref] writes:
quoted
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.
quoted
+	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.
Let's keep what is already in 'next' ;-) and possibly follow-up with
a separate patch to remove this line, justfying the change as
performance improvement.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help