Re: [PATCH 0/2] [RFC] Switch default merge backend from recursive to ort

[Elijah Newren <newren@gmail.com>]

On Mon, Aug 2, 2021 at 9:05 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 7/31/2021 8:07 PM, Elijah Newren via GitGitGadget wrote:
> > This is an RFC series designed to spur feedback about switching the default
> > merge backend (reviewing the patches is of secondary importance at this
> > point). Some questions:
> >
> >  * Are there things others want before this series is considered for
> >    inclusion?
> >  * What kind of timeline do others think is reasonable?
> >  * Would it be beneficial to let this series sit in 'next' for an extended
> >    duration to gain more feedback?
> >
> > Some potentially useful context in relation to the above:
> >
> >  * I've personally used the ort backend for well over a year
> >  * I have ~50 testers using ort as the default merge backend since Nov.
> >    2020.
> >  * ort fixes known bugs in recursive, and there are no known regressions
> >    (more testers may change that)
> >  * ort is significantly faster than recursive
> >  * ort provides one new feature already, and enables more that are on the
> >    way
> >  * The commit message of patch 1 has more details about the last three items
> >    above
> >
> > So...thoughts?
>
> I fully endorse this change as soon as possible. I've applied the patches
> you supplied here and submitted a PR to microsoft/git [1] to take them.
>
> [1] https://github.com/microsoft/git/pull/404
>
> I've done my own share of testing on some of our private monorepos to see
> how ORT compares to the recursive algorithm. My data is not perhaps as
> rigorous as yours, but I did notice that the ORT algorithm was consistently
> within the 5-6 second range while the recursive algorithm would vary within
> the 7-20 second range (and some outliers near 30s).
>
> Of course, I'm particularly excited about the benefits to the sparse index
> work. I also have a prototype of a 'git merge' integration with sparse
> index which was not very hard because the ORT strategy does not use the
> index as a data structure. With that change, my tests dropped to between
> 0.5s and 1.5 seconds. (This shows just how much my earlier timings were
> stuck on index reads and writes.) Now, the largest indicator of time is
> how long it takes to resolve text conflicts.
>
> Thanks! What a monumental effort.

It's certainly been a long haul.  And you jumped after all my
preparatory patches when I started submitting actual merge-ort
patches.  Since that time about a year ago, you have reviewed nearly
every single one of these patches, and there have been well over a
hundred of them and they've been far from trivial.  The code is better
today because of your comments and suggestions.  Thanks for the
monumental review effort.

One quick question on your timings here, though: in the past, when you
compared timings, you compared merge-recursive *without* rename
detection to merge-ort *with* rename detection.  Are you doing the
same here (just so others have a better sense of what is being
compared)?

Thanks again!
Elijah