Re: [PATCH 6/7] stash: merge applied stash with merge-ort
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Apr 26, 2022, 13:02 UTC
- 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:
Show 8 quoted lines
> 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