-
Notifications
You must be signed in to change notification settings - Fork 34
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
Linting on change/save (with autosave) can be slow #454
Comments
@liren-jin Can you share any settings you are using? and the logs with lines where it runs the linter. |
Can you confirm the version of linter you were using before vs now? the extension ships with |
pylint 3.0.2 2023-11-10 18:43:56.594 [info] SUPPORTED pylint>=2.12.2 |
@liren-jin share the logs (Output > pylint) it should have time information on how long it took to run pylint. Also, verify that running pylint from the terminal is faster. |
Hi, detecting errors is super fast, around 2-3 ms. However, after correction, the error message is still there for ~5 s: 2023-11-10 18:51:03.198 [info] [Trace - 6:51:03 PM] Received notification 'textDocument/publishDiagnostics'. |
Can you include previous part of the logs where it includes |
I'm getting hit by this HARD, its actively making me less productive in business measurable ways over a refactor because I have to wait for it to catch up with me and it takes long enough I can get a cup of coffee, come back, and its still not done. In testing, I now have a theory that its possible visual studio code's autosave feature may be involved as when I turned it off, the issue went away and it got out of the loop; I'm assuming that the pylint plugin is conflicting with this somehow if vsc is not respecting the 1second delay due to eventing on file edits or saves. Is that possible? Thing is, if thats the issue, that's not a valid work around either, I should not have to turn off a core visual studio feature - autosave from the file menu - to get my productivity back. But I can confirm that the slowness in my case does seem to be pylint checking the entire file on every keyboard keypress in the IDE, eventually lagging out hard if you type fast enough and create enough of these events, based on the errors I was getting that made it look like it was thinking half typed words I was banging out as fast as I could were to be considered code that should be run through pylint. I don't feel it should be running code I am not done typing out through pylint, ad that in itself is a big defect. |
@duaneking What is your An update on this issue. From the logs provided by users, it seems like processing code actions was taking too long. A potential fix is to resolve the possible auto-fixes lazily. PR #457 |
I'm using default settings, from my perspective I just click the burger bar -> files -> "auto save" menu item and its always checked. I think in VSC is a default delay of 1 second. I think what happened was my turning it off killed it, and then the new refresh caught up, or at least, that seems reasonable. |
Just wanted to update and log that this plugin was struck on files that have not existed for hours, based on the text I could copy out of the popups that were fleshing on my screen endlessly so suddenly. I have git logs that show the files listed in the exception popups have not existed for over an hour. but it was looping hard and giving me a massive amount of popups claiming asteroid errors saying it was directed to a file that does not exist, endlessly, until I started clicking around because it was hurting my productivity and freaking out the other person on the screen share as I was i a meeting at the time. I suspect this shows that files are being cached far longer than they need to be, and that makes me wonder. Clearly, its not handling file renames or deletes well. |
@duaneking You can disable notifications for |
I want the notifications on when something happens; they are useful when they are correct, so I would consider this error to be a defect because it shouldn't be trying to access files that don't exist anyway, and if the file doesn't exist because it was renamed an hour ago, as is this situation, I honestly consider the fact that is looking for a file an hour after it's renamed to be a bigger issue. |
@duaneking I just published a pre-release with a potential fix for the performance problem. Build version 2023.11.13191929 can be installed using "Install Another Version" option in the extensions view. Please try that version, and to debug further I would need logs from Output > Pylint. To see why pylint is being triggered on files that don't exist. |
I just checked on this, and installed the latest beta version. Almost immediately it started having issues; But that log output includes code I cant give you (its not open source), but the big error does seem to be in its use of pylint-django itself; A RecursionError output
Clearly the pylint plugin has some bugfixes to do; And maybe it should consider the django extension as less trustworthy? |
Hope the above helps. |
Here is the smoking gun that proves its linting on every keypress: ` 2023-11-15 12:36:22.771 [info] [Trace - 12:36:22 PM] Received notification 'textDocument/publishDiagnostics'. ` This is just WRONG in so many ways. |
It should NOT be trying to lint the file and pull in the entire projects code base on every keypress, that is just broken, and as you can see from the timestamps, its lagging hard. |
@duaneking I need the initial part of the log where it details how the server was started. Also is the with pre-release version I mentioned before. Also, we don't try to pull in anything, all we do is run pylint on the file that is opened. The reason I don't understand this output is because I don't see Try adding this to the your user settings the error with |
Most of the work of collecting the logs requires that I censor the output; this code is not open source and so I'm limited on what I can give you. lsp errors:
I get lots of of didChnge stuff but it like this:
|
@duaneking Can you provide initial part of the logs where it prints out the settings passed to the server. Also, You have not mentioned if you are using pre-release or stable. An improvement was pushed to |
As I said before, my settings are default with the usual .pylint file: Initial plugin restart output: Output023-11-15 13:35:12.412 [info] No interpreter found from setting pylint.interpreter 2023-11-15 13:35:12.426 [info] [Trace - 1:35:12 PM] Sending notification 'exit'. 2023-11-15 13:35:12.427 [info] No interpreter found from setting pylint.interpreter 2023-11-15 13:35:13.645 [info] [Trace - 1:35:13 PM] Received notification 'window/logMessage'. 2023-11-15 13:35:13.645 [info] CWD Server: c:\Users\Duane\Workspace\com 2023-11-15 13:35:13.646 [info] Settings used to run Server: 2023-11-15 13:35:13.646 [info] [Trace - 1:35:13 PM] Received notification 'window/logMessage'. 2023-11-15 13:35:13.647 [info] Global settings: 2023-11-15 13:35:13.647 [info] [Trace - 1:35:13 PM] Received notification 'window/logMessage'. 2023-11-15 13:35:13.647 [info] sys.path used to run Server: 2023-11-15 13:35:13.655 [info] c:\Users\Duane\Workspace\com.venv\Scripts\python.exe -m pylint --version 2023-11-15 13:35:13.655 [info] CWD Linter: c:\Users\Duane\Workspace\com 2023-11-15 13:35:13.988 [info] 2023-11-15 13:35:13.988 [info] [Trace - 1:35:13 PM] Received notification 'window/logMessage'. 2023-11-15 13:35:13.988 [info] Version info for linter running for C:\Users\Duane\Workspace\com: 2023-11-15 13:35:13.989 [info] [Trace - 1:35:13 PM] Received notification 'window/logMessage'. 2023-11-15 13:35:13.989 [info] SUPPORTED pylint>=2.12.2 2023-11-15 13:35:13.993 [info] [Trace - 1:35:13 PM] Received response 'initialize - (0)' in 1543ms. 2023-11-15 13:35:13.994 [info] [Trace - 1:35:13 PM] Sending notification 'initialized'. 2023-11-15 13:35:13.997 [info] [Trace - 1:35:13 PM] Sending notification 'textDocument/didOpen'. 2023-11-15 13:35:13.997 [info] [Trace - 1:35:13 PM] Sending notification 'textDocument/didOpen'. 2023-11-15 13:35:14.004 [info] [Trace - 1:35:14 PM] Received notification 'window/logMessage'. 2023-11-15 13:35:18.632 [info] [Trace - 1:35:18 PM] Received notification 'window/logMessage'. 2023-11-15 13:35:18.634 [info] [Trace - 1:35:18 PM] Received notification 'textDocument/publishDiagnostics'. 2023-11-15 13:35:20.674 [info] [Trace - 1:35:20 PM] Received notification 'textDocument/publishDiagnostics'. `` The .pylintrc file: ` [MESSAGES CONTROL] [pylint.MESSAGES_CONTROL] |
My visual studio code settings file checked in with the project and used successfully on multiple machines
|
I hope all that helps. |
Any update on this? It's taking me a long time to see errors on change as well. |
@duaneking You can turn off linting on change. The autosave will trigger linting on save, so it can use the save timeout. Also note that you can use Ruff extension which implements We don't want to introduce debounce logic, we plan on moving to a newly added diagnostic request type that could work better in these scenarios. If someone wants to try with debounce logic the code is in python and we are open to take PRs. vscode-pylint/bundled/tool/lsp_server.py Lines 100 to 131 in f869c86
The debounce logic can go in |
If you want This is the extension: https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff The python server that is used is the same for both pylint and ruff. The only difference is the framework the linters are written on top of. We prioritize issues based on usage, number reports, impact. As mentioned in the previous comment, we are open to take the debounce logic PR if we are not able to get to this in time. |
I'm constantly getting pauses and lag due to what I can see is warnings for code that no longer exists, long after it has been typed, and typing faster makes the problem worse. Per the above, if I type the text:
I get warnings for:
Until finally:
.. is typed and I'm stuck waiting for pylint to catch tf up. Thing is, I use this same desktop system for stable diffusion and AI model inference just fine. It handles modern games just fine. Even the rust compiler, as slow as it is to compile bevy, works just fine. Pylint is the slowest thing to run on my system, and at this point I have lost so much productivity from the forcibly broken "flow state" this per-keystroke lag creates, that I am forced to assume pylint is critically broken and harmful to use. The fact is, it should NOT be being triggered on every keypress like this. It should NOT be triggered even on every word. It should not be triggered AT ALL as I type this, until I am done typing and autosave kicks in after a second of inactivity the moment I am done typing. Pylint is broken. |
No, I cant. Not in a central way that respects my .pylint file. There is no such option that I can find. And any attempt to use the old style "python.linting.*" stuff that is now legacy and explicitly no longer supported results in warnings that I need to remove it and upgrade to the plugin I'm already using, I'm even given an aka.ms link in the terminal as it tries to scare me into upgradeing to this plugin. VSC doesn't let you try to use settings like I have looked everywhere, there is no option to turn off the per-keystroke run in a way that works for my flow. I have googled, I have asked chatgpt, I have begged friends, the only response back I get back is random pylint warning about code that has not existed for a long while.
.. and that adds to the queue so its always the last to run. It should be kicking out all the other events and explicitly cutting the queue, bouncing the old events out as they are worthless on save, as these events are no longer valid. These events are simply queued wrong, they should be using a time based priority queue and throwing out old events, not blindly adding new requests to the queue. |
Out of good faith, I clarified above, and to further clarify: The .pylint files are not respected here. The settings.json does not get modified by this. There does not seem to be a way for me to have my settings to do this be serialized, and I expected VSC to update the settings.json file but it does not. Maybe that in itself is a defect? |
@duaneking I don't understand. What do you mean it is not serialized? You can see here that this is settings.json with Also, I am not sure where you got |
With respect, you just changed the title of the bug to the wrong thing. The settings file that I check in to git is the only one that I care about. Anything that requires I modify the machine is a non-starter. If this can't be represented in a Settings.JSON file or as part of a .pylint file, then it doesn't respect our continuous delivery or dev stuff. What's the point of centralizing and standardizing our setting as a team for the code base if we're not able to all use the same configuration? |
And for the record, the settings that you showed do not show up on my screen at all. When I make these changes in the UI, it does not edit the settings.json |
This comment was marked as off-topic.
This comment was marked as off-topic.
I'm sorry the performance has been so crippling for you, @duaneking. I just wanted to share that I do see the I hope you're able to get things configured to your liking! I do believe that the necessary options are available to achieve a better result for you. |
Honestly, because linting on change is not a heavily used feature due to Pylint often not being able to keep up (which is not a criticism of Pylint; it does a lot!). Due to bug reports mis-attributing the perf issues to us we are actually considering dropping the feature and asking people to rely lint-on-save w/ auto-save. If you would like to propose a PR to implement debounce, then please do so and we can have a look. |
I want to stress that what Brett just said have been 👍 by two pylint maintainers. There's checks that do not scale: duplicate-code in particular must compare each lines of each file in the entire repo. It will never be fast enough for a keystroke lint. Even if you deactivate that kind of slow checks, pylint is not supposed to be fired on each keystrokes, the underlying ast that does not trust the typing and try to infer value by looking at calls is not fast enough and probably never will. Also, with ruff existing, it wouldn't make sense to focus all our very limited maintainer time on perfs for the check that do not use inference. We would still be 50 times slower than ruff for those in the end and those are easily reimplemented. I think greying the option to be able to lint on change would be helpful in order to prevent frustration and to make the fact that pylint can't do that explicit to vscode-pylint users. |
My requirement is that I be able to update only the .vscode/settings.json file and have the settings be respected; This is a strict requirement so that it can be checked in as part of the project and so that multiple machines can use the correct settings by default; that is not the case here. Attempting to update the settings manually to a non-standard non-default install as you have requested, does not update that file. I understand that you want me to do a custom install of Visual Studio code that does not use the default settings; What I'm trying to explain, and what I've been trying to explain before my comments were marked as off topics so rudely, Is that I'm prioritizing accessibility and attempting to have a standard configuration based on the project with our configuration files checked in so that our visually disabled developers who work on the project can participate and have equal access. |
Then it might be possible this extension just isn't able to meet your specific needs.
I'm sorry you feel that way, but it was a single comment and it didn't add to the discussion while making us feel dismissed while doing the best we could to help you. You are welcome to disagree w/ our assessment, but we have prioritized this issue in a way we feel is reasonable.
We believe some settings are not appropriate to set for all users, such as lint on save. That's not a good experience if someone set that on some open source project w/ no way to turn it off since it would require doing so by editing |
Ran into the same issue but turning of "lint on change" as suggested resolved it. Thanks! |
…tiple processes Refs microsoft/vscode-pylint#454 Closes pylint-dev#9341
same here. Looks like this pylint extension also makes my vscode terminal very slow (need to wait for >5sec whatever cmd I use). |
Toooooooooooooo slow especially for large project. |
…tiple processes Refs microsoft/vscode-pylint#454 Closes pylint-dev#9341
…tiple processes Refs microsoft/vscode-pylint#454 Closes pylint-dev#9341
…tiple processes Refs microsoft/vscode-pylint#454 Also add known caveats for custom parallization. Closes pylint-dev#9341 Co-authored-by: Jacob Walls <[email protected]>
…tiple processes Refs microsoft/vscode-pylint#454 Also add known caveats for custom parallization. Closes #9341 Co-authored-by: Jacob Walls <[email protected]>
still relevant (under ubuntu 24.04): LSP server takes a long time to send a response 2024-12-10 23:26:52.005 [info] CWD Linter: /home/ilyat/Projects/hamvestors/hamvestors-service on v2023.8.0 is no such problem |
Hi,
Recently I switched from python.pylint to this pylint extension, however, I found this extension is pretty slow. e.g., after correction, I have to wait for ~4-5 seconds until the warning disappears.
The text was updated successfully, but these errors were encountered: