Re: [PATCH 6/7] diffcore-rename: simplify and accelerate register_rename_src()
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Dec 9, 2020, 22:40 UTC
- Message-ID
- <X9FScb3pzY0EBLvS@nand.local>
- In-Reply-To
- <306a48820dd3c338d2b9e2288e5a3e1c7c89f36e.1607223276.git.gitgitgadget@gmail.com>
On Sun, Dec 06, 2020 at 02:54:35AM +0000, Elijah Newren via GitGitGadget wrote:
Show 9 quoted lines
> 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 ?
Show 12 quoted lines
> 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?
Show 9 quoted lines
> 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!
Show 11 quoted lines
> 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