Thread (17 messages) 17 messages, 7 authors, 2021-08-03

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

flat view

From: Elijah Newren <hidden>
Date: 2021-08-02 16:27:44

On Mon, Aug 2, 2021 at 9:05 AM Derrick Stolee [off-list ref] wrote:
On 7/31/2021 8:07 PM, Elijah Newren via GitGitGadget wrote:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help