Re: [PATCH 0/8] Optimization batch 10: avoid detecting even more irrelevant renames

On Mon, Mar 15, 2021 at 8:21 AM Derrick Stolee [off-list ref] wrote:

On 3/13/2021 5:22 PM, Elijah Newren via GitGitGadget wrote:> === Results ===

quoted

For the testcases mentioned in commit 557ac03 ("merge-ort: begin performance
work; instrument with trace2_region_* calls", 2020-10-28), the changes in
just this series improves the performance as follows:

                     Before Series           After Series
no-renames:        5.680 s ±  0.096 s     5.665 s ±  0.129 s
mega-renames:     13.812 s ±  0.162 s    11.435 s ±  0.158 s
just-one-mega:   506.0  ms ±  3.9  ms   494.2  ms ±  6.1  ms


While those results may look somewhat meager, it is important to note that
the previous optimizations have already reduced rename detection time to
nearly 0 for these particular testcases so there just isn't much left to
improve. The final patch in the series shows an alternate testcase where the
previous optimizations aren't as effective (a simple cherry-pick of a commit
that simply adds one new empty file), where there was a speedup factor of
approximately 3 due to this series:

                     Before Series           After Series
pick-empty:        1.936 s ±  0.024 s     688.1 ms ±  4.2 ms


There was also another testcase at $DAYJOB where I saw a factor 7
improvement from this particular optimization, so it certainly has the
potential to help when the previous optimizations are not quite enough.

These performance numbers continue to be impressive.

I read through this series and found it clearly described. I can't
completely vouch for its correctness, because there are a lot of
moving parts at this point. I'll trust the test cases and Elijah's
additional manual testing at this point.

Thanks for reading through it.  I understand the comment; in my
opinion this was the most complex of the nine "Optimization batch"
series to review (batch 8 was the second most complex, and batch 13
will be the third); I tried to simplify it as much as possible, but
there's just more stuff to simultaneously track in order to get this
one implemented.  Thanks for reading through it.

Outside of me talking out loud about an enum (which you can ignore)
I didn't see anything unusual about the code.

Ah, that answers my other question then.  :-)

Thanks!