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

make contentChanges smaller in DidChangeTextDocumentParams #241

Merged
merged 17 commits into from
Jan 14, 2019

Conversation

mattn
Copy link
Collaborator

@mattn mattn commented Jan 13, 2019

Current implementation send whole text of buffer when text was changed. This make vim slowly. This change make it smaller using https://github.com/natebosch/vim-lsc/blob/master/autoload/lsc/diff.vim .

Thanks to @natebosch

See following screenshots. I'm editing src/compiler/checker.ts (1.8MB) in repository of typescript .

Before applying this change

omni-old

After applying this change

omni-new

I guess most of time was taken in typescript-language-server.

If the buffer is smaller, completion is very fast.

omni-new

@mattn
Copy link
Collaborator Author

mattn commented Jan 13, 2019

Please look how short the cursor stops blinking while completion of text due to this change.

@mattn
Copy link
Collaborator Author

mattn commented Jan 13, 2019

I'll fix errors by vint.

Copy link
Owner

@prabirshrestha prabirshrestha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this. This will definitely help people with large files and will also solve hangs for most of the people.

Also any plans to send let diff = diff(text1, text2) patches to vim in the future? My theory is that computing diff in C would help with the speed.

autoload/lsp.vim Outdated Show resolved Hide resolved
autoload/lsp.vim Outdated Show resolved Hide resolved
autoload/lsp.vim Outdated Show resolved Hide resolved
autoload/lsp/utils/diff.vim Show resolved Hide resolved
@prabirshrestha
Copy link
Owner

So I played with this branch for around 15mins and was impossible to hang vim. Before this it used to take me < 10sec to hang vim on large files. It is a great improvement. For now I will be using this branch until it gets merged to master. Good to see all the perf fixes coming well.

@mattn
Copy link
Collaborator Author

mattn commented Jan 14, 2019

Also any plans to send let diff = diff(text1, text2) patches to vim in the future? My theory is that computing diff in C would help with the speed.

Do you have an idea for the API? It must be generic API.

autoload/lsp.vim Show resolved Hide resolved
autoload/lsp.vim Outdated Show resolved Hide resolved
autoload/lsp.vim Outdated Show resolved Hide resolved
autoload/lsp/utils/diff.vim Show resolved Hide resolved
autoload/lsp.vim Show resolved Hide resolved
@mattn
Copy link
Collaborator Author

mattn commented Jan 14, 2019

If my english is bad, please point that.

README.md Outdated
@@ -39,6 +39,8 @@ if executable('typescript-language-server')
endif
```

vim-lsp support incremental changes of Language Server Protocol.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supports

@prabirshrestha
Copy link
Owner

👍 Approved. Tested the branch and works. Can be merged after the CI completes.
As for the vim diff support will start a separate thread on it in vim mailing list.

@prabirshrestha prabirshrestha merged commit 629a501 into master Jan 14, 2019
@prabirshrestha prabirshrestha deleted the small-diffs branch January 14, 2019 19:53
@prabirshrestha
Copy link
Owner

Merged. Thanks,

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

Successfully merging this pull request may close these issues.

2 participants