-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
YCM blocks vim UI thread during ycmd startup #2071
Comments
Huh, I'm surprised that's the case. We call python's @micbou @puremourning @vheon Thoughts? If we indeed do block on ycmd startup, that sounds like an area ripe for a perf improvement. |
The issue is that when loading the global |
Any improvement to startup time in general wins cookies. |
That sounds unfortunate but livable.
That's the part that shouldn't happen, especially the UI blocking. Ugh. :( |
We definitely block the UI on buffer read. I just found this while I had ycmd paused in the debugger. Vim hangs until ycmd responds after doing something like |
Also, further notes:
Detail is something like
|
I have a solution for this YcmCorePreload blocking Vim. I'll send a PR sometime soon. For background, the solution is to make the |
@puremourning. If the PR still isn't ready, would you mind providing a bit more detail so I (and others who face the issue) can get around this? It's been a serious thorn in my use of YCM |
It looks like I wrote a fair amount of info on this thread. Suffice to say that it is fiddly internals. Anyway, the simplest thing would be to avoid doing slow stuff in the core preload call. I know that's not super hearty, but short of rejiggling the vim client internals around this, there aren't realm any workarounds. |
Fair enough, poked around a bit more and I can see what you mean now, agreed (and thanks). |
@puremourning Any updates on the PR? I am still facing massive UI freezes while starting neovim. |
I guess you can infer from the open nature of the issue that the answer is the status is very much quo |
For anyone else plagued by the issue, last I checked, this fork simply deletes the offending code (clearly this changes the behaviour but I was happy with it) |
[READY] Rely on connect timeout instead of checking that the server is alive Currently, we always check that the ycmd process is up (with the `IsServerAlive` method) before sending a request. Without this check, each request could block Vim until a `NewConnectionError` exception is raised if the server crashed. This is the case on Windows where it takes ~1s before the exception is raised which makes Vim unusable. However, even with this check, Vim may still be blocked in the following cases: - the server crashes just after the check but before sending the request; - the server is up but unresponsive (e.g. its port is closed). To avoid both cases, we instead use [the connect timeout parameter from Requests](http://docs.python-requests.org/en/master/user/advanced/?highlight=connect%20timeout#timeouts) and set it to a duration sufficiently short (10 ms) that the blocking can't be noticed by the user. Since the server is supposed to run locally (this is what YCM is designed for), 10ms is largely enough to establish a connection. The `IsServerAlive` check is removed almost everywhere except in `OnFileReadyToParse` because we still want to notify the user if the server crashed. This change makes it possible to not have to [wait for the server to be healthy before sending asynchronous requests](https://github.com/Valloric/YouCompleteMe/blob/master/python/ycm/client/base_request.py#L137-L138). This will dramatically improve startup time (see issue #2085) and fixes #2071. Next PR once this one is merged. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2514) <!-- Reviewable:end -->
Still not fixed but soon. |
[READY] Remove healthy check This PR removes the code that, if the server is not yet healthy, tries in a separate thread to send the same request at different intervals until it gets a response or reaches 5 failed attempts. Main issue with that code is that it blocks Vim if a synchronous request (`semantic_completion_available`, `run_completer_command`, etc.) is sent while the server is not up. See issue #2071. Other issues are that it's hard to understand how this code behaves (it combines threads and retries), it becomes useless once the server is healthy (because `SERVER_HEALTHY` is cached and never reset), it depends on arbitrary parameters (why 5 retries with a delay of 0.5s multiplied by 1.5 at each attempt?), and it has a package dependency `retries`. Now that we catch all server exceptions (PR #2453) and define a very short connection timeout (PR #2514), we can just drop this code. This change gives a huge improvement in startup time on Windows (don't worry, more improvements are coming for all platforms). The reason is that `requests` (probably `urllib3`) tries for ~1s to check if the server is healthy on Windows while it's almost instantaneous on other platforms. <table> <tr> <th rowspan="2">Platform</th> <th colspan="2">First run (ms)</th> <th colspan="2">Subsequent runs (ms)</th> </tr> <tr> <td>Before</td> <td>After</td> <td>Before</td> <td>After</td> </tr> <tr> <td>Ubuntu 16.04 64-bit</td> <td>228</td> <td>248</td> <td>176</td> <td>151</td> </tr> <tr> <td>macOS 10.12</td> <td>433</td> <td>417</td> <td>263</td> <td>263</td> </tr> <tr> <td>Windows 10 64-bit</td> <td>1861</td> <td>844</td> <td>814</td> <td>292</td> </tr> </table> These results were obtained by running the `prof.py` script from [this branch](https://github.com/micbou/YouCompleteMe/tree/profiling-startup). The difference between first run and subsequent runs is Python bytecode generation (`*.pyc` files). Fixes #2071. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2517) <!-- Reviewable:end -->
To repro, create ycm-extra-conf with:
import time
def YcmCorePreload():
time.sleep(10)
$ vim
<now anything you type will be delayed by multiple seconds, until the first connection times out>
@d0k
The text was updated successfully, but these errors were encountered: