On Mon, Mar 15, 2021 at 8:21 AM Derrick Stolee <stolee@xxxxxxxxx> wrote:
>
> On 3/13/2021 5:22 PM, Elijah Newren via GitGitGadget wrote:> === Results ===
> >
> > 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!
