Use async callbacks where possible to prevent UI from hanging

[timfjord]

This PR tries to address a problem with the blocking of the UI when moving the cursor(selection)

Even that brackets are highlighted in a separate thread there are some lags(the app is like waiting for something) when you move the cursor quickly(I'd say relatively quickly). For example, in my case, when I moved the cursor from the beginning of the line to the end of the line a few times it sometimes took a few seconds for the cursor to finish the command. When I replaced normal callbacks with their async versions the blocking issue had gone

I can try to record a video to illustrate the problem. Also, I disabled/enabled plugins to make sure it is BracketHighlighter causing the delay

[facelessuser]

I think it is important to mention how BH works. BH takes performance as it's main priority. Originally, there was a popular bracket plugin that blocked the UI to annoying degree. BH was written to address all those issues.

First, it comes with a max threshold to prevent matching brackets too far away. It matches in a sliding vertical window.

Second, it does not auto match immediately on purpose. It acknowledges multiple edit events and cursor moving events but does not match the brackets until the events die down so that it doesn't freeze up the interface while typing, etc.

The delay of seeing brackets highlight is mainly due to this as we have a waiting period to see if the events die down before we perform the work.

I'm more concerned with smooth typing than I am with instant bracket highlighting.

Please explain in a bit more detail what you are trying to fix as it may be something we simply aren't concerned about.

[timfjord]

Sure, let me try to record a video. This PR basically addresses lags when typing or moving the cursor around I am aware of the async nature of the plugin, I know there is a delay, but, maybe on my side, I don't know, there were lags when I was moving my cursor and changing to async callbacks fixed that.

[facelessuser]

It definitely shouldn't be between fast consecutive moves. The actual payload may still be done on the main thread as I imagine the normal set_timeout executes on the main thread. Assuming this actually puts the main payload somehow in the async thread, I'd worry a little that we may get some asynchronous states at some times making matching a little less reliable as we sometimes need to check scopes of the current edit state. This applies to selections and such as well.

There isn't a lot of work between normal fast consecutive moves, and that is what is normal performed during the on_selection, on_selection_modified events. I imagine there is a little more setup with on_load, but that is only performed once on load, and on_activated is just performed when a view takes focus after not having focus.

[facelessuser]

I'd also try and test your environment with just BH. I have not experienced any typing issues with BH in the latest ST builds, so I am a bit skeptical that BH is causing any serious lag or is the sole contributor. Typing lag as been a concern from a very early point in developing BH and something considered solved long ago.

[facelessuser]

Also, please provide the OS you are running BH on, the language of open view, file size where lag is occurring, etc. I currently run BH on macOS and Windows, and only occasionally on Linux these days.

I'm definitely not saying switching to async calls is going to be rejected, but in order to be accepted, this will require extensive testing and probably very clear justification before such a significant change to the core is accepted. Basic core functionality has been pretty stable for a long time with no real major complaints. So that is why I'm skeptical and surprised to hear complaints about sudden lag issues.

[timfjord]

Sure, I will try to provide as much information as I can. What I am going to do is to test this in safe mode to make sure there is no connection with other packages

[timfjord]

@facelessuser Just want to ask one thing. Do use the LSP package together with BracketHighlighter?

[facelessuser]

I have been playing with it a little. Mainly on my Mac system and only using LSP-Pyright for Python.

[timfjord]

Ok, thanks. Anyway, I will play with this issue with all packages but BracketHighlighter turned off, because I can easily reproduce it in my normal setup

[facelessuser]

Sure. Thanks for understanding. I'm interested to see what your findings are.

[timfjord]

Ok, so here we go, please check on the second 4(I was moving the pointer to highlight the delay). It is like I pressed the key but the cursor changed the position with delay

https://user-images.githubusercontent.com/471335/126322051-4ccb9c3a-6790-4dca-b16e-17f3394aff2d.mov

It was quite tricky to record one because this lag isn't consistent. I was able to reproduce it more often but right after I started to record a screen the issue had gone 🤷🏼‍♂️

Anyway, it might be related to my setup, unfortunately, I couldn't activate the package in Safe mode. So what I did was I disabled the heaviest package(LSP) but kept the rest.

Worth mentioning that I am a ruby developer and because Ruby has quite advanced support it might be related to Ruby brackets, I don't know. I used this file https://github.com/svenfuchs/i18n-active_record/blob/master/lib/i18n/backend/active_record/translation.rb#L80 in the video

I noticed this issue right after I installed the package for the first time(a few months ago), but with my fix(async callbacks) I no longer experience it.

In order to reproduce it just open the file and simply jump between the end and beginning of the line. If it is reproducible I am sure you will notice it, otherwise, it might be something on my end. Maybe because my OS is a bit outdated(MacBook Pro, macOS Catalina, 10.15.7)

[facelessuser]

Thanks for the info. I'll look into this and see what I find.

[facelessuser]

So, I just checked out the video, and yeah, that is really hard to see in the video. I'm still not exactly sure what I'm seeing in that video. I'll try to reproduce locally. I'll mention this though. Sublime is not without some cursor issues: https://github.com/sublimehq/sublime_text/issues/4077.

I'm not saying the above as a "cop-out", only to highlight there are potentially other issues at play. I've had cases where the cursor is physically in one place, but visually somewhere else until the cursor moves again. I'm not attributing the behavior to BH, or any plugin really, but is it a possibility that some plugins (BH included) not using some async functions is the cause of it? Maybe. Is it possible that some plugins exacerbate the issue? Maybe.

Hopefully, I can get a better understanding of the issue when I play with it a bit more. Assuming I can attribute some of this behavior to BH, I still need to make sure that I am not running into issues by switching to this model (using async). Considering the payload isn't directly executed in these functions, I assume it may be fine, but we'll have to test to be sure.

[timfjord]

Yeah, sorry about the video, I can imagine how confusing it was Let me explain what I did there. So I was trying to press the keys with an equal interval(around 2-3 times per second) to kind of replicated moving around the text. During the first 4 seconds, the cursor moved right after I pressed the keys(like it should) but on the second 4-5 there was a delay, it was like I pressed the key but the cursor jumped only after 1/1.5 seconds. Sometimes I experienced even longer delay, up to 3-4 sec, I literally could count to 4 after pressing the key and only then it jumped to the desired position.

I am not sure if it is because of some intersection with other plugins or maybe there is a bug in Sublime(as sometimes some operations take longer than usual in Sublime, but very rarely) or maybe even because I move around the text quite often and tend to use the trackpad as little as possible, but I am almost sure that with async fix those issues are not noticeable, however there are still some delays on some operations but they most likely Sublime issues

I understand that the proposed change is quite fundamental especially if this case can be related to my setup only. I will anyway continue to use this plugin with my fix on top as this plugin has become a part of my must-have(by the way, thanks for it) and once I have some free time I will try to debug it further.

[facelessuser]

Understood. I'm just trying to set expectations before I look into it. I'm potentially okay migrating the functions to async even if there is no issue. The payload will still probably be in the main thread, but these event based checks are probably fine being async. I just need time to ensure whether there is an actual problem and test that the proposed fix has no side effects.

[facelessuser]

A new issue was created so I don't forget to look into this. The new issue is #592.