Re: [PATCH v2 3/5] diffcore-rename: enable limiting rename detection to relevant destinations

On Thu, Jun 3, 2021 at 5:54 AM Derrick Stolee [off-list ref] wrote:

On 6/1/2021 10:58 AM, Elijah Newren via GitGitGadget wrote:

quoted

From: Elijah Newren <redacted>

Our former optimizations focused on limiting rename detection to a
pre-specified set of relevant sources.  This was because the merge logic
only had a way of knowing which sources were relevant.  However, other
callers of rename detection might benefit from being able to limit
rename detection to a known set of relevant destinations.  In
particular, a properly implemented `git log --follow` might benefit from
such an ability.

I would be interested in seeing such an improvement. It could also
help the batch-download of missing blobs in a partial clone situation
because it would only need the "deletes" portion of the diff, since we
only care about one "add" (we'd need that blob, too, of course).

Oh, good point.  And in order to work well with the new prefetching
improvements (which will be the next series submitted), I should
really rename remove_unneeded_paths_from_src() to
remove_unneeded_paths() and add a relevant_destinations parameter and
use it, instead of having the strset_contains() check in
diffcore_rename_extended.

quoted

Since the code to implement such limiting is very similar to what we've
already done, just implement it now even though we do not yet have any
callers making use of this ability.

However, I'm not sure that this change is warranted without such an
integration. Perhaps keep this patch here on the list as a reference
for anyone who wants to do the `git log --follow` speedup? This person
can include "you in the future".

I'm just worried that without a consumer, this code has no automated
validation and can either be buggy now or become buggy in the future
before someone starts trying to use the logic.

Fair enough; I'll pull it out.