sublimehq / sublime_text

Issue tracker for Sublime Text
https://www.sublimetext.com
803 stars 39 forks source link

Processing large number of completions blocks the main ST process #6249

Open rchl opened 8 months ago

rchl commented 8 months ago

Description of the bug

When there is a very large number of completions (where definition of "very large" varies per machine), ST blocks the main process while processing the completions, resulting in choppy editing experience.

Steps to reproduce

  1. Create a plugin:
import sublime
import sublime_plugin
import time

COMPLETIONS_COUNT = 1000000
COMPLETIONS_FLAGS = sublime.INHIBIT_WORD_COMPLETIONS | sublime.INHIBIT_EXPLICIT_COMPLETIONS | sublime.DYNAMIC_COMPLETIONS
completions = []

def plugin_loaded():
    generate_completions()

def generate_completions():
    global completions

    completions = [
        sublime.CompletionItem(
            str(i),
            annotation=str(i),
            kind=sublime.KIND_FUNCTION,
            details='abc'
        )
        for i in range(1, COMPLETIONS_COUNT)
    ]

class CompletionListener(sublime_plugin.ViewEventListener):
    def __init__(self, view: sublime.View):
        super().__init__(view)
        self.last_modified_ms = 0

    def on_query_completions(self, prefix, locations):
        completion_list = sublime.CompletionList()
        completion_list.set_completions(completions, flags=COMPLETIONS_FLAGS)
        return completion_list

    def on_modified(self):
        now = time.time()
        since_modified = now - self.last_modified_ms
        self.last_modified_ms = now
        print('seconds since previous on_modifed', since_modified)
  1. Open new view (can be plain text) and trigger completions with (ctrl+space or cmd+space).
  2. Type "123" all at once quickly
  3. Open ST console to see some additional timing logs.

Expected behavior

The main process should not be blocking on typing "1", "2" and "3".

Or it should be optimized, if possible, so that the lag is not noticeable (but it will still be possible to trigger lag with edge cases).

Actual behavior

The process blocks on typing "123" and nothing can be done until all completions are processed and completions popup is shown. This can take different amount of time depending on the number of completions and the machine the ST is running on but on my mac M2 pro, with 1000000 completions, ST blocks for over a second on each completions query.

Sublime Text build number

4168

Operating system & version

macOS

(Linux) Desktop environment and/or window manager

No response

Additional information

The plugin is logging the time it takes between "on_modified" notifications to roughly show how long the process is blocked for each completions query which might be useful for debugging.

OpenGL context information

No response

rchl commented 8 months ago

Creating CompletionItem with just a "trigger" seems to make no significant difference to how fast ST processes completions.

deathaxe commented 8 months ago

Execution time of generate_completions() is already round about 1.4s on my Ryzen 5600X box.

Most of sublime.CompletionList.set_completions()'s execution time, is also caused by all CompletionItem objects being converted to tuples in preparation for sending them to ST.

It's not ideal that GUI is blocked while processing completions, as it results in poor editing experience.

Most of the delay is a result of python's limited performance. So even without GUI blocking it is to expect so many completions show up several seconds late.

rchl commented 8 months ago

Execution time of generate_completions() is already round about 1.4s on my Ryzen 5600X box.

Note that this part is not that relevant to the issue. It happens on plugin load once and in a LSP scenario it would happen in a separate process.

deathaxe commented 8 months ago

This comparison was made to clarify, that even creating those completions is a very expensive operation, which takes too much time. In a real world plugin, such as LSP those completions would be created at runtime and would be relevant for the delay, even though this is not the part, which blocks the GUI.

The point is python being too slow to handle such massive amounts of completions. So there's little which ST can do, except avoiding GUI to block while processing.

BenjaminSchaaf commented 7 months ago

Note that while we can and should make this faster (decoding completions and setting up the auto-complete is taking a substantial amount of time), a big part of why this is slow is unfortunately just python. There are some things we can do to make things faster on the python side but fundamentally making 1M objects is just slow in python. That generate_completions takes 2 seconds on my machine for instance.

The only real ways to manage that are:

  1. Cache the CompletionItems (which already happens in this example)
  2. Don't have as many completions
  3. Call set_completions (and create the items) in a background thread
rchl commented 7 months ago

I've mentioned this before but will say it again: in case of the LSP, we are creating completions on the async thread so while the completions can be slow to appear at least that doesn't block the main thread which is the main issue here.

(blocking the async thread for a long time is also not ideal but lets leave that discussion out of here for now)

rchl commented 7 months ago

3. Call set_completions (and create the items) in a background thread

So you are also mentioning calling set_completions on the async thread... We have this code in LSP:

        # Resolve on the main thread to prevent any sort of data race for _set_target (see sublime_plugin.py).
        sublime.set_timeout(lambda: clist.set_completions(completions, flags))

which suggests that doing that is currently problematic.

deathaxe commented 7 months ago

(blocking the async thread for a long time is also not ideal but lets leave that discussion out of here for now)

Maybe plugins, heavily using async functions should maintain their own background thread, instead of relying on ST's global one.

rchl commented 7 months ago

Not sure if that's feasible in all cases. If a plugin has to interact with *_async ST APIs then introducing another background thread to the mix can result in race conditions. It's hard to say whether something like that would be feasible in a complex plugin like LSP.

deathaxe commented 7 months ago

Preventing or blocking other plugins due to long lasting expensive tasks is not ideal, too.

You are right about _async APIs becoming pointless, then as synchronous API's are perfect match to add tasks to a dedicated background thread's queue as former would just mean useless extra delay. Adding a task to a queue is sheep enough to not cause performance issues when done right.

GitGutter is doing exactly that. Using only synchronous APIs and pushing tasks to a permanently running background thread. If all logic of a plugin (e.g. LSP) uses its own background thread, race conditions should not be of an issue and it would in worst case block its own concurrently running background tasks if a single one is very expensive, without affecting others (which is only partly right due to GIL, but well).

rchl commented 7 months ago

ST also seems to have special handling for when calling ST API from async thread - it blocks the async thread until task on the main thread is done. A lot of code relies on that currently and, even if possible to replicate with custom thread, it probably wouldn't be a simple change.

In any case, I don't even know if a background thread would provide any solution for this issue because I don't quite recall what the issue with set_completions that I've mentioned in https://github.com/sublimehq/sublime_text/issues/6249#issuecomment-1918665273 is about exactly. But I feel like it easily could have the same issue with custom thread.

BenjaminSchaaf commented 7 months ago

After some optimizations I've gotten to the point where I can have 50k completions without being annoyed by the latency - previously this was 5k. That's all the low hanging fruit I could find, so we'll see how this performs in the next build.

predragnikolic commented 6 months ago

build v4168

gif ![output4168](https://github.com/sublimehq/sublime_text/assets/22029477/23278268-d45b-4c56-9e34-fe5ac907dda1)

build v4170

gif ![output4170](https://github.com/sublimehq/sublime_text/assets/22029477/90b22128-db35-4dc9-9eef-261fb74cbd1a)

While I can 4170 being faster than 4168 the delay is still noticeable.

I would not close this ticket until the delay is fixed. I think that limits must be set on some side.

What is the difference if someone offers you: a) 1_000_000 options and b) offering no options at all.

Is it useful if someone get that many completions? No, not really. (speaking for myself)

Is it useful if someone get no completions at all? No.

The only difference is that getting no options at all is cheaper, and will not block the UI that much :)

In LSP, there are at least two servers which can generate a lot of completions tailwindcss and typescript and most people complaints in ST are directed to them, but I can see this happening in others servers as well.