LSP server extremely slow to respond since v1.13.0 (20+ times slower than Pyright)

lemontheme commented 4 days ago

I use basedpyright's LSP server with Helix. It's a wonderful, low hassle combination that I've already converted one or two colleagues to. Thanks to basedpyright, I feel like I can finally get the best of Pyright without the feeling of missing out (on arbitrarily VS Code-exclusive features). So thanks! :)

Unfortunately, basedpyright's LSP has gotten slower. It's particularly noticeable in large modules. There are a few such files that I work on on a frequent basis. They range in the 1000–2000 lines of code. They are only partially typed, and they import from utterly horribly typed first-party and third-party dependencies. In short, these modules are a nightmare for type checkers. (I won't be able to share them to repro though, as the code is 100% proprietary.)

A completion request sent in the context of these files takes between 15 and 50 seconds to complete. Other requests take similarly long until the server has been running for a few minutes. For example, 'go to definition' requests speed up after a while. But completion requests remain slow to the point where they make basedpyright unusable. (I think we can both agree that completions need to resolve fairly quickly, seeing as the idea is to be faster than typing everything out.)

Some investigating later, I've arrived at the conclusion in the title: v1.12.6 was the last version that didn't suffer from these slow response times. Something changed in v1.13.0 and up.

What I did to arrive at this conclusion:

I played around with the server settings. Nothing made an appreciable difference, not even useLibraryCodeForTypes = false. The Helix logs confirm that the settings were correctly acknowledged by the LSP server.
I compared against Pyright using the same settings. Pyright was still as fast as I remembered basedpyright being: Completions resolved in under a second.
I compared against basedpyright v1.0.0 and noticed the same superior speeds. From there I bisected until I found the cutoff: v.12.6.

So, while I'm sure the files I'm working in aren't great from a type checking standpoint, it appears that they didn't always pose an issue for basedpyright.

Let me know if there's anything I can do to help you diagnose this regression. For now I'll be downgrading to v1.12.6, saying goodbye to those sweet, sweet docstrings for builtins. :'(

DetachHead commented 4 days ago

that's odd, i don't think there were any changes in 1.13 that would effect performance (and i personally haven't noticed any slowdown). the only thing that comes to mind is that its loading more docstrings than it used to in some cases.

if you update to 1.13.0 and use the typeshed version from 1.12.6 (by setting the typeshedPath option), does that make any difference?

lemontheme commented 3 days ago

The typeshed information under dist/typeshed-fallback/, right?

Just tested both ways of providing the path to the LSP server: a) as an argument of the LSP CLI command and b) as a config parameter.

(a)

[language-server.basedpyright]
command = "basedpyright-langserver"
args = ["--stdio", "--verbose", "--typeshedpath", "/Users/adriaan/.local/pipx/venvs/basedpyright/lib/python3.12/site-packages/basedpyright/dist/typeshed-fallback/"]

(b)

[language-server.basedpyright.config]

basedpyright.analysis.diagnosticMode = "openFilesOnly"
basedpyright.analysis.typeCheckingMode = "standard"
basedpyright.analysis.autoSearchPaths = true
basedpyright.analysis.useLibraryCodeForTypes = true
basedpyright.analysis.typeshedPaths = ["/Users/adriaan/.local/pipx/venvs/basedpyright/lib/python3.12/site-packages/basedpyright/dist/typeshed-fallback/"]  # v1.12.6 installed here with pipx

I'm afraid that in neither case did I notice a speed up.

lemontheme commented 3 days ago

In case it helps, here are the --stats from running the basedpyright CLI (not basedpyright-langserver):

v1.2.16:

Analysis stats
Total files parsed and bound: 708
Total files checked: 14

Timing stats
Find Source Files:    0sec
Read Source Files:    0.02sec
Tokenize:             0.13sec
Parse:                0.21sec
Resolve Imports:      0.14sec
Bind:                 0.26sec
Check:                0.63sec
Detect Cycles:        0sec

v1.13.0


Analysis stats
Total files parsed and bound: 741
Total files checked: 14

Timing stats
Find Source Files:    0sec
Read Source Files:    0.03sec
Tokenize:             0.16sec
Parse:                0.23sec
Resolve Imports:      0.16sec
Bind:                 0.31sec
Check:                0.93sec
Detect Cycles:        0sec

v1.13.1

Completed in 20.726sec

Analysis stats
Total files parsed and bound: 741
Total files checked: 14

Timing stats
Find Source Files:    0sec
Read Source Files:    0.03sec
Tokenize:             0.16sec
Parse:                0.22sec
Resolve Imports:      0.12sec
Bind:                 0.29sec
Check:                19.7sec
Detect Cycles:        0sec

Main culprit appears to be the 'check' step.

Also interesting to note that from v13.0.0 onward more files are parsed and bound.

lemontheme commented 3 days ago

It seems I was mistaken. As the stats above show, the major slowdown didn't hit until v1.13.1 – although I remember noticing some sluggishness in v1.13.0.

KotlinIsland commented 3 days ago

I wonder if this could be reportUnsafeMultipleInheritance, @DetachHead maybe as a first step we could prepare a build with that change reverted and see if it affects the performance in this scenario.

lemontheme commented 3 days ago

Just in case you missed it, @KotlinIsland, I'm running with typeCheckingMode = "standard".

According to the table comparing the default diagnostic rules, the reportUnsafeMultipleInheritance diagnostic is inactive in the 'standard' mode – or at least, that's how I interpreted it.

Your comment got me thinking though. I just did another experiment, this time with typeCheckingMode = "off". Result: no difference.

DetachHead commented 3 days ago

its either that or the reportUnreachable change i made in 285d807dbebd4df9a525b245853bb63c87a57472. i'll make some changes to both checks which will hopefully address the issue.

thanks for all the effort you put in to help us narrow this down!

Diaoul commented 2 days ago

I'm having performance issues as well but it seems for a longer time. The culprit for me is 1.2.0 (45 sec warm up) vs 1.1.0 (1-2 sec warm up). The latter is similar to what I have with pyright.

This is especially annoying when wanting to move around in the code and jump to definition.

~~@DetachHead what is the best way to investigate such issues? How can I turn on/off some additional features that would come with a hefty performance cost?~~ RTFM

For information I'm on a very large codebase so performance issues are exacerbated.

Diaoul commented 2 days ago

reportImportCycles was the culprit for me, I just disabled that and I'm back on reasonable performance, sorry about the noise.

DetachHead commented 2 days ago

@Diaoul no need to apologize. my guess is that your issue was caused by an upstream change since we haven't touched any code related to import cycles

@lemontheme can you see if #464 fixes it?

pip install git+https://github.com/DetachHead/basedpyright@24ce32a6be3a0a1d0e971813e0d90b48696a794b

if that works, and if you have the time, if you want to check against each commit and let me know which one fixed it that would be appreciated:

pip install git+https://github.com/DetachHead/basedpyright@07edddf3449d6b165fbdae35c09ae155ca0393d4

pip install git+https://github.com/DetachHead/basedpyright@e2545b12c056e8ee5feec07b519a1997204a5a96

thanks!

lemontheme commented 2 days ago

@DetachHead, thanks for taking the time to look at this!

I'm having some trouble pip installing from the first commit. When I launch the language server, I'm presented with this error message:

node:internal/modules/cjs/loader:1148
  throw err;
  ^

Error: Cannot find module '/Users/adriaan/[REDACTED]/.venv/lib/python3.10/site-packages/basedpyright/langserver.index.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1145:15)
    at Module._load (node:internal/modules/cjs/loader:986:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:174:12)
    at node:internal/main/run_main_module:28:49 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v20.15.0

Seems like I'm missing a build step somewhere. Problem is, I don't know enough about Node to know where to start digging.

By contrast, when I pip install from the latest release tag, the language server starts up without error.

Let me know what I'm missing and I'll try out each of those commits :)

DetachHead commented 2 days ago

