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

Re: [PATCH v2 04/20] merge-ort: use histogram diff

From
Elijah Newren <newren@gmail.com>
Date
Nov 11, 2020, 16:47 UTC
Message-ID
<CABPp-BFd2Wg-9h9+Gx20szX=YfKLqvBMWGg-eZU0yL8_DPh6kw@mail.gmail.com>
In-Reply-To
<cd05d258-a526-9843-f646-d2d42f1b8cbc@gmail.com>
On Wed, Nov 11, 2020 at 5:54 AM Derrick Stolee <stolee@gmail.com> wrote:
Show 15 quoted lines
>
> On 11/2/2020 3:43 PM, Elijah Newren wrote:
> > I have some ideas for using a histogram diff to improve content merges,
> > which fundamentally relies on the idea of a histogram.  Since the diffs
> > are never displayed to the user but just used internally for merging,
> > the typical user preference shouldn't matter anyway, and I want to make
> > sure that all my testing works with this algorithm.
> >
> > Granted, I don't yet know if those ideas will pan out and I haven't even
> > tried any of them out yet, but it's easy to change the diff algorithm in
> > the future if needed or wanted.  For now, just set it to histogram.
>
> If you are not making use of the histogram yet, then could you set this
> patch aside until you _do_ use it? Or are there performance implications
> that are also a side benefit?
Long story...

git folks tend to value performance pretty strongly -- including sometimes valuing it OVER correctness. For example, if fast-export completely munges some merge commits and you send your first ever patch in to the list to fix it (by turning on topo_order), you might run into folks asking if it should be made an option so that we don't slow down exports except for people who happen to know they need it (and thus risk breaking exports for people who happen to be unaware that they need it...). Luckily, Peff bailed me out in that situation by doing some timings and finding that topo_order actually made fast-export _faster_, to everyone's surprise at the time. Or, to take another example, perhaps someone will introduce some commit-date cutoff on the revision walking machinery that sometimes breaks the answer but makes things faster...and then causes a bunch of headaches years down the road when someone tries to introduce commit-graphs to get us always correct _and_ fast answers.

In this case, histogram diffs in my cursory investigation are about 2% slower than Myers diffs. I think others may have done even more detailed benchmarks. They've been around for years, but haven't been made the default, despite giving obviously better looking diffs to users in a number of cases where Myers diffs are unintelligible. But, far more importantly, there are real merge bugs we know about that are even affecting git.git and linux.git that I don't have a clue how to address without the additional information that I believe is provided by histogram diffs. See the following:

https://lore.kernel.org/git/20190816184051.GB13894@sigill.intra.peff.net/ https://lore.kernel.org/git/CABPp-BHvJHpSJT7sdFwfNcPn_sOXwJi3=o14qjZS3M8Rzcxe2A@mail.gmail.com/ https://lore.kernel.org/git/CABPp-BGtez4qjbtFT1hQoREfcJPmk9MzjhY5eEq1QhXT23tFOw@mail.gmail.com/

I don't like mismerges. I really don't like silent mismerges. While I am sometimes willing to make performance and correctness tradeoff, I'm much more interested in correctness in general. I want to fix the above bugs. I have not yet started doing so, but I believe histogram diff at least gives me an angle. But I can't rely on using the information from histogram diff unless it's in use. And it hasn't been used because of a few percentage performance hit.

But, since I happen to be speeding up typical non-trivial merges/rebases/cherry-picks by factors of 10 or more (at least, anywhere that read-and-update-the-index is a trivial percentage of overall time instead of 99.9% of overall time like it is for you), now is golden opportunity to switch out the underlying diff algorithm so that I can get the data I need to fix the bugs I know are there. Whether I can actually fix them is yet to be seen; I won't even start until merge-ort is complete and merged.

