Re: [PATCH v2 00/13] Optimization batch 11: avoid repeatedly detecting same renames

[Derrick Stolee <stolee@gmail.com>]

On 5/14/2021 1:37 PM, Elijah Newren wrote:
> On Mon, May 3, 2021 at 7:12 PM Elijah Newren via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> This series avoids repeatedly detecting the same renames in a sequence of
>> merges such as a rebase or cherry-pick of several commits. It's
>> unfortunately become a bit lengthy, but much of the length (the first five
>> patches) is owed to special testcases and documentation.
> 
> Since this obviously hasn't inspired much review, let me note that you
> can cut out 90% of the review size by skipping patches 2 & 5.
> 
> Patch 2 is essentially written as something approaching a formal
> proof, so yes it's dense and lengthy, but it's not at all required;
> there's no code there.  Think of it as insurance for if someone wants
> to introduce some new tricky optimizations or radically different
> features to the merge machinery, because the remember-renames
> optimization by its nature tends to interact with other optimizations.
> I figured because of that interaction that documenting why and how the
> remember renames optimization works at a much deeper level than is
> typical for optimizations or code in git in general that it might help
> with future maintenance...and it happened to help me catch two minor
> bugs.
> 
> Patch 5 is very much related to patch 2; it's testcases inspired by
> that document.  Most of those tests were just "what could possibly go
> wrong in a new from-scratch implementation of this optimization?"
> based on what's written in this proof-like document.  Most of the
> tests didn't turn up anything, but a couple found some small issues in
> my implementation.  I decided to just include all of them; it's nice
> to be thorough.
> 
> You can get 95% of the whole idea behind this optimization skipping
> those patches and reading Junio's great two-paragraph summary at
> https://lore.kernel.org/git/xmqqzgyrukic.fsf@gitster.g/, and then just
> read the other patches in this series.

Sorry, yes. I've been reading this a bit but haven't commented yet.

Patch 2 was enlightening and I appreciate the attention to detail
there. The overall argument made sense to me.

I can promise a completed review on Monday.

-Stolee