git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/8] Optimization batch 9: avoid detecting irrelevant renames

From
Elijah Newren <newren@gmail.com>
Date
Mar 1, 2021, 16:39 UTC
Message-ID
<CABPp-BEVvfMLGFthb5EQpYORSe9BifQRNb40+mgcg=bGCABc5Q@mail.gmail.com>
In-Reply-To
<pull.845.git.1614484707.gitgitgadget@gmail.com>

On Sat, Feb 27, 2021 at 7:58 PM Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 94 quoted lines
>
> This series depends textually on ort-perf-batch-8, but semantically it's
> almost completely unrelated and can be reviewed without any familiarity with
> any of the previous patch series.
>
> === Basic Optimization idea ===
>
> This series determines paths which meet 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. This series represents
> "Optimization #2" from my Git Merge 2020 talk[1], though this series has
> some improvements on the optimization relative to what I had at that time.
>
> The basic idea here is that if side A of history:
>
>  * only modifies/adds/deletes a few files
>  * adds new files to few if any of the directories that side B deleted or
>    renamed
>
> then when we do rename detection on side B we can avoid even looking at most
> (and perhaps even all) paths that side B deleted. Since commits being
> rebased or cherry-picked tend to only modify a few files, this optimization
> tends to be particularly effective for rebases and cherry-picks.
>
> Basing rename detection on what the other side of history did to a file
> means that extra information needs to be fed from merge-ort to
> diffcore-rename in order to take advantage of such an optimization.
>
> === Comparison to previous series ===
>
> This series differs from my two previous optimizations[2][3] (focusing on
> basename-guided rename detection) in two important aspects:
>
>  * there are no behavioral changes (there is no heuristic involved)
>
>  * this optimization is merge specific (it does not help the diff/status/log
>    family of commands, just merge/rebase/cherry-pick and such)
>
> === Results ===
>
> For the testcases mentioned in commit 557ac0350d ("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:       12.596 s ±  0.061 s     5.680 s ±  0.096 s
> mega-renames:    130.465 s ±  0.259 s    13.812 s ±  0.162 s
> just-one-mega:     3.958 s ±  0.010 s   506.0  ms ±  3.9  ms
>
>
> However, interestingly, if we had ignored the basename-guided rename
> detection optimizations[2][3], then this optimization series would have
> improved the performance as follows:
>
>                Before Basename Series   After Just This Series
> no-renames:      13.815 s ±  0.062 s      5.728 s ±  0.104 s
> mega-renames:  1799.937 s ±  0.493 s     18.213 s ±  0.139 s
> just-one-mega    51.289 s ±  0.019 s    891.9  ms ±  7.0  ms
>
>
> 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
>
>
> === Competition between optimizations ===
>
> We now have three major rename-related optimizations:
>
>  * exact rename detection
>  * basename-guided rename detection[2][3]
>  * skip-because-unnecessary (this series)
>
> It is possible for all three to potentially apply for specific paths (they
> do for the majority of renamed paths in our testcases), but we cannot use
> more than one for any given path. It turns out that the priority we give
> each optimization is very important and can drastically affect performance.
> We get best results by prioritizing them as follows:
>
>  1. exact rename detection
>  2. skip-because-unnecessary
>  3. basename-guided rename detection
>
> The third-to-last patch of this series also discusses this ordering and
> another minor variant of the skip-because-unnecessary optimization that was
> tried (and resulted in less effective performance gains than reported here),
> as well as some of the preparatory work over the past few years that this
> series relies on in order to enable this optimization.

Oops. When I restructured the series I carefully re-read the commit messages to make sure I didn't forget to update one...but I apparently forgot to update the cover letter. The discussion was actually split across a few patches by the refactoring, and what is now the third-to-last patch doesn't contain any of that discussion.

Show 48 quoted lines
> === Near optimal? ===
>
> You may remember that there was a row labelled "everything else" from the
> commit message of 557ac0350d that represented the maximum possible speed-up
> from accelerating rename detection alone; as stated in that commit, those
> rows represented how fast the code could be if we had somehow infinitely
> parallelized the inexact rename detection. However, if you compare those
> "maximum speedup" numbers to what we have above, you'll note that the
> infinitely parallelized inexact rename detection would have been slightly
> slower than the results we have now achieved. (The reason this is possible,
> despite the fact that we still spend time in rename detection after our
> optimizations, is because we implemented two optimizations outside of
> diffcore_rename() along the way.) However, this good news does also come
> with a downside -- it means that our remaining optimization potential is
> somewhat limited, and subsequent optimization series will have to fight for
> much smaller gains.
>
> [1]
> https://github.com/newren/presentations/blob/pdfs/merge-performance/merge-performance-slides.pdf
> [2]
> https://lore.kernel.org/git/pull.843.git.1612651937.gitgitgadget@gmail.com/
> [3]
> https://lore.kernel.org/git/pull.844.git.1613289544.gitgitgadget@gmail.com/
>
> Elijah Newren (8):
>   diffcore-rename: enable filtering possible rename sources
>   merge-ort: precompute subset of sources for which we need rename
>     detection
>   merge-ort: add data structures for an alternate tree traversal
>   merge-ort: introduce wrappers for alternate tree traversal
>   merge-ort: precompute whether directory rename detection is needed
>   merge-ort: use relevant_sources to filter possible rename sources
>   merge-ort: skip rename detection entirely if possible
>   diffcore-rename: avoid doing basename comparisons for irrelevant
>     sources
>
>  diffcore-rename.c |  63 ++++++++++---
>  diffcore.h        |   1 +
>  merge-ort.c       | 236 +++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 285 insertions(+), 15 deletions(-)
>
>
> base-commit: 4be565c472088d4144063b736308bf2a57331f45
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-845%2Fnewren%2Fort-perf-batch-9-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-845/newren/ort-perf-batch-9-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/845
> --
> gitgitgadget
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren
Message 10 of 39 in “Optimization batch 9: avoid detecting irrelevant renames”
  1. 0/8 Optimization batch 9: avoid detecting irrelevant renamesElijah Newren via GitGitGadget, Feb 28, 2021
  2. 1/8 diffcore-rename: enable filtering possible rename sourcesElijah Newren via GitGitGadget, Feb 28, 2021
  3. 3/8 merge-ort: add data structures for an alternate tree traversalElijah Newren via GitGitGadget, Feb 28, 2021
  4. 2/8 merge-ort: precompute subset of sources for which we need rename detectionElijah Newren via GitGitGadget, Feb 28, 2021
  5. 4/8 merge-ort: introduce wrappers for alternate tree traversalElijah Newren via GitGitGadget, Feb 28, 2021
  6. 5/8 merge-ort: precompute whether directory rename detection is neededElijah Newren via GitGitGadget, Feb 28, 2021
  7. 6/8 merge-ort: use relevant_sources to filter possible rename sourcesElijah Newren via GitGitGadget, Feb 28, 2021
  8. 7/8 merge-ort: skip rename detection entirely if possibleElijah Newren via GitGitGadget, Feb 28, 2021
  9. 8/8 diffcore-rename: avoid doing basename comparisons for irrelevant sourcesElijah Newren via GitGitGadget, Feb 28, 2021
  10. Elijah NewrenMar 1, 2021
  11. Elijah NewrenMar 4, 2021
  12. 0/8 Optimization batch 9: avoid detecting irrelevant renamesElijah Newren via GitGitGadget, Mar 9, 2021
  13. 2/8 merge-ort: precompute subset of sources for which we need rename detectionElijah Newren via GitGitGadget, Mar 9, 2021
  14. 1/8 diffcore-rename: enable filtering possible rename sourcesElijah Newren via GitGitGadget, Mar 9, 2021
  15. Derrick StoleeMar 9, 2021
  16. Elijah NewrenMar 9, 2021
  17. Derrick StoleeMar 9, 2021
  18. 6/8 merge-ort: use relevant_sources to filter possible rename sourcesElijah Newren via GitGitGadget, Mar 9, 2021
  19. 3/8 merge-ort: add data structures for an alternate tree traversalElijah Newren via GitGitGadget, Mar 9, 2021
  20. 4/8 merge-ort: introduce wrappers for alternate tree traversalElijah Newren via GitGitGadget, Mar 9, 2021
  21. Derrick StoleeMar 9, 2021
  22. 5/8 merge-ort: precompute whether directory rename detection is neededElijah Newren via GitGitGadget, Mar 9, 2021
  23. 8/8 diffcore-rename: avoid doing basename comparisons for irrelevant sourcesElijah Newren via GitGitGadget, Mar 9, 2021
  24. 7/8 merge-ort: skip rename detection entirely if possibleElijah Newren via GitGitGadget, Mar 9, 2021
  25. Derrick StoleeMar 9, 2021
  26. Elijah NewrenMar 9, 2021
  27. Derrick StoleeMar 9, 2021
  28. Derrick StoleeMar 10, 2021
  29. 0/8 Optimization batch 9: avoid detecting irrelevant renamesElijah Newren via GitGitGadget, Mar 11, 2021
  30. 1/8 diffcore-rename: enable filtering possible rename sourcesElijah Newren via GitGitGadget, Mar 11, 2021
  31. 3/8 merge-ort: add data structures for an alternate tree traversalElijah Newren via GitGitGadget, Mar 11, 2021
  32. 5/8 merge-ort: precompute whether directory rename detection is neededElijah Newren via GitGitGadget, Mar 11, 2021
  33. 2/8 merge-ort: precompute subset of sources for which we need rename detectionElijah Newren via GitGitGadget, Mar 11, 2021
  34. 4/8 merge-ort: introduce wrappers for alternate tree traversalElijah Newren via GitGitGadget, Mar 11, 2021
  35. 8/8 diffcore-rename: avoid doing basename comparisons for irrelevant sourcesElijah Newren via GitGitGadget, Mar 11, 2021
  36. 6/8 merge-ort: use relevant_sources to filter possible rename sourcesElijah Newren via GitGitGadget, Mar 11, 2021
  37. 7/8 merge-ort: skip rename detection entirely if possibleElijah Newren via GitGitGadget, Mar 11, 2021
  38. Derrick StoleeMar 15, 2021
  39. Elijah NewrenMar 15, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.