Does that help?
Previous: Derrick StoleeNext: Derrick Stolee
Message 62 of 83 in “fundamentals of merge-ort implementation”
  1. 00/20 fundamentals of merge-ort implementationElijah Newren, Nov 2, 2020
  2. 10/20 merge-ort: avoid recursing into identical treesElijah Newren, Nov 2, 2020
  3. Derrick StoleeNov 11, 2020
  4. 08/20 merge-ort: compute a few more useful fields for collect_merge_infoElijah Newren, Nov 2, 2020
  5. Jonathan TanNov 6, 2020
  6. Elijah NewrenNov 6, 2020
  7. Jonathan TanNov 9, 2020
  8. Elijah NewrenNov 9, 2020
  9. 11/20 merge-ort: add a preliminary simple process_entries() implementationElijah Newren, Nov 2, 2020
  10. Jonathan TanNov 11, 2020
  11. Elijah NewrenNov 12, 2020
  12. 09/20 merge-ort: record stage and auxiliary info for every pathElijah Newren, Nov 2, 2020
  13. Jonathan TanNov 6, 2020
  14. Elijah NewrenNov 7, 2020
  15. Jonathan TanNov 9, 2020
  16. Elijah NewrenNov 9, 2020
  17. Derrick StoleeNov 11, 2020
  18. Elijah NewrenNov 11, 2020
  19. Elijah NewrenNov 11, 2020
  20. Derrick StoleeNov 12, 2020
  21. Derrick StoleeNov 12, 2020
  22. 15/20 merge-ort: step 3 of tree writing -- handling subdirectories as we goElijah Newren, Nov 2, 2020
  23. Jonathan TanNov 12, 2020
  24. Elijah NewrenNov 12, 2020
  25. Elijah NewrenNov 24, 2020
  26. Jonathan TanNov 25, 2020
  27. Elijah NewrenNov 26, 2020
  28. Jonathan TanNov 30, 2020
  29. 17/20 merge-ort: add implementation of checkout()Elijah Newren, Nov 2, 2020
  30. 18/20 tree: enable cmp_cache_name_compare() to be used elsewhereElijah Newren, Nov 2, 2020
  31. 14/20 merge-ort: step 2 of tree writing -- function to create tree objectElijah Newren, Nov 2, 2020
  32. Jonathan TanNov 11, 2020
  33. Elijah NewrenNov 11, 2020
  34. 19/20 merge-ort: add implementation of record_unmerged_index_entries()Elijah Newren, Nov 2, 2020
  35. 20/20 merge-ort: free data structures in merge_finalize()Elijah Newren, Nov 2, 2020
  36. 16/20 merge-ort: basic outline for merge_switch_to_result()Elijah Newren, Nov 2, 2020
  37. 12/20 merge-ort: have process_entries operate in a defined orderElijah Newren, Nov 2, 2020
  38. Derrick StoleeNov 11, 2020
  39. Elijah NewrenNov 11, 2020
  40. 13/20 merge-ort: step 1 of tree writing -- record basenames, modes, and oidsElijah Newren, Nov 2, 2020
  41. Jonathan TanNov 11, 2020
  42. Elijah NewrenNov 11, 2020
  43. Jonathan TanNov 12, 2020
  44. 07/20 merge-ort: avoid repeating fill_tree_descriptor() on the same treeElijah Newren, Nov 2, 2020
  45. Derrick StoleeNov 11, 2020
  46. Elijah NewrenNov 11, 2020
  47. Eric SunshineNov 11, 2020
  48. 05/20 merge-ort: add an err() function similar to one from merge-recursiveElijah Newren, Nov 2, 2020
  49. Derrick StoleeNov 11, 2020
  50. Elijah NewrenNov 11, 2020
  51. Derrick StoleeNov 11, 2020
  52. 01/20 merge-ort: setup basic internal data structuresElijah Newren, Nov 2, 2020
  53. Jonathan TanNov 6, 2020
  54. Elijah NewrenNov 6, 2020
  55. Jonathan TanNov 9, 2020
  56. 02/20 merge-ort: add some high-level algorithm structureElijah Newren, Nov 2, 2020
  57. 03/20 merge-ort: port merge_start() from merge-recursiveElijah Newren, Nov 2, 2020
  58. Derrick StoleeNov 11, 2020
  59. Elijah NewrenNov 11, 2020
  60. 04/20 merge-ort: use histogram diffElijah Newren, Nov 2, 2020
  61. Derrick StoleeNov 11, 2020
  62. Elijah NewrenNov 11, 2020
  63. Derrick StoleeNov 11, 2020
  64. Elijah NewrenNov 11, 2020
  65. 06/20 merge-ort: implement a very basic collect_merge_info()Elijah Newren, Nov 2, 2020
  66. Jonathan TanNov 6, 2020
  67. Elijah NewrenNov 6, 2020
  68. Jonathan TanNov 9, 2020
  69. Derrick StoleeNov 11, 2020
  70. Elijah NewrenNov 11, 2020
  71. Derrick StoleeNov 3, 2020
  72. Elijah NewrenNov 3, 2020
  73. Elijah NewrenNov 7, 2020
  74. Derrick StoleeNov 7, 2020
  75. Elijah NewrenNov 7, 2020
  76. Derrick StoleeNov 9, 2020
  77. Elijah NewrenNov 9, 2020
  78. Derrick StoleeNov 9, 2020
  79. Elijah NewrenNov 9, 2020
  80. Derrick StoleeNov 11, 2020
  81. Elijah NewrenNov 11, 2020
  82. Derrick StoleeNov 11, 2020
  83. Elijah NewrenNov 11, 2020

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.