Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor undo #1727

Closed
Chillee opened this issue May 20, 2017 · 3 comments
Closed

Refactor undo #1727

Chillee opened this issue May 20, 2017 · 3 comments

Comments

@Chillee
Copy link
Member

Chillee commented May 20, 2017

I feel like our undo is unnecessarily slow right now.

Try :sort or something on a fairly large file, and you'll see that although we finish the text update fairly quickly, it takes a lot longer before it becomes responsive again.

That's because this line takes such a long time.

https://github.com/VSCodeVim/Vim/blob/master/src/history/historyTracker.ts#L387

Our undo also has some other issues related to undoing large changes.

As for approaches to take, I see two major branches we can go down:

  1. Try to leverage VSCode's default API undo stops. The main advantage of this, obviously, is that it should be the fastest possible approach. In addition, it also seems relatively simple.

  2. Use Neovim for its undo functionality. The biggest advantage of this is that it lets use an UNDO TREE! Wow! As well as all the other Vim undo features (:earlier, etc.) Main disadvantage is that it's almost definitely slower, and that it ties us even close to a neovim dependency (which isn't necessarily terrible).

@johnfn
Copy link
Member

johnfn commented May 20, 2017

Yes, we need to diff the file against itself, which is O(n). On a large enough file that can be expensive.

This is one thing that will be fixed when we use VSCode's history tracking stuff instead of our own.

@jleclanche
Copy link

Also see #1490, #928, #565

@J-Fields
Copy link
Member

Re: performance, I've made some changes in the last few months so we do a full file diff only when necessary.

Long-term plan is #1490 and possibly #5206; closing in favor of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants