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

Increased time to first diagnostic/language server initialization with 1.14.2 #386

Closed
mjlbach opened this issue Feb 7, 2021 · 21 comments
Closed
Labels
bug Something isn't working

Comments

@mjlbach
Copy link
Contributor

mjlbach commented Feb 7, 2021

Describe the bug
@clason and I noticed an increase in time to first diagnostic on macOS for 1.14.2. It used to provide diagnostics almost instantly on a file, but now it seems to take approximately 30 seconds before giving some timeout messages

LSP[sumneko_lua][Info] Too large file: test/unit/viml/expressions/parser_tests.lua skipped
. The currently set size limit is: 100 KB, and the file size is: 212.211 KB.
LSP[sumneko_lua][Info] Too large file: test/functional/plugin/shada_spec.lua skipped. The
currently set size limit is: 100 KB, and the file size is: 104.13 KB.
LSP[sumneko_lua][Info] Too large file: test/unit/eval/typval_spec.lua skipped. The current
ly set size limit is: 100 KB, and the file size is: 119.11 KB.

To Reproduce
Steps to reproduce the behavior:

  1. Clone the neovim source repo
  2. Open diagnostic.lua
  3. runtime/lua/vim/lsp/diagnostic.lua
  4. Wait 30 seconds

Expected behavior
Prior to 723f3d2 diagnostics were pretty much instant. This commit does reduce CPU usage though at idle, which is great.

Environment (please complete the following information):

  • OS: macOS
  • Client: neovim
@mjlbach mjlbach changed the title Increased time to first diagnostic with 1.14.2 Increased time to first diagnostic/language server initialization with 1.14.2 Feb 7, 2021
@clason
Copy link

clason commented Feb 7, 2021

Same here (Intel macOS); last good version (no error, instant response on startup) was 1.12.0 (despite the hang in the test); going back to 1.13.0 already shows the error, but the timeout is much quicker (only a few seconds).

@sumneko
Copy link
Collaborator

sumneko commented Feb 8, 2021

test

I cannot reproduce this in my computer(Windows 10).
If it is caused by 723f3d2, it may be fixed when I done this: #329 (comment).

@clason
Copy link

clason commented Feb 8, 2021

No, I assume this is something specific to macOS (and possibly neovim); otherwise I'd have expected someone to have already reported this days ago ;)

So there are two regressions here:

  1. the "Too large file" error, which didn't happen before 1.13;
  2. the time it takes for the error, which was much shorter before 1.14.

The second issue may indeed be caused by that commit, but 1. seems to have a different cause. I'll try to bisect it in the next hours.

@sumneko
Copy link
Collaborator

sumneko commented Feb 8, 2021

  1. the "Too large file" error, which didn't happen before 1.13;

This is not an error, but some large files were skipped for performance. You can control this size in the settings, the default is 100k. Perhaps in a certain version, your project has files that just exceed the threshold.

@clason
Copy link

clason commented Feb 8, 2021