that can happen if the language server is locked by your editor while pip is trying to uninstall it and replace it with the new one. if you close your editor, uninstall basedpyright, delete any folders starting with special characters from your venv (eg. site-packages/~asedpyright. these are corrupted partially installed packages i think) then try again it should work. if you still have issues you may need to just delete and re-create the venv

lemontheme commented 2 days ago

Still nothing, I'm afraid.

What strikes me is that when installing from the release tag, pip really takes its take on the wheel building step, i.e. onBuilding wheel for basedpyright (pyproject.toml) ... [spinner]. However, when installing from your commits, that step is practically instant. It's the same in a clean venv.

DetachHead commented 2 days ago

jeez, turns out i completely broke the build in 1baf3ec700c98d9baa6d351eb791fd692e2b2d05 and never noticed. no idea how that went undetected by the ci. it seems to behave differently depending on whether you're builing the wheel or building+installing it... man i despise the python build system

can you try installing fc161d92a97183840ba691e3213985f0c7d4886b and see if that works? (if it does and you want to also try those other commits from my comments above, their hashes have changed to 71b11897478cdf25fb8fb8b03815903749b07678 and 13aef28093cbb5513cbb1e30e87d294640de9bf2)

lemontheme commented 1 day ago

Whoops! Haha, yeah, it never quite works the way you expect it to. Anyway, all three commits install properly now. :)

Below are the --stats of each. I took special care to run pip install with the --no-cache-dir and --force-reinstall flags. For each commit, I installed in 3 different ways: a) in my main venv, b) in a clean venv, c) with pipx. The --stats results were always the same.

The conclusion, I'm sorry to report, is that the issue persists.

Results:

fc161d9:

Completed in 20.531sec

Analysis stats
Total files parsed and bound: 741
Total files checked: 14

Timing stats
Find Source Files:    0sec
Read Source Files:    0.06sec
Tokenize:             0.16sec
Parse:                0.22sec
Resolve Imports:      0.13sec
Bind:                 0.29sec
Check:                19.47sec
Detect Cycles:        0sec

71b1189:

Completed in 20.239sec

Analysis stats
Total files parsed and bound: 741
Total files checked: 14

Timing stats
Find Source Files:    0sec
Read Source Files:    0.06sec
Tokenize:             0.16sec
Parse:                0.21sec
Resolve Imports:      0.12sec
Bind:                 0.28sec
Check:                19.21sec
Detect Cycles:        0sec

13aef28:

Completed in 20.442sec

Analysis stats
Total files parsed and bound: 741
Total files checked: 14

Timing stats
Find Source Files:    0sec
Read Source Files:    0.06sec
Tokenize:             0.16sec
Parse:                0.23sec
Resolve Imports:      0.12sec
Bind:                 0.28sec
Check:                19.41sec
Detect Cycles:        0sec

DetachHead commented 1 day ago

damn. do you mind also checking against https://github.com/DetachHead/basedpyright/pull/466? there were some upstream performance fixes i think, maybe that could fix it

is the repo you're testing against public?

lemontheme commented 1 day ago

Sure thing. Below are the --stats results again when installing from db6568c:

Good news and bad news.

Good news: the LSP is fast again! – which resolves this issue. Bad news: the basedpyright CLI is still 30x slower than in v1.12.6.

Completed in 20.208sec

Analysis stats
Total files parsed and bound: 741
Total files checked: 14

Timing stats
Find Source Files:    0sec
Read Source Files:    0.07sec
Tokenize:             0.16sec
Parse:                0.25sec
Resolve Imports:      0.16sec
Bind:                 0.3sec
Check:                19.06sec
Detect Cycles:        0sec

The 3 commits compared in my previous comment didn't exhibit the same discrepancy between LSP and CLI speed.

The repo I'm testing on is proprietary code that belongs to my company.

DetachHead commented 1 day ago

thats unfortunate. i think some upstream changes to the primer may have uncovered the same performance issue(s) as well, so i'll try and use that to narrow down exactly when this started happening, assuming it's the same issue.

DetachHead / basedpyright

LSP server extremely slow to respond since v1.13.0 (20+ times slower than Pyright) #461