From: Derrick Stolee <derrickstolee@github.com>
Date: Tue, 26 Apr 2022 13:02:16 GMT
Subject: Re: [PATCH 6/7] stash: merge applied stash with merge-ort
Message-ID: <f2b4b885-b473-bea7-29e5-9e4f6002fc27@github.com>
In-Reply-To: <76f2a9e87223e1c1ebf2de6629c46bdf7ad326f1.1650908958.git.gitgitgadget@gmail.com>

On 4/25/2022 1:49 PM, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@github.com>
> 
> Change the merge function used in 'do_apply_stash()' from 'merge_recursive'
> to 'merge_ort_recursive'. In addition to aligning with the default merge
> strategy used by 'git merge' (6a5fb96672 (Change default merge backend from
> recursive to ort, 2021-08-04)), this allows 'git stash <apply|pop>' to
> operate without expanding the index by default. Update tests in 't1092'
> verifying index expansion for 'git stash' accordingly.

This is an interesting change, not just in the sparse index sense. Yes,
using ORT by default is a good idea to align with our default merge
strategy _and_ allowing us to avoid expanding the sparse index.

But: should we allow this algorithm to change via our pull.twoHead
config value? By default, it will be fast with the sparse index. Would
there be value in allowing a user to change back to the recursive
strategy if they want?

This also seems like we should have a measurable performance impact
for 'git stash <apply|pop>' when the stash contains a lot of renames.
It would be interesting to have that documented here. Elijah might
have some thoughts on how to measure this performance impact.

Thanks,
-Stolee