Sorry, "warning" would have been better. Nevertheless, this did not occur previously, and I got instant diagnostics/completions. (Not complaining, mind you -- there's probably a good reason for it! Just trying to understand what is happening and how to get back the old behaviour.)

@sumneko
Copy link
Collaborator

sumneko commented Feb 8, 2021

Sorry, "warning" would have been better. Nevertheless, this did not occur previously, and I got instant diagnostics/completions. (Not complaining, mind you -- there's probably a good reason for it! Just trying to understand what is happening and how to get back the old behaviour.)

Oh, relax, I didn't feel any discomfort. My wording and tone may be a bit strange, it is all the fault of Google Translate. I'm just worried that you might have misunderstood this prompt, so I explained it a little bit.
By the way, I tried to open your project with version 1.12 on my computer, and I can see these tips.

@clason
Copy link

clason commented Feb 8, 2021

No problem, I just wanted to make sure -- as you write, it's always better to explain!

So I bisected and, not surprisingly, got 43a29d8 as the first bad commit.

I started from 1.12.2, which does give a tip, but it's a different one:

LSP[sumneko_lua][Info] Preloaded files has reached the upper limit (1000), you need to manually open the files
 that need to be loaded.

I also get completions before this hint appears. (The message changes over the next few commits, offering actions, as you know.)

@sumneko
Copy link
Collaborator

sumneko commented Feb 8, 2021

It seems that your project needs to get more lua scripts through other means. In my test, there were only more than 400 lua files in the work area, so it seemed that the initialization was completed quickly.

Due to the need to calculate global variables and require references, the correct approach is indeed to wait for the initialization to complete before responding to operations such as automatic completion. However, considering the following two reasons, in the previous version, I allowed to respond before the initialization was completed:

  1. The cache will be refreshed every time a file is read or a file changes, so as your input, the auto-completed result will be more and more correct.
  2. If there is no response from the automatic completion, the user will be confused.

But then the situation changed, so I changed to wait for the initialization to complete:

  1. During continuous input, auto-complete will directly use the last result without recalculation, so the result is always incorrect as you input.
  2. The progress protocol is supported, and the user can learn the current initialization status from the progress bar, and understand that it is reasonable that the automatic completion is not responding.

@clason
Copy link

clason commented Feb 8, 2021

Thank you for the explanation, that makes sense. So the wait is needed to guarantee correct results, and the only issue is that -- at least on macOS -- increasing the sleep value by 10x also increases the time it takes (from ~3s on 1.13 to ~30s in 1.14).

One thing that is still a bit confusing is that I also have to wait 30s for diagnostics if I open a new file test.lua in an empty directory (and no workspace defined in the settings). I don't get a timeout warning in this case, though.

@sumneko
Copy link
Collaborator

sumneko commented Feb 8, 2021

Please send me the log and see what I am doing in these 30 seconds.
See https://github.com/sumneko/lua-language-server/wiki/Default-log-path

In addition, please help test it, manually change 0.01 second back to 0.001 second result

@clason
Copy link

clason commented Feb 8, 2021

Sure! Here's the logs from an empty test file for an empty directory (it seems it's still indexing the lua-language-server directory, which explains why it's still taking time):
log.zip

@clason
Copy link

clason commented Feb 8, 2021

And, yes, decreasing the thread.sleep time down to 0.001 gives diagnostics after 3s instead of 30s. (But idle CPU load increases again from ~0.5% to ~3.3% -- still better than originally, I think? I notice that the thread.sleep in brave.lua is now gone completely.)

@sumneko
Copy link
Collaborator

sumneko commented Feb 8, 2021

Sure! Here's the logs from an empty test file for an empty directory (it seems it's still indexing the lua-language-server directory, which explains why it's still taking time):
log.zip

This log shows that initialization was completed in 0.4 seconds and only 12 meta files were loaded.

And, yes, decreasing the thread.sleep time down to 0.001 gives diagnostics after 3s instead of 30s. (But idle CPU load increases again from ~0.5% to ~3.3% -- still better than originally, I think? I notice that the thread.sleep in brave.lua is now gone completely.)

But it doesn't seem to matter anymore. The problem is that the behavior of sleep on the macOS platform is more complicated than I thought.

This problem is indeed caused by 723f3d2 (increasing the initialization time from 3 seconds to 30 seconds), and 43a29d8 exposed the problem (need to wait for the initialization to complete before responding).

I will temporarily change sleep back to 0.001 seconds, and then use #329 (comment) to completely solve the problem.

@sumneko sumneko added the bug Something isn't working label Feb 8, 2021
sumneko added a commit that referenced this issue Feb 9, 2021
@clason
Copy link

clason commented Feb 10, 2021

I'm really sorry to say this, but since 1.15 I no longer get any diagnostics or completion (both on macOS and Linux). (CPU time is at 0% now, though...)

Log files:
log.zip

@sumneko
Copy link
Collaborator

sumneko commented Feb 10, 2021

Oh, I'm in holiday in next 7 days, have no machine to debug this.
You could only rollback for now.

@clason
Copy link

clason commented Feb 10, 2021

No worries, happy Lunar New Year! Version 1.14.3 works well (not too long delay, and not too much CPU use).

@sumneko
Copy link
Collaborator

sumneko commented Feb 18, 2021

osclock
My preliminary inspection found that this was caused by the inconsistent behavior of the os.clock function on the Windows platform and the Linux platform. I am still conducting further research.

EDIT: I found this: https://stackoverflow.com/questions/27178789/c-different-implementation-of-clock-in-windows-and-other-os. I will use other methods to achieve related functions.

sumneko added a commit that referenced this issue Feb 18, 2021
@sumneko
Copy link
Collaborator

sumneko commented Feb 18, 2021

@clason I have fixed it, please check out and test it, thank you!

@mjlbach
Copy link
Contributor Author

mjlbach commented Feb 18, 2021

Fixes the problem for me!

@mjlbach mjlbach closed this as completed Feb 18, 2021
@clason
Copy link

clason commented Feb 18, 2021

Yes, works really smoothly now. Thank you!

@charlie39
Copy link

What about Linux and macos?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants