Skip to content

Commit

Permalink
LS: Get rid of needless scarb metadata calls (#6689)
Browse files Browse the repository at this point in the history
  • Loading branch information
piotmag769 authored Nov 20, 2024
1 parent 5cd0e4e commit 7808bcf
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 6 deletions.
25 changes: 22 additions & 3 deletions crates/cairo-lang-language-server/src/lang/db/swapper.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashSet;
use std::panic::{AssertUnwindSafe, catch_unwind};
use std::path::PathBuf;
use std::sync::Arc;
use std::time::{Duration, SystemTime};

Expand Down Expand Up @@ -56,6 +57,7 @@ impl AnalysisDatabaseSwapper {
config: &Config,
tricks: &Tricks,
notifier: &Notifier,
loaded_scarb_manifests: &mut HashSet<PathBuf>,
) {
let Ok(elapsed) = self.last_replace.elapsed() else {
warn!("system time went backwards, skipping db swap");
Expand All @@ -71,7 +73,7 @@ impl AnalysisDatabaseSwapper {
return;
}

self.swap(db, open_files, config, tricks, notifier)
self.swap(db, open_files, config, tricks, notifier, loaded_scarb_manifests)
}

/// Swaps the database.
Expand All @@ -83,11 +85,20 @@ impl AnalysisDatabaseSwapper {
config: &Config,
tricks: &Tricks,
notifier: &Notifier,
loaded_scarb_manifests: &mut HashSet<PathBuf>,
) {
let Ok(new_db) = catch_unwind(AssertUnwindSafe(|| {
loaded_scarb_manifests.clear();

let mut new_db = AnalysisDatabase::new(tricks);
self.migrate_file_overrides(&mut new_db, db, open_files);
self.detect_crates_for_open_files(&mut new_db, open_files, config, notifier);
self.detect_crates_for_open_files(
&mut new_db,
open_files,
config,
notifier,
loaded_scarb_manifests,
);
new_db
})) else {
error!("caught panic when preparing new db for swap");
Expand Down Expand Up @@ -130,14 +141,22 @@ impl AnalysisDatabaseSwapper {
open_files: &HashSet<Url>,
config: &Config,
notifier: &Notifier,
loaded_scarb_manifests: &mut HashSet<PathBuf>,
) {
for uri in open_files {
let Ok(file_path) = uri.to_file_path() else {
// We are only interested in physical files.
continue;
};

Backend::detect_crate_for(new_db, &self.scarb_toolchain, config, &file_path, notifier);
Backend::detect_crate_for(
new_db,
&self.scarb_toolchain,
config,
&file_path,
notifier,
loaded_scarb_manifests,
);
}
}
}
15 changes: 13 additions & 2 deletions crates/cairo-lang-language-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
//! }
//! ```
use std::collections::HashSet;
use std::panic::RefUnwindSafe;
use std::path::{Path, PathBuf};
use std::process::ExitCode;
Expand All @@ -55,7 +56,7 @@ use crossbeam::select;
use lsp_server::Message;
use lsp_types::RegistrationParams;
use salsa::{Database, Durability};
use tracing::{debug, error, info, warn};
use tracing::{debug, error, info, trace, warn};

use crate::config::Config;
use crate::lang::db::AnalysisDatabase;
Expand All @@ -66,7 +67,7 @@ use crate::lsp::capabilities::server::{
use crate::lsp::ext::{CorelibVersionMismatch, ScarbMetadataFailed};
use crate::lsp::result::LSPResult;
use crate::project::ProjectManifestPath;
use crate::project::scarb::update_crate_roots;
use crate::project::scarb::{get_workspace_members_manifests, update_crate_roots};
use crate::project::unmanaged_core_crate::try_to_init_unmanaged_core;
use crate::server::client::{Notifier, Requester, Responder};
use crate::server::connection::{Connection, ConnectionInitializer};
Expand Down Expand Up @@ -377,6 +378,7 @@ impl Backend {
&state.config,
&state.tricks,
&notifier,
&mut state.loaded_scarb_manifests,
);
}

Expand All @@ -394,9 +396,15 @@ impl Backend {
config: &Config,
file_path: &Path,
notifier: &Notifier,
loaded_scarb_manifests: &mut HashSet<PathBuf>,
) {
match ProjectManifestPath::discover(file_path) {
Some(ProjectManifestPath::Scarb(manifest_path)) => {
if loaded_scarb_manifests.contains(&manifest_path) {
trace!("scarb project is already loaded: {}", manifest_path.display());
return;
}

let metadata = scarb_toolchain
.metadata(&manifest_path)
.with_context(|| {
Expand All @@ -409,6 +417,7 @@ impl Backend {
.ok();

if let Some(metadata) = metadata {
loaded_scarb_manifests.extend(get_workspace_members_manifests(&metadata));
update_crate_roots(&metadata, db);
} else {
// Try to set up a corelib at least.
Expand Down Expand Up @@ -449,6 +458,7 @@ impl Backend {
notifier: &Notifier,
requester: &mut Requester<'_>,
) -> LSPResult<()> {
state.loaded_scarb_manifests.clear();
state.config.reload(requester, &state.client_capabilities)?;

for uri in state.open_files.iter() {
Expand All @@ -460,6 +470,7 @@ impl Backend {
&state.config,
&file_path,
notifier,
&mut state.loaded_scarb_manifests,
);
}
}
Expand Down
12 changes: 11 additions & 1 deletion crates/cairo-lang-language-server/src/project/scarb.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::collections::HashMap;
use std::fs;
use std::path::Path;
use std::path::{Path, PathBuf};

use anyhow::{Context, Result, bail, ensure};
use cairo_lang_filesystem::cfg::CfgSet;
Expand All @@ -18,6 +18,16 @@ use tracing::{debug, error, warn};
use crate::lang::db::AnalysisDatabase;
use crate::project::crate_data::Crate;

/// Get paths to manifests of the workspace members.
pub fn get_workspace_members_manifests(metadata: &Metadata) -> Vec<PathBuf> {
metadata
.workspace
.members
.iter()
.map(|package_id| metadata[package_id].manifest_path.clone().into())
.collect()
}

/// Updates crate roots in the database with the information from Scarb metadata.
///
/// This function attempts to be graceful. Any erroneous cases will be reported as warnings in logs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ impl SyncNotificationHandler for DidOpenTextDocument {
&state.config,
&path,
&notifier,
&mut state.loaded_scarb_manifests,
);
}

Expand Down
3 changes: 3 additions & 0 deletions crates/cairo-lang-language-server/src/state.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashSet;
use std::ops::{Deref, DerefMut};
use std::path::PathBuf;
use std::sync::Arc;

use lsp_types::{ClientCapabilities, Url};
Expand All @@ -17,6 +18,7 @@ use crate::toolchain::scarb::ScarbToolchain;
pub struct State {
pub db: AnalysisDatabase,
pub open_files: Owned<HashSet<Url>>,
pub loaded_scarb_manifests: Owned<HashSet<PathBuf>>,
pub config: Owned<Config>,
pub client_capabilities: Owned<ClientCapabilities>,
pub scarb_toolchain: ScarbToolchain,
Expand All @@ -38,6 +40,7 @@ impl State {
Self {
db: AnalysisDatabase::new(&tricks),
open_files: Default::default(),
loaded_scarb_manifests: Default::default(),
config: Default::default(),
client_capabilities: Owned::new(client_capabilities.into()),
scarb_toolchain,
Expand Down

0 comments on commit 7808bcf

Please sign in to comment.