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

perf!: consolidated updates to address performance issues #868

Merged
merged 4 commits into from
Jul 17, 2023

Conversation

Jint-lzxy
Copy link
Collaborator

This fixes #866 and closes #867.

cc @fecet

This fixes #866 and closes #867.

---------

Signed-off-by: Jint-lzxy <50296129+Jint-lzxy@users.noreply.github.com>
Co-authored-by: Layton <caolei6767@google.com>
@ayamir
Copy link
Owner

ayamir commented Jul 17, 2023

Sorry, I can't see the improvement of the scroll velocity when open this file.

What's more, load_big_files_faster option doesn't work for this file too. The startuptime keeps 20 seconds whether enable it or not.
image

image

@fecet
Copy link
Contributor

fecet commented Jul 17, 2023

I also haven't observed any performance improvement, and I don't use scrollview and neoscroll. The performance issue should be unrelated to them.

@Jint-lzxy
Copy link
Collaborator Author

Sorry, I can't see the improvement of the scroll velocity when open this file.

Hmmm I somehow cannot reproduce this:
info

The current startup time on my side is close to the that of nvim --clean 🤔

Perhaps explaining some background may be helpful? What this PR hopes to address is:

  • The startup time wrt opening files of normal sizes - by deferring LSP setup to prevent them from interfering with startup of those UI components;
  • Open big files when load_big_files_faster is true - Stop loading most plugins to simulate the behavior of native nvim.

If the startup time of any of the above situations is still slow, it may be because:

:lua vim.treesitter.get_parser():parse()

This command should take a long time to complete.

@ayamir
Copy link
Owner

ayamir commented Jul 17, 2023

Now it ok for me after Lazy sync 👍

@Jint-lzxy
Copy link
Collaborator Author

cc @fecet

@CharlesChiuGit
Copy link
Collaborator

CharlesChiuGit commented Jul 17, 2023

Does this PR need further modifications? Or is it ready to go?

@Jint-lzxy
Copy link
Collaborator Author

I think it's good to go. Currently waiting for @fecet's response as he reported very limited performance improvements.

@fecet
Copy link
Contributor

fecet commented Jul 17, 2023

Still don't find performance improvement unless set

return require("rainbow-delimiters").strategy["local"]

this to global.

lua/modules/plugins/editor.lua Outdated Show resolved Hide resolved
lua/modules/configs/editor/rainbow_delims.lua Outdated Show resolved Hide resolved
@Jint-lzxy
Copy link
Collaborator Author

@fecet So u're using treesitter on large files? We don't make any gurantee regarding treesitter-related components under big files, as using strategy["global"] only reduces the number of times LanguageTree:parse() gets called, but each call could still leads to significant performance degradation. To verify this execute the following command urself:

:lua vim.treesitter.get_parser():parse()

This can only be resolved by updating the related parser(s), we cannot do anything. But yes we can use simpler processing methods on relatively larger files, a.k.a, strategy["global"]. Will push a fix for this.

@fecet
Copy link
Contributor

fecet commented Jul 17, 2023

The file I tested is dwm.cpp , it's not so large I guess? I don't really understand how ts works so cannot give further comment.

@Jint-lzxy
Copy link
Collaborator Author

The file I tested is dwm.cpp , it's not so large I guess? I don't really understand how ts works so cannot give further comment.

Thanks for the sample! Will do some testings there.

@Jint-lzxy
Copy link
Collaborator Author

Still don't find performance improvement

I should have identified the problem. Now rainbow-delimiters will select an appropriate subtree when the cursor moves, but due to the issue with cpp parser, the generated AST becomes too tall, making one iteration too expensive.

Copy link
Collaborator

@CharlesChiuGit CharlesChiuGit left a comment

Choose a reason for hiding this comment

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

LGTM

@Jint-lzxy
Copy link
Collaborator Author

@fecet ec24fef should fix ur issue 👍

@fecet
Copy link
Contributor

fecet commented Jul 17, 2023

OK, it's good for me now

@Jint-lzxy Jint-lzxy merged commit 1165861 into main Jul 17, 2023
4 checks passed
@Jint-lzxy Jint-lzxy deleted the perf/fix-plugin-load-seq branch July 17, 2023 11:43
singlemancombat pushed a commit to singlemancombat/nvim-config that referenced this pull request Jul 17, 2023
* perf!: consolidated updates to address performance issues

This fixes ayamir#866 and closes ayamir#867.

* fixup! perf!: consolidated updates to address performance issues

* revert: `nvim-bufdel` shall always be available (API calls)

* feat(rainbow_delims): line detections for c/cpp

---------

Signed-off-by: Jint-lzxy <50296129+Jint-lzxy@users.noreply.github.com>
Co-authored-by: Layton <caolei6767@google.com>
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.

Very slow scroll
4 participants