Re: [PATCH 6/7] diffcore-rename: simplify and accelerate register_rename_src()

[Taylor Blau <me@ttaylorr.com>]

On Sun, Dec 06, 2020 at 02:54:35AM +0000, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
>
> reigster_rename_src() took pains to create an array in rename_src which
> was sorted by pathname of the contained diff_filepair.  However, the
> fact that this array was sorted was not needed anywhere, and thus
> represented wasted time.  Simply append to the end of the array, which
> in a usecase of note saved 45% of diffcore_rename() setup time for me.
>
> Technically, there is one difference in the end result for the code.  IF

s/IF/If ?

> the caller of diffcore-rename provides a set of pairs with a duplicate
> filename in the sources (an erroneous input condition), the old code
> would simply ignore the duplicate (without warning or error).  The new
> code will simply swallow it and thus allow multiple pairings for the
> "same" source file.  Checking for this error condition is expensive
> (basically undoing the optimization) and the only existing callers
> already avoid passing such an invalid input.  If someone really wants to
> add some error checking here on this input condition, I believe they
> should add a separate function which can be called before
> diffcore_rename(), and then other callers that don't need additional
> checks because of their design (such as merge-ort) can avoid the extra
> overhead.

It seems like this is currently impossible to trigger, making any extra
(expensive) checking of it worthless, no?

> Also, note that I dropped the return type on the function since it was
> unconditionally discarded anyway.
>
> This patch is being submitted in a different order than its original
> development, but in a large rebase of many commits with lots of renames
> and with several optimizations to inexact rename detection,
> diffcore_rename() setup time was a sizeable chunk of overall runtime.
> This patch dropped execution time of rebasing 35 commits with lots of
> renames by 2% overall.

Neat!

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  diffcore-rename.c | 30 +++---------------------------
>  1 file changed, 3 insertions(+), 27 deletions(-)
>
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 3d637ba4645..816d2fbac44 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -76,36 +76,12 @@ static struct diff_rename_src {
> [...]

This looks obviously correct.

Thanks,
Taylor