language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-01 06:11:43 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Avoid indexing the same workspace multiple times (#15495)

Browse source 
## Summary

This is not lazy indexing but it should somewhat help with #13686.

Currently, processing the change notifications for config files doesn't
account for the fact that multiple config files could belong to the same
workspace. This means that the server will re-index the same workspace
`n` times where `n` is the number of file events which belongs to the
same workspace. This is evident in the following trace logs:

**Trace logs:**

```
[Trace - 6:21:15 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {
    "changes": [
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/pylint/ruff.toml",
            "type": 1
        },
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/script/ruff.toml",
            "type": 2
        },
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/script/scaffold/templates/ruff.toml",
            "type": 2
        },
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/pyproject.toml",
            "type": 2
        }
    ]
}

...

[Trace - 6:21:19 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {
    "changes": [
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/tests/testing_config/custom_components/ruff.toml",
            "type": 1
        },
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/tests/ruff.toml",
            "type": 2
        }
    ]
}

...
```

**Server logs:**

```
 14.838004208s TRACE     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::server::api: enter
  14.838043583s DEBUG     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::session::index::ruff_settings: Indexing settings for workspace: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core
  14.854324541s DEBUG ThreadId(55) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.vscode
  14.854388500s DEBUG ThreadId(55) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.git
  14.937713291s DEBUG     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::session::index::ruff_settings: Indexing settings for workspace: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core
  14.954429833s DEBUG ThreadId(75) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.git
  14.954675708s DEBUG ThreadId(66) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.vscode
  15.041465500s DEBUG     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::session::index::ruff_settings: Indexing settings for workspace: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core
  15.056731541s DEBUG ThreadId(78) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.vscode
  15.056796833s DEBUG ThreadId(78) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.git
  15.117545833s DEBUG     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::session::index::ruff_settings: Indexing settings for workspace: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core
  15.133091666s DEBUG ThreadId(90) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.vscode
  15.133146500s DEBUG ThreadId(90) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.git
  15.220340666s TRACE ruff:worker:6 request{id=5 method="textDocument/diagnostic"}: ruff_server::server::api: enter
  15.220401458s DEBUG ruff:worker:6 request{id=5 method="textDocument/diagnostic"}: ruff_server::resolve: Included path via `include`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/homeassistant/bootstrap.py
  18.577521250s TRACE     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::server::api: enter
  18.577561291s DEBUG     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::session::index::ruff_settings: Indexing settings for workspace: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core
  18.616564583s DEBUG ThreadId(102) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.vscode
  18.616627291s DEBUG ThreadId(102) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.git
  18.687424250s DEBUG     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::session::index::ruff_settings: Indexing settings for workspace: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core
  18.704441416s DEBUG ThreadId(114) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.vscode
  18.704694958s DEBUG ThreadId(121) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.git
  18.769627500s TRACE ruff:worker:4 request{id=6 method="textDocument/diagnostic"}: ruff_server::server::api: enter
  18.769696791s DEBUG ruff:worker:4 request{id=6 method="textDocument/diagnostic"}: ruff_server::resolve: Included path via `include`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/homeassistant/bootstrap.py
```

This PR updates the logic to consider all the change events at once
keeping track of the workspace path that have been already indexed.

I want to include this in tomorrow's release to check how this change
would affect the linked issue.

## Test Plan

Run the same scenario as above and check the logs to see that the server
isn't re-indexing the same workspace multiple times:

**Trace logs:**

```
[Trace - 6:04:07 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {
    "changes": [
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/script/ruff.toml",
            "type": 1
        },
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/script/scaffold/templates/ruff.toml",
            "type": 1
        },
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/pylint/ruff.toml",
            "type": 2
        },
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/pyproject.toml",
            "type": 2
        }
    ]
}

...

[Trace - 6:04:11 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {
    "changes": [
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/tests/testing_config/custom_components/ruff.toml",
            "type": 1
        },
        {
            "uri": "file:///Users/dhruv/work/astral/parser-checkouts/home-assistant-core/tests/ruff.toml",
            "type": 2
        }
    ]
}

...
```

**Server logs:**

```
  17.047706750s TRACE     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::server::api: enter
  17.047747875s DEBUG     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::session::index::ruff_settings: Indexing settings for workspace: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core
  17.080006083s DEBUG ThreadId(54) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.vscode
  17.080085708s DEBUG ThreadId(54) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.git
  17.145328791s TRACE ruff:worker:6 request{id=5 method="textDocument/diagnostic"}: ruff_server::server::api: enter
  17.145386166s DEBUG ruff:worker:6 request{id=5 method="textDocument/diagnostic"}: ruff_server::resolve: Included path via `include`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/homeassistant/bootstrap.py
  20.756845958s TRACE     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::server::api: enter
  20.756923375s DEBUG     ruff:main notification{method="workspace/didChangeWatchedFiles"}: ruff_server::session::index::ruff_settings: Indexing settings for workspace: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core
  20.781733916s DEBUG ThreadId(66) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.vscode
  20.781825875s DEBUG ThreadId(75) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/.git
  20.848340750s TRACE ruff:worker:7 request{id=6 method="textDocument/diagnostic"}: ruff_server::server::api: enter
  20.848408041s DEBUG ruff:worker:7 request{id=6 method="textDocument/diagnostic"}: ruff_server::resolve: Included path via `include`: /Users/dhruv/work/astral/parser-checkouts/home-assistant-core/homeassistant/bootstrap.py
```
This commit is contained in:
Dhruv Manilawala 2025-01-15 18:58:28 +05:30 • committed by GitHub
parent 73488e71f8
commit b5dbb2a1d7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 48 additions and 31 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs
View file

@ -20,9 +20,7 @@ impl super::SyncNotificationHandler for DidChangeWatchedFiles {
requester: &mut Requester, requester: &mut Requester,
params: types::DidChangeWatchedFilesParams, params: types::DidChangeWatchedFilesParams,
) -> Result<()> { ) -> Result<()> {
for change in &params.changes { session.reload_settings(&params.changes);
session.reload_settings(&change.uri);
}
if !params.changes.is_empty() { if !params.changes.is_empty() {
if session.resolved_client_capabilities().workspace_refresh { if session.resolved_client_capabilities().workspace_refresh {

8
crates/ruff_server/src/session.rs
View file

@ -2,7 +2,7 @@
use std::sync::Arc; use std::sync::Arc;
use lsp_types::{ClientCapabilities, NotebookDocumentCellChange, Url}; use lsp_types::{ClientCapabilities, FileEvent, NotebookDocumentCellChange, Url};
use crate::edit::{DocumentKey, DocumentVersion, NotebookDocument}; use crate::edit::{DocumentKey, DocumentVersion, NotebookDocument};
use crate::server::Workspaces; use crate::server::Workspaces;
@ -131,9 +131,9 @@ impl Session {
Ok(()) Ok(())
} }
/// Reloads the settings index /// Reloads the settings index based on the provided changes.
pub(crate) fn reload_settings(&mut self, changed_url: &Url) { pub(crate) fn reload_settings(&mut self, changes: &[FileEvent]) {
self.index.reload_settings(changed_url); self.index.reload_settings(changes);
} }
/// Open a workspace folder at the given `url`. /// Open a workspace folder at the given `url`.

27
crates/ruff_server/src/session/index.rs
View file

@ -4,8 +4,8 @@ use std::path::PathBuf;
use std::{collections::BTreeMap, path::Path, sync::Arc}; use std::{collections::BTreeMap, path::Path, sync::Arc};
use anyhow::anyhow; use anyhow::anyhow;
use lsp_types::Url; use lsp_types::{FileEvent, Url};
use rustc_hash::FxHashMap; use rustc_hash::{FxHashMap, FxHashSet};
use thiserror::Error; use thiserror::Error;
pub(crate) use ruff_settings::RuffSettings; pub(crate) use ruff_settings::RuffSettings;
@ -264,9 +264,20 @@ impl Index {
Some(controller.make_ref(cell_url, url, document_settings)) Some(controller.make_ref(cell_url, url, document_settings))
} }
/// Reloads relevant existing settings files based on a changed settings file path. /// Reload the settings for all the workspace folders that contain the changed files.
pub(super) fn reload_settings(&mut self, changed_url: &Url) { ///
let Ok(changed_path) = changed_url.to_file_path() else { /// This method avoids re-indexing the same workspace multiple times if multiple files
/// belonging to the same workspace have been changed.
///
/// The file events are expected to only contain paths that map to configuration files. This is
/// registered in [`try_register_capabilities`] method.
///
/// [`try_register_capabilities`]: crate::server::Server::try_register_capabilities
pub(super) fn reload_settings(&mut self, changes: &[FileEvent]) {
let mut indexed = FxHashSet::default();
for change in changes {
let Ok(changed_path) = change.uri.to_file_path() else {
// Files that don't map to a path can't be a workspace configuration file. // Files that don't map to a path can't be a workspace configuration file.
return; return;
}; };
@ -284,6 +295,11 @@ impl Index {
break; break;
} }
if indexed.contains(root) {
continue;
}
indexed.insert(root.clone());
settings.ruff_settings = ruff_settings::RuffSettingsIndex::new( settings.ruff_settings = ruff_settings::RuffSettingsIndex::new(
root, root,
settings.client_settings.editor_settings(), settings.client_settings.editor_settings(),
@ -291,6 +307,7 @@ impl Index {
); );
} }
} }
}
pub(super) fn open_text_document(&mut self, url: Url, document: TextDocument) { pub(super) fn open_text_document(&mut self, url: Url, document: TextDocument) {
self.documents self.documents

2
crates/ruff_server/src/session/index/ruff_settings.rs
View file

@ -115,6 +115,8 @@ impl RuffSettingsIndex {
editor_settings: &ResolvedEditorSettings, editor_settings: &ResolvedEditorSettings,
is_default_workspace: bool, is_default_workspace: bool,
) -> Self { ) -> Self {
tracing::debug!("Indexing settings for workspace: {}", root.display());
let mut has_error = false; let mut has_error = false;
let mut index = BTreeMap::default(); let mut index = BTreeMap::default();
let mut respect_gitignore = None; let mut respect_gitignore = None;