perf: prevent multiple calls of M.refresh() by autocmds

[hinell]

• eliminates performance bottleneck in big files
• prevents multiple async calls to M.refresh(), by autocmds
• refactor debounce timer

Overview

While profiling, I've discovered that M.refresh() is actually called twice many times. This should be fixed now.

Demo

Demo (run in bash) 50K LOCs:

cd PATH_TO_IBL_REPO
gh pr checkout 790
nvim -c "set ft=typescript" <(curl -L https://raw.githubusercontent.com/microsoft/TypeScript/main/src/compiler/parser.ts)

Related

• 776

Profiling files

For comparison:

• ibl.original.profile.txt - from master branch
• ibl.skip-doublerefresh-new-timer.profile.txt - current PR

[lukas-reineke]

There are probably cases where this can be optimized, but this PR removes important functionality.

The first call to debounced_refresh should not be debounced, or there is noticeable lag when first opening a buffer, or jumping to a section that was not rendered yet. Creating and removing the timer on each refresh is going to hurt performance. There are cases that need to take priority and rerender, just not allowing this will cause bugs.

[hinell]

Reverted. Please see.

[lukas-reineke]

There are cases that need to take priority and rerender, just not allowing this will cause bugs.

This is still true. Just not allowing the refresh is not possible. I took another look at the code, and there is one small issue I found that leads to multiple refresh calls. But when testing it did not happen often enough to make a difference I think. #791

[hinell]

There are cases that need to take priority and rerender, just not allowing this will cause bugs.

This is still true. Just not allowing the refresh is not possible.

This PR skips M.refresh from running again if it's already running asynchronously by some other autocmd. Lua puts multiple M.refresh calls for the same buffer on stack producing huge overhead otherwise.

Btw, the lag is still noticiable even with old timer approach.

[lukas-reineke]

This PR skips M.refresh from running again if it's already running asynchronously by some other autocmd.

Which will cause bugs. Some events need to run even if the buffer is already being refreshed, like changes to the config. Every none important event goes through the debounce already.

Lua puts multiple M.refresh calls for the same buffer on stack producing huge overhead otherwise

If this happens to you, it probably means the execution of refresh takes longer than the debounce. In that case, you need to set debounce higher. I am fairly confident that there are no duplicate triggers of refresh that don't get debounced. If there are, please show me where.

[hinell]

The performance gain is obvious. I haven't observed any bugs so far.

I wouldn't really want M.refresh() to race, regardless what it does inside.

Best.

[lukas-reineke]

The performance gain is obvious

You will get the same performance gain by increasing debounce. This is working like it should. You can easily test it by adding a counter to refresh, and setting debounce to a high value, like 1 second. Refresh won't be called more than once a second.

I wouldn't really want M.refresh() to race, regardless what it does inside.

refresh updates the buffer linearly from top to bottom. It is safe, and necessary, to run it before the previous execution is done. But only in special cases, normal moving around the buffer will not cause this.

Unless refresh takes longer than what debounce is set to (in which case you need to update your config), there won't be multiple calls to refresh for the same buffer at the same time. With the exception of special events like config changes.

If there are for you, then there is a bug somewhere.