Re: [PATCH 2/2] diffcore-break: save cnt_data for other phases

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 20, 2009, 07:32 UTC
Message-ID: <7vtywp7it7.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20091119152234.GA6877@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I don't know if it is worth refactoring though.

I would rather not touch it.

Show 5 quoted lines

> Also, I don't see where we ever actually free the cnt_data. After we run
> the whole O(n^2) loop, we should be able to drop cnt_data entirely. I
> think in practice it doesn't matter all that much, since it isn't that
> big, and afterwards we just generate the patch and exit (unless we are
> doing diffcore-break, too, which will actually free all of the data).

Originally, each phase of the diffcore pipeline was designed to be independent from other phases, and keeping things in core was done as an optimization for smallish trees so that later phases in the diffcore pipeline can reuse prepopulated data. If this were year 2006, I would have said that keeping it in core would be better because we could add new phases after it later, even though there is nobody who uses cnt_data after diffcore-rename in the diffcore pipeline right now,

But it seems that our performance is more than adequate without such keeping-things-around for smallish trees anyway, and it is wiser to optimize for heavier workload. More importantly, nobody seems to be planning new phases in the pipeline, so I agree it is a good idea to drop cnt_data once the rename detection phase is done with it.