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

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

Closed
lemontheme opened this issue Jun 30, 2024 · 24 comments
Labels
language server pyright parity something we broke that isn't an issue in pyright

Comments

@lemontheme
Copy link

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 DetachHead added language server pyright parity something we broke that isn't an issue in pyright labels Jun 30, 2024
@DetachHead
Copy link
Owner

DetachHead commented Jun 30, 2024

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
Copy link
Author

lemontheme commented Jul 1, 2024

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
Copy link
Author

lemontheme commented Jul 1, 2024

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
Copy link
Author

lemontheme commented Jul 1, 2024

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Owner

its either that or the reportUnreachable change i made in 285d807. 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
Copy link

Diaoul commented Jul 2, 2024

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
Copy link

Diaoul commented Jul 2, 2024

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

@DetachHead
Copy link
Owner

DetachHead commented Jul 2, 2024

@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
Copy link
Author

lemontheme commented Jul 2, 2024

@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
Copy link
Owner

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
Copy link
Author

lemontheme commented Jul 2, 2024

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
Copy link
Owner

DetachHead commented Jul 2, 2024

jeez, turns out i completely broke the build in 1baf3ec 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 fc161d9 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 71b1189 and 13aef28)

@lemontheme
Copy link
Author

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
Copy link
Owner

damn. do you mind also checking against #466? there were some upstream performance fixes i think, maybe that could fix it

is the repo you're testing against public?

@lemontheme
Copy link
Author

lemontheme commented Jul 3, 2024

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
Copy link
Owner

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
Copy link
Owner

DetachHead commented Jul 6, 2024

so i spent the last few days trying to narrow this down and came to the conclusion that the performance issues uncovered by the primer were an unrelated upstream regression that had already been fixed in the latest release. unfortunately that means i'm back to square 1 and have no idea what the issue could be :(

the only thing i can suggest is trying to create a minimal repro of the issue that you are able to share. if that's not possible, perhaps you could try installing each commit from between 1.12.6 and 1.13.1 and narrowing down exactly which commit introduced the issue

@lemontheme
Copy link
Author

lemontheme commented Jul 6, 2024

Edit: I have no idea what shortcut I accidentally pressed. I did not mean to close this issue as complete.


Thanks for sticking with this issue! I fully appreciate how little satisfication it brings to try to diagnose problems while driving blind.

What we need is something to benchmark against – a large open-source code base that has all the complexities of real-life Python code, such as varying degrees of type completeness, strange (potentially dynamic) imports, and dependencies on non-Python code. 'Real-life' for me at least, as someone working in the ML space.

I believe I've found the ideal candidate: https://github.com/huggingface/transformers.

Running pyright and basedpyright with --stats on the src directory gives a very similar time distribution, with check accounting for practically the entire runtime.

(I also tried Django, another fairly large codebase, but the time spent of the various steps is distributed differently: 'Check' and 'Detect Cycles' both take 8 seconds.)

Here is the result of --stats:

pyright 1.1.370:

Completed in 53.481sec

Analysis stats
Total files parsed and bound: 2821
Total files checked: 1690

Timing stats
Find Source Files:    0.03sec
Read Source Files:    0.42sec
Tokenize:             1.03sec
Parse:                1.73sec
Resolve Imports:      0.41sec
Bind:                 1.94sec
Check:                47.38sec
Detect Cycles:        0sec

basedpyright 1.12.6:

Completed in 56.091sec

Analysis stats
Total files parsed and bound: 2823
Total files checked: 1690

Timing stats
Find Source Files:    0.03sec
Read Source Files:    0.33sec
Tokenize:             1.05sec
Parse:                1.74sec
Resolve Imports:      0.45sec
Bind:                 1.97sec
Check:                49.95sec
Detect Cycles:        0sec

basedpyright 1.13.1 and basedpyright 1.13.2:

Literally unable to complete. Or at least, it's been running for about 10 minutes now and it's still stuck on analyzing one of the modules. So... That's interesting! =p


What I'll do now is, as you suggest, run through each of the commits between 1.12.6 and 1.13.1 and test against both the Transformers repo and my private repo.

@DetachHead DetachHead reopened this Jul 6, 2024
@lemontheme
Copy link
Author

I've started collecting a few benchmarks in this Google Sheet.

I'm evaluating on my private code base, Django, HF Transformers, and Scikit-learn.

Key takeaways:

  • v1.13.2 is almost as fast on my private code base as v1.12.6 was, i.e. fast enough. The LSP is fast enough too. (This info may suffice to close this issue) 🎉
  • v1.13.1 and v1.13.2 consistently parse and bind more files than v1.12.6 (I don't know what this means).
  • Despite the obvious improvements in v1.13.2, this version is unusably slow on Transfomers. I gave up after 10 minutes. So if you're looking for a way to improve basedpyright by really putting it through its paces, that's the code base you want to throw at it!

@DetachHead
Copy link
Owner

glad to hear your original issue is resolved. as for the outstanding issue with transformers, i was able to reproduce it on pyright 1.1.370 by turning on all diagnostic rules:

[tool.pyright]
typeCheckingMode = "strict"
deprecateTypingAliases = true
enableExperimentalFeatures = true
reportCallInDefaultInitializer = true
reportImplicitOverride = true
reportImplicitStringConcatenation = true
reportImportCycles = true
reportMissingSuperCall = true
reportPropertyTypeMismatch = true
reportShadowedImports = true
reportUninitializedInstanceVariable = true
reportUnnecessaryTypeIgnoreComment = true
reportUnusedCallResult = true

feel free to raise another issue for that if you want. though i would probably suggest raising it on the upstream repo instead, as i don't really have the experience with the pyright codebase to know where to begin diagnosing and fixing such performance issues.

thanks again for your help and patience

@lemontheme
Copy link
Author

Interesting... So that probably means there are one or more rules that are by default enabled in basedpyright but disabled in pyright. I'll play around with them a bit before opening an issue.

Likewise, thanks for helping me out :)

@DetachHead
Copy link
Owner

all rules are enabled by default in basedpyright (the motivation for that decision is explained here). pyright also doesn't have the "all" option for typeCheckingMode, which is why i had to set it to "strict" yet still had to enable a bunch of rules explicitly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language server pyright parity something we broke that isn't an issue in pyright
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants