Skip to content

Optimization batch 10: avoid detecting even more irrelevant renames#853

Closed
newren wants to merge 8 commits intogitgitgadget:temporary/ort-perf-batch-9from
newren:ort-perf-batch-10
Closed

Optimization batch 10: avoid detecting even more irrelevant renames#853
newren wants to merge 8 commits intogitgitgadget:temporary/ort-perf-batch-9from
newren:ort-perf-batch-10

Conversation

@newren
Copy link
Copy Markdown

@newren newren commented Jan 29, 2021

This series depends on ort-perf-batch-9.

=== Basic Optimization idea ===

This series adds additional special cases where detection of renames
is irrelevant, where the irrelevance is due to the fact that the merge
machinery will arrive at the same result regardless of whether a
rename is detected for any of those paths. That high level wording
makes it sound the same as ort-perf-batch-9, and basically it is, it's
just trying to take the optimization a step further.

As noted in the last series, there are two reasons that the merge
machinery needs renames:

  • in order to do three-way content merging (pairing appropriate files)
  • in order to find where directories have been renamed

ort-perf-batch-9 provided a rough approximation for the second
criteria that was good enough, but which still left us detecting more
renames than necessary. This series focuses further on that criteria
and finds ways to avoid the need to detect as many renames while still
detecting directory renames identically to before. Thus, this series
is an improvement on "Optimization #2" from my Git Merge 2020 talk[1].

=== 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.

As a reminder, before any merge-ort/diffcore-rename performance work,
the performance results we started with (as noted in the same commit
message) were:

no-renames-am:      6.940 s ±  0.485 s
no-renames:        18.912 s ±  0.174 s
mega-renames:    5964.031 s ± 10.459 s
just-one-mega:    149.583 s ±  0.751 s

[1] https://github.com/newren/presentations/blob/pdfs/merge-performance/merge-performance-slides.pdf

cc: Derrick Stolee dstolee@microsoft.com
cc: Ævar Arnfjörð Bjarmason avarab@gmail.com
cc: Jonathan Tan jonathantanmy@google.com
cc: Taylor Blau me@ttaylorr.com
cc: Derrick Stolee stolee@gmail.com
cc: Elijah Newren newren@gmail.com

@newren newren force-pushed the ort-perf-batch-10 branch 2 times, most recently from 4523b80 to 2a75ef0 Compare February 3, 2021 05:32
@newren newren force-pushed the temporary/ort-perf-batch-9 branch from 0a72a5e to 69d8e51 Compare February 3, 2021 19:03
@newren newren force-pushed the ort-perf-batch-10 branch from 2a75ef0 to 57f5b94 Compare February 3, 2021 19:03
@newren newren force-pushed the temporary/ort-perf-batch-9 branch from 69d8e51 to d33d841 Compare February 3, 2021 20:33
@newren newren force-pushed the ort-perf-batch-10 branch 4 times, most recently from 38a5af7 to fe3c2d3 Compare February 10, 2021 16:26
@newren newren force-pushed the temporary/ort-perf-batch-9 branch from a6e7c2e to ec57808 Compare February 10, 2021 16:35
@newren newren force-pushed the temporary/ort-perf-batch-9 branch from ec57808 to 043aba7 Compare February 11, 2021 07:37
@newren newren force-pushed the ort-perf-batch-10 branch 3 times, most recently from 59c6d19 to 911a730 Compare February 12, 2021 21:12
@newren newren force-pushed the temporary/ort-perf-batch-9 branch from 0d3c472 to 4077b17 Compare February 14, 2021 03:31
@newren newren force-pushed the ort-perf-batch-10 branch 2 times, most recently from 5676cfd to d8e921a Compare February 14, 2021 03:38
@newren newren force-pushed the temporary/ort-perf-batch-9 branch from 0c00d8e to 43fe23d Compare February 25, 2021 01:21
@newren newren force-pushed the temporary/ort-perf-batch-9 branch from 43fe23d to 9365786 Compare February 26, 2021 00:02
@newren newren force-pushed the temporary/ort-perf-batch-9 branch from 9365786 to d40e692 Compare February 26, 2021 20:47
@newren newren force-pushed the ort-perf-batch-10 branch 2 times, most recently from 4e5a08e to 9f16076 Compare February 27, 2021 06:17
@newren newren force-pushed the ort-perf-batch-10 branch 3 times, most recently from 1ab395a to 495c109 Compare March 13, 2021 00:55
@newren
Copy link
Copy Markdown
Author

newren commented Mar 13, 2021

/submit

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Mar 30, 2021

This patch series was integrated into seen via git@2c6478c.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Mar 30, 2021

This patch series was integrated into seen via git@40cd16d.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Mar 31, 2021

There was a status update about the branch en/ort-perf-batch-10 on the Git mailing list:

I made a mistake of picking these up before they got sufficient
exposure to the reviewers and ended up a source of potential mess
when it turns out that any of the earlier ones need rewriting (I
probably should stop picking up nested series that exceeds reviewer
bandwidth), but how ready is this and subsequent topics?

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Mar 31, 2021

This patch series was integrated into seen via git@8f5d9e1.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 2, 2021

This patch series was integrated into seen via git@be266b8.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 2, 2021

This patch series was integrated into seen via git@36fe10e.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 4, 2021

This patch series was integrated into seen via git@41a1fe2.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 6, 2021

This patch series was integrated into seen via git@eb724bc.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 6, 2021

This patch series was integrated into seen via git@16bde12.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 6, 2021

This patch series was integrated into seen via git@4218b45.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 6, 2021

There was a status update about the branch en/ort-perf-batch-10 on the Git mailing list:

Various rename detection optimization to help "ort" merge strategy
backend.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 7, 2021

This patch series was integrated into seen via git@b12fdbd.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 8, 2021

This patch series was integrated into seen via git@1523dff.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 8, 2021

This patch series was integrated into next via git@9826d89.

@gitgitgadget gitgitgadget bot added the next label Apr 8, 2021
@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 8, 2021

This patch series was integrated into seen via git@d154185.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 8, 2021

This patch series was integrated into seen via git@946bfd0.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 8, 2021

There was a status update about the branch en/ort-perf-batch-10 on the Git mailing list:

Various rename detection optimization to help "ort" merge strategy
backend.

Will merge to 'master'.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 9, 2021

This patch series was integrated into seen via git@0104f53.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 13, 2021

There was a status update in the "Cooking" section about the branch en/ort-perf-batch-10 on the Git mailing list:

Various rename detection optimization to help "ort" merge strategy
backend.

Will merge to 'master'.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 14, 2021

This patch series was integrated into seen via git@01216b2.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 15, 2021

This patch series was integrated into seen via git@1381089.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 16, 2021

There was a status update in the "Cooking" section about the branch en/ort-perf-batch-10 on the Git mailing list:

Various rename detection optimization to help "ort" merge strategy
backend.

Will merge to 'master'.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 16, 2021

This patch series was integrated into seen via git@e2e1a03.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 16, 2021

This patch series was integrated into next via git@e2e1a03.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 16, 2021

This patch series was integrated into master via git@e2e1a03.

@gitgitgadget gitgitgadget bot added the master label Apr 16, 2021
@gitgitgadget gitgitgadget bot closed this Apr 16, 2021
@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Apr 16, 2021

Closed via e2e1a03.

@newren newren deleted the ort-perf-batch-10 branch April 23, 2021 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant