Significantly improve performance of :reload

[pascalkuthe]

Closes #4228 and #433

When :reload is called helix currently uses similar to compare
the on-disk file with the current state, to create a changeset that can be added to the history.
Similar implements a basic version of myers algorithm.
However, myers algorithm without additional enhancements quickly behaves quadratic over the filesize.
The implementation in similar also has many inefficiencies which lead to huge memory consumption as reported in #4228.
The situation is worsened by the fact that similar requires random access to the collections it is diffing.
This requires us to convert the ropes to a String (essentially duplicating the entire file in memory) before performing the diff.
Finally, helix used a char diff over the whole file.
While this produces ChangeSets this leads to extremely poor performance due to the large number of tokens (minimal changes).

Other popular tools that use myers algorithm (GNU diff and git for example) remedy this problem using a couple techniques:

• perform a line diff
• intern all lines before performing the diff (makes comparisons O(1)`` instead of O(n)`, which are one of the main operations during myer diff)
• preprocess the file by removing lines that never occur in the other file
• abort the snake search using two heuristics (one that aborts if a "good enough" snake is found and a second that uses the best snake after a hard iteration limit)

With these optimizations myers diff shows very good performance even for large files/diffs.
I have ported gits implement of myers algorithm to rust and published it as the imara-diff crate.
Furthermore, I have also ported gits implementation of a histogram diff to this crate.
This algorithm provides more human-readable output similar to patience diff while performing on par with (or better than) myers algorithm.
Compared to myers diff, the version in git wasn't actually all that optimized.
After the initial port I further optimized my histogram implementation, the final version beats myers algorithm for every line-diff I have tested by 5%-100%.
The repo contains detailed benchmarks and comparisons between the two algorithms and similar.
Note that the benchmarks shown there are performed on source code (linux kernel, rustc) and not on huge (500MB+) files like #4228.

This PR changes the compare-ropes implementation in helix-core to use imara-diff instead of similar.
The implementation performs a line-diff using the histogram algorithm as it generally performs better.
A char diff with myers algorithm is then performed on each changed hunk, because the histogram heuristic does not work well for char-diffs.
This ensures that the generated change sets remain reasonably small while offering much better performance.

I have stress tested this PR by generating a 1 GB file with:

base64 /dev/urandom | head -c 1000000000 > file.tx

Afterwards I performed multiple tests by editing/deleting/copying in random places and then reloading.
Another test I performed was repeating the above command while helix was open and then reloading.

All tests made helix spin forever on master (and use up to 50GB of RAM before being killed).
With this PR helix only took a few seconds (up to 20 seconds for complete replacement) and 3.6GB peak RAM usage.

Alternative Approaches

@kirawi suggested the rsync algorithm or the xi_rope algorithm, which is somewhat similar.
While these algorithms are faster they do not fit within helix without significant changes, because they provide a fundamentally different output:
A sequence of insertions and random copies from the base document.
For example consider the following diff:

foo
foo
bar

foo
bar
foo

An (ideal) rsync algorithm would produce a diff like this:

Copy(0) // foo  
Copy(2) // bar
Copy(0) // foo

By comparison a diff algorithm will produce:

Retain(1) //foo
Delete(1) //remove foo
Retain(1) //bar
Insert("foo")

This kind of output can not be converted to the (reversible) edit-sequences helix uses.
It might be feasible to refactor helix to store the undo/redo transactions separately in a format more akin to the output of rsync.
However, this would require a large refactor of a core component (Transaction) that would touch many parts of the editor.

This PR only requires minimal changes while providing large performance wins

[archseer]

Nice work! Do you have a flamegraph of the new algorithm available?

[pascalkuthe]

Nice work! Do you have a flamegraph of the new algorithm available?

The below flamegraph is for reloading a 1GB file with a single line removed.
As you can see most of the time is spent interning the input lines.
For non-trivial source-code diff (larger diffs, smaller files) the diff itself usually takes more time (here it takes 2% of time, while benchmarking the repo it takes around 20% - 50%). As you can see improving the Lines iter performance would help a lot, I might look into sending a PR for ropey in the future.

Coming up with a single representative testcase is kind of difficult, that's why I took the more quantitative approach with benchmarks in the imara-diff repo.

[pickfire]

Looks good to me.

[pascalkuthe]

This PR is affected by two bugs in ropey:

• a rare panic in the PartialEq implementation when using non ascii characters: Fix panic when comparing ropes with chunks that do not align at char bounds cessen/ropey#65
• an incorrect Hash implementation that causes the Hasher (ahash) used by the imara-diff interner to frequently detect identical lines as changed lines: fix Hash implementation cessen/ropey#66

These bugs don't really break this PR, because the panic is pretty rate and the incorrect hashes just causes imara-diff to detect more changes than there actually are in a file (the good news is that this PR will get even faster with that fix). While unoptimal deleting and inserting the same line again is a valid transaction and does not cause problems.

We might still want to hold off merging this until the upstream issues are fixed (I am torn on this).
Note that the previous reload implementation was not affected because it used a char diff (so it hashed and compared chars instead of RopeSlice)

[archseer]

Great work! I think it's ok to wait for the fixes to land in ropey, it shouldn't take too long. Maybe we can attempt to resolve cessen/ropey#25 and get it all into 1.6.0. There's already a couple of other iterators that do similar book-keeping so we don't have to fully reinvent the wheel.

[pascalkuthe]

Great work! I think it's ok to wait for the fixes to land in ropey, it shouldn't take too long. Maybe we can attempt to resolve cessen/ropey#25 and get it all into 1.6.0. There's already a couple of other iterators that do similar book-keeping so we don't have to fully reinvent the wheel.

Sounds like a plan 👍 I think it would also be great to land cessen/ropey#67. The bug fixed there doesn't effect this PR (or at least not more then the rest of helix) but it could potentially cause memory safety issues (not 100% sure if it does but it does create a rope which violates its internal invariants so all bets are off). I will try my hand at implementing a line iterator that fixes cessen/ropey#25, Oonce I have a prototype I will post the PR here.

[poliorcetics]

I'm working on cessen/ropey#25 @pascalkuthe, I should have a solution within the month, preliminary results are good :)

[pascalkuthe]

I'm working on cessen/ropey#25 @pascalkuthe, I should have a solution within the month, preliminary results are good :)

That's a bit unfortunate timing we did some duplicate work here. I already had an optimized version of forward iteration ready and only had a couple bugs left to iron out in reverse iteration. Life just got in the way this weekend/this week so I didn't get around to finishing those last few edgecases and posting a PR yet. The implementation was a bit subtle, I had to use a few tricks to avoid doing duplicate work when creating the subslices. Also there was a bug in the original implementation of reverse iteration or Light rope slices that did not have a trailing newline. Perhaps it makes sense to combine efforts in some form. Could you upload your implementation somewhere and I could compare it to mine?

[pascalkuthe]

My line iterator implementation (cessen/ropey#70) now works decently well (it was really tricky to get all edge cases right, luckily ropey has a great testsuite). That PR alone brought the total time (exlcuding io) to reload a 1GB file down from 14 seconds to 7 so it was really worth it to invest the time here.

Until the new ropey version releases I have pointed this PR at my fork of ropey that contains all relevant improvements for people that already want to test it.

[archseer]

With these improvements, does it still make sense to do the git gutter diffing in an async task or could we do it sync?

[pascalkuthe]

With these improvements, does it still make sense to do the git gutter diffing in an async task or could we do it sync?

Good point, my gut feeling is the anwser is yes because even if a diff is really fast it still is an expensive full file computation and would still introduce quite a bit of latency into the core edit loop.I would likd do some testing to determine the worst case impact on latency of doing the diff sync.

[pascalkuthe]

ropey-1.5.1-alpha contains the necessary fixes for this PR so we are not blocked on upstream anymore

[David-Else]

Is there a reason this is using an old version? We have Version 0.1.5 https://docs.rs/imara-diff/latest/imara_diff/

[pascalkuthe]

Those were just bugfixes that did not affect this PR. The newest version should also be in the lockfile (cargo updates to the largest compatible version unless you pin the dependency with ^) so I doesn't make a difference anyway