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.

Sign up for GitHub

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

Jump to bottom

perf(lsp): store settings in Arc #24191

Merged
merged 1 commit into from
Jun 13, 2024
Merged

perf(lsp): store settings in Arc #24191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 52 additions & 30 deletions cli/lsp/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,8 +722,9 @@ impl WorkspaceSettings {

#[derive(Debug, Default, Clone)]
pub struct Settings {
pub unscoped: WorkspaceSettings,
pub by_workspace_folder: BTreeMap<ModuleSpecifier, Option<WorkspaceSettings>>,
pub unscoped: Arc<WorkspaceSettings>,
pub by_workspace_folder:
BTreeMap<ModuleSpecifier, Option<Arc<WorkspaceSettings>>>,
pub first_folder: Option<ModuleSpecifier>,
}

Expand Down Expand Up @@ -815,9 +816,9 @@ impl Settings {

#[derive(Clone, Debug, Default)]
pub struct Config {
pub client_capabilities: ClientCapabilities,
pub settings: Settings,
pub workspace_folders: Vec<(ModuleSpecifier, lsp::WorkspaceFolder)>,
pub client_capabilities: Arc<ClientCapabilities>,
pub settings: Arc<Settings>,
pub workspace_folders: Arc<Vec<(ModuleSpecifier, lsp::WorkspaceFolder)>>,
pub tree: ConfigTree,
}

Expand Down Expand Up @@ -845,25 +846,33 @@ impl Config {
&mut self,
folders: Vec<(ModuleSpecifier, lsp::WorkspaceFolder)>,
) {
self.settings.by_workspace_folder =
folders.iter().map(|(s, _)| (s.clone(), None)).collect();
self.settings.first_folder = folders.first().map(|(s, _)| s.clone());
self.workspace_folders = folders;
self.settings = Arc::new(Settings {
unscoped: self.settings.unscoped.clone(),
by_workspace_folder: folders
.iter()
.map(|(s, _)| (s.clone(), None))
.collect(),
first_folder: folders.first().map(|(s, _)| s.clone()),
});
self.workspace_folders = Arc::new(folders);
}

pub fn set_workspace_settings(
&mut self,
unscoped: WorkspaceSettings,
folder_settings: Vec<(ModuleSpecifier, WorkspaceSettings)>,
) {
self.settings.unscoped = unscoped;
for (folder_uri, settings) in folder_settings.into_iter() {
if let Some(settings_) =
self.settings.by_workspace_folder.get_mut(&folder_uri)
{
*settings_ = Some(settings);
}
}
let mut by_folder = folder_settings.into_iter().collect::<HashMap<_, _>>();
self.settings = Arc::new(Settings {
unscoped: Arc::new(unscoped),
by_workspace_folder: self
.settings
.by_workspace_folder
.keys()
.map(|s| (s.clone(), by_folder.remove(s).map(Arc::new)))
.collect(),
first_folder: self.settings.first_folder.clone(),
});
}

pub fn workspace_settings(&self) -> &WorkspaceSettings {
Expand Down Expand Up @@ -966,7 +975,7 @@ impl Config {
&mut self,
client_capabilities: ClientCapabilities,
) {
self.client_capabilities = client_capabilities;
self.client_capabilities = Arc::new(client_capabilities);
}

pub fn workspace_capable(&self) -> bool {
Expand Down Expand Up @@ -1906,10 +1915,15 @@ mod tests {
fn test_config_specifier_disabled_path() {
let root_uri = resolve_url("file:///root/").unwrap();
let mut config = Config::new_with_roots(vec![root_uri.clone()]);
config.settings.unscoped.enable = Some(true);
config.settings.unscoped.enable_paths =
Some(vec!["mod1.ts".to_string(), "mod2.ts".to_string()]);
config.settings.unscoped.disable_paths = vec!["mod2.ts".to_string()];
config.set_workspace_settings(
WorkspaceSettings {
enable: Some(true),
enable_paths: Some(vec!["mod1.ts".to_string(), "mod2.ts".to_string()]),
disable_paths: vec!["mod2.ts".to_string()],
..Default::default()
},
vec![],
);

assert!(config.specifier_enabled(&root_uri.join("mod1.ts").unwrap()));
assert!(!config.specifier_enabled(&root_uri.join("mod2.ts").unwrap()));
Expand Down Expand Up @@ -2107,7 +2121,6 @@ mod tests {
async fn config_enable_via_config_file_detection() {
let root_uri = resolve_url("file:///root/").unwrap();
let mut config = Config::new_with_roots(vec![root_uri.clone()]);
config.settings.unscoped.enable = None;
assert!(!config.specifier_enabled(&root_uri));

config
Expand All @@ -2129,19 +2142,27 @@ mod tests {
fn config_specifier_enabled_matches_by_path_component() {
let root_uri = resolve_url("file:///root/").unwrap();
let mut config = Config::new_with_roots(vec![root_uri.clone()]);
config.settings.unscoped.enable_paths = Some(vec!["mo".to_string()]);
config.set_workspace_settings(
WorkspaceSettings {
enable_paths: Some(vec!["mo".to_string()]),
..Default::default()
},
vec![],
);
assert!(!config.specifier_enabled(&root_uri.join("mod.ts").unwrap()));
}

#[tokio::test]
async fn config_specifier_enabled_for_test() {
let root_uri = resolve_url("file:///root/").unwrap();
let mut config = Config::new_with_roots(vec![root_uri.clone()]);
config.settings.unscoped.enable = Some(true);

config.settings.unscoped.enable_paths =
Some(vec!["mod1.ts".to_string(), "mod2.ts".to_string()]);
config.settings.unscoped.disable_paths = vec!["mod2.ts".to_string()];
let mut settings = WorkspaceSettings {
enable: Some(true),
enable_paths: Some(vec!["mod1.ts".to_string(), "mod2.ts".to_string()]),
disable_paths: vec!["mod2.ts".to_string()],
..Default::default()
};
config.set_workspace_settings(settings.clone(), vec![]);
assert!(
config.specifier_enabled_for_test(&root_uri.join("mod1.ts").unwrap())
);
Expand All @@ -2151,7 +2172,8 @@ mod tests {
assert!(
!config.specifier_enabled_for_test(&root_uri.join("mod3.ts").unwrap())
);
config.settings.unscoped.enable_paths = None;
settings.enable_paths = None;
config.set_workspace_settings(settings, vec![]);

config
.tree
Expand Down
23 changes: 13 additions & 10 deletions cli/lsp/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1611,21 +1611,21 @@ mod tests {
fn mock_config() -> Config {
let root_uri = resolve_url("file:///").unwrap();
Config {
settings: Settings {
unscoped: WorkspaceSettings {
settings: Arc::new(Settings {
unscoped: Arc::new(WorkspaceSettings {
enable: Some(true),
lint: true,
..Default::default()
},
}),
..Default::default()
},
workspace_folders: vec![(
}),
workspace_folders: Arc::new(vec![(
root_uri.clone(),
lsp::WorkspaceFolder {
uri: root_uri,
name: "".to_string(),
},
)],
)]),
..Default::default()
}
}
Expand Down Expand Up @@ -1719,10 +1719,13 @@ let c: number = "a";
// now test disabled specifier
{
let mut disabled_config = mock_config();
disabled_config.settings.unscoped = WorkspaceSettings {
enable: Some(false),
..Default::default()
};
disabled_config.set_workspace_settings(
WorkspaceSettings {
enable: Some(false),
..Default::default()
},
vec![],
);

let diagnostics = generate_lint_diagnostics(
&snapshot,
Expand Down
6 changes: 3 additions & 3 deletions cli/lsp/language_server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ impl LanguageServer {
if capable {
let mut scopes = Vec::with_capacity(folders.len() + 1);
scopes.push(None);
for (_, folder) in &folders {
for (_, folder) in folders.as_ref() {
scopes.push(Some(folder.uri.clone()));
}
let configs = client
Expand All @@ -461,7 +461,7 @@ impl LanguageServer {
let mut configs = configs.into_iter();
let unscoped = configs.next().unwrap();
let mut folder_settings = Vec::with_capacity(folders.len());
for (folder_uri, _) in &folders {
for (folder_uri, _) in folders.as_ref() {
folder_settings.push((folder_uri.clone(), configs.next().unwrap()));
}
let mut inner = self.inner.write().await;
Expand Down Expand Up @@ -3146,7 +3146,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
)
})
.collect::<Vec<(ModuleSpecifier, WorkspaceFolder)>>();
for (specifier, folder) in &inner.config.workspace_folders {
for (specifier, folder) in inner.config.workspace_folders.as_ref() {
if !params.event.removed.is_empty()
&& params.event.removed.iter().any(|f| f.uri == folder.uri)
{
Expand Down