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

fix high cpu #121

Merged
merged 7 commits into from
Feb 25, 2024
Merged

fix high cpu #121

merged 7 commits into from
Feb 25, 2024

Conversation

jinzhongjia
Copy link
Contributor

Recently, my main development environment was changed to windows. I found that the event used by git-blame is cursormoved, and this event is triggered very frequently.

Git itself on windows runs slower, which causes the entire neovim to be blocked

I replaced cursormoved with cursorhold

@jinzhongjia
Copy link
Contributor Author

jinzhongjia commented Jan 30, 2024

for some users, they may like using cursormoved to get an immediate response, we can implement one debounce function

@jinzhongjia
Copy link
Contributor Author

I decided to implement additional debounce functionality, although CursorHold is enough for me

@jinzhongjia
Copy link
Contributor Author

jinzhongjia commented Feb 1, 2024

This pr add three config options:

  • gitblame_schedule_event
  • gitblame_clear_event
    Instructions have been added to the readme

@f-person
Copy link
Owner

f-person commented Feb 1, 2024

Hi! Thanks for the PR! I'll check it in the near future!
In the meantime, is the debounce functionality different from https://github.com/f-person/git-blame.nvim?tab=readme-ov-file#visual-delay-for-displaying-the-blame-info?

@jinzhongjia
Copy link
Contributor Author

jinzhongjia commented Feb 2, 2024

Oh, I didn't notice this before
I implemented debounce, schedule and clear virtual text for both

@jinzhongjia
Copy link
Contributor Author

Now, I think every thing is ok, current code only enhances the original functions, adds event configurations for schedule_show_info_display and clear_virtual_text, and debounce time is 250 milliseconds by default.

Copy link
Collaborator

@bossley9 bossley9 left a comment

Choose a reason for hiding this comment

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

This is great! I left a few comments mostly on pull request polishing.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
lua/gitblame/init.lua Show resolved Hide resolved
lua/gitblame/init.lua Outdated Show resolved Hide resolved
jinzhongjia and others added 2 commits February 8, 2024 09:17
Co-authored-by: Sam Bossley <sam@bossley.xyz>
README.md Outdated Show resolved Hide resolved
@bossley9
Copy link
Collaborator

bossley9 commented Feb 8, 2024

Other than that, I'm ok with this as long as @f-person is ok with it! 🚀

Co-authored-by: Sam Bossley <sam@bossley.xyz>
@jinzhongjia
Copy link
Contributor Author

ok, thx

@bossley9
Copy link
Collaborator

@f-person have you had any time to take a look at this?

Copy link
Owner

@f-person f-person left a comment

Choose a reason for hiding this comment

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

thank you, this is really great! i approve this pr, but left a comment; let's discuss that matter a bit further and maybe merge it or decide to do something else. looking forward from hearing back from you! thanks! <3

README.md Show resolved Hide resolved
@bossley9
Copy link
Collaborator

I linked the issues you mentioned since it is very likely this PR will fix them. The best way to test a big repository is to clone https://github.com/nixos/nixpkgs and try to open pkgs/top-level/all-packages.nix 😄

@jinzhongjia
Copy link
Contributor Author

jinzhongjia commented Feb 25, 2024

I tried it(nixpkgs pkgs/top-level/all-packages.nix) briefly. At least for now, gitblame will not prevent me from scrolling, but when git gets the commit message in the background, there will still be a high CPU usage.
Both windows and linux are

@jinzhongjia
Copy link
Contributor Author

jinzhongjia commented Feb 25, 2024

By the way, it is recommended to close all treesitter related plugins during testing, including itself, because the performance of treesitter is really poor.
Only turning on treesitter related plugins will prevent me from scrolling. . .

@f-person
Copy link
Owner

Okay, let's merge this! Thank you very much for the effort on this, @jinzhongjia & @bossley9!! <3
Ping me if you're ever in the Netherlands, and I'll get you a coffee or a beer :)

@f-person f-person merged commit ed84c1c into f-person:master Feb 25, 2024
@f-person
Copy link
Owner

Maybe clearing the existing message should be done immediately on cursor move and not whenever we set the next one?

@jinzhongjia
Copy link
Contributor Author

jinzhongjia commented Feb 25, 2024

I think that's ok, let me create a new pr
Well, the amount of code is a bit too much, so I won’t createa new PR for the time being.

f-person added a commit that referenced this pull request Apr 1, 2024
The PR #121 introduced some side-effects which made the plugin feel more
laggy overall because of how the virtual text got cleared with a delay.
This commit retains the original delay behavior for displaying, while immediately
clearing the virtual text on cursor move.
f-person added a commit that referenced this pull request May 28, 2024
The PR #121 introduced some side-effects which made the plugin feel more
laggy overall because of how the virtual text got cleared with a delay.
This commit retains the original delay behavior for displaying, while immediately
clearing the virtual text on cursor move.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants