astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
33.01k stars 1.1k forks source link

Ruff server pegs CPU and blocks VSCode when changing many files #13686

Open adamh-oai opened 1 month ago

adamh-oai commented 1 month ago

In our large mostly-python monorepo, when using the VSCode ruff extension + ruff server, if I update to a commit has many changed files, ruff server will consume lots of CPU (up to 1000% in some cases), and I cannot save in VSCode because it is blocked on "Getting code actions (..., "Ruff")".

image

Sample of debug logs:

2024-10-08 14:39:50.935 [info] Falling back to bundled executable: /Users/adamh/.vscode/extensions/charliermarsh.ruff-2024.50.0-darwin-arm64/bundled/libs/bin/ruff
2024-10-08 14:39:50.955 [info] Resolved 'ruff.nativeServer: auto' to use the native server
2024-10-08 14:39:50.955 [info] Found Ruff 0.6.6 at /Users/adamh/.vscode/extensions/charliermarsh.ruff-2024.50.0-darwin-arm64/bundled/libs/bin/ruff
...
2024-10-08 17:00:20.165 [info] [Trace - 5:00:20 PM]    0.427054875s DEBUG ThreadId(06) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: ...
2024-10-08 17:00:20.241 [info] [Trace - 5:00:20 PM] Sending notification '$/cancelRequest'.
2024-10-08 17:00:20.241 [info] [Trace - 5:00:20 PM] Sending request 'textDocument/codeAction - (6)'.
...

024-10-08 17:10:03.895 [info] [Trace - 5:10:03 PM]  584.158088958s DEBUG ThreadId(57) ruff_server::session::index::ruff_settings: Ignored path via `exclude`:
...

There are lots and lots of the "ignored", which isn't surprising since I run with a config that excludes most paths in the repo.

adamh-oai commented 1 month ago

@hauntsaninja said I should tag @dhruvmanila since you've been talking about this.

dhruvmanila commented 1 month ago

Thanks! I'll need to try to reproduce this somehow using a large repository, maybe I'll pickup home-assistant/core for this.

adamh-oai commented 1 month ago

Looks like ours is about 5x larger than home-assistant/core. I can reproduce this pretty easily, if you want to give me something to test. A few other details:

Both this and the 50-copies-of-rg issue sort of feel like VSCode is triggering a whole bunch of update notifications and they queue up somewhere and then are processed even though they all represent the same change.

MichaReiser commented 1 month ago

@adamh-oai is there a high likelihood that each pull changes some pyproject.toml?

Both this and the 50-copies-of-rg issue sort of feel like VSCode is triggering a whole bunch of update notifications and they queue up somewhere and then are processed even though they all represent the same change.

That's what I suspect as well

dhruvmanila commented 1 month ago

If they're representing the same changes, then there's a chance that VS Code must be sending the cancel requests as well (as also noted in your logs). It might become important to implement that in the server although I'd need to verify.

adamh-oai commented 1 month ago

@dhruvmanila this hits us pretty regularly, is there anything I can test to narrow it down?

MichaReiser commented 1 month ago

@dhruvmanila I wonder if it's time to change our settings discovery to be lazy instead of doing it eagerly. It would not only help with the single file case but also reduce the indexing cost here because the LSP would only ever discover the settings for the files currently open in the editor instead of all settings (and only traverse upwards, but never downwards)

dhruvmanila commented 1 month ago

Yeah, I think we might want to prioritize that. And, even if we add support for cancellation, lazy settings discovery would still be more beneficial comparatively.

dhruvmanila commented 1 month ago

I experimented a bit using home-assistant/core and I think we might be re-indexing the entire project multiple times.

dhruvmanila commented 4 weeks ago

@adamh-oai Can I know how many Ruff configuration files exists in your project? It could be either pyproject.toml / ruff.toml / .ruff.toml.

hauntsaninja commented 4 weeks ago

We have well over a thousand pyproject.toml in the repo, only about 150 of which contain tool.ruff. It's common to have a per-project ruff setup that extend-s from the root ruff config. Looks like there are two ruff.toml

dhruvmanila commented 4 weeks ago

Thanks, that's very useful. I think we've narrowed down the problem and will be working towards a fix for it.

dhruvmanila commented 1 week ago

This is still on our mind. I'm planning to pick it up either next or the following week.