diff --git a/crates/cairo-lang-language-server/src/lang/db/mod.rs b/crates/cairo-lang-language-server/src/lang/db/mod.rs index bde7ab38f43..4b59460bb7d 100644 --- a/crates/cairo-lang-language-server/src/lang/db/mod.rs +++ b/crates/cairo-lang-language-server/src/lang/db/mod.rs @@ -17,10 +17,12 @@ use cairo_lang_test_plugin::test_plugin_suite; use cairo_lang_utils::Upcast; pub use self::semantic::*; +pub use self::swapper::*; pub use self::syntax::*; use crate::Tricks; mod semantic; +mod swapper; mod syntax; /// The Cairo compiler Salsa database tailored for language server usage. diff --git a/crates/cairo-lang-language-server/src/lang/db/swapper.rs b/crates/cairo-lang-language-server/src/lang/db/swapper.rs new file mode 100644 index 00000000000..c2d97290851 --- /dev/null +++ b/crates/cairo-lang-language-server/src/lang/db/swapper.rs @@ -0,0 +1,104 @@ +use std::collections::HashSet; +use std::sync::Arc; +use std::time::{Duration, SystemTime}; + +use cairo_lang_filesystem::db::FilesGroup; +use cairo_lang_filesystem::ids::FileId; +use cairo_lang_utils::ordered_hash_map::OrderedHashMap; +use cairo_lang_utils::{Intern, LookupIntern}; +use lsp_types::Url; +use tracing::{error, warn}; + +use crate::lang::db::AnalysisDatabase; +use crate::lang::lsp::LsProtoGroup; +use crate::server::panic::ls_catch_unwind; +use crate::{Tricks, env_config}; + +/// Swaps entire [`AnalysisDatabase`] with empty one periodically. +/// +/// Salsa does not perform GC, which means that whatever computes in query groups stays in memory +/// forever (with little exception being the LRU mechanism). +/// The usage patterns of Salsa queries in the Cairo compiler cause Salsa to steadily allocate +/// new memory, which is a problem if the user is having a long coding session. +/// This object realises a nuclear GC strategy by wiping the entire analysis database from time to +/// time. +/// +/// The swapping period can be configured with environment variables. +/// Consult [`env_config::db_replace_interval`] for more information. +/// +/// The new database has a clean state. +/// It is expected that diagnostics will be refreshed on it as quickly as possible, otherwise +/// the entire workspace would be recompiled at an undetermined time leading to bad UX delays. +pub struct AnalysisDatabaseSwapper { + last_replace: SystemTime, + db_replace_interval: Duration, +} + +impl AnalysisDatabaseSwapper { + /// Creates a new `AnalysisDatabaseSwapper`. + pub fn new() -> Self { + Self { + last_replace: SystemTime::now(), + db_replace_interval: env_config::db_replace_interval(), + } + } + + /// Checks if enough time has passed since last db swap, and if so, swaps the database. + #[tracing::instrument(level = "trace", skip_all)] + pub fn maybe_swap( + &mut self, + db: &mut AnalysisDatabase, + open_files: &HashSet, + tricks: &Tricks, + ) { + let Ok(elapsed) = self.last_replace.elapsed() else { + warn!("system time went backwards, skipping db swap"); + + // Reset last replace time because in this place the old value will never make sense. + self.last_replace = SystemTime::now(); + + return; + }; + + if elapsed <= self.db_replace_interval { + // Not enough time passed since the last swap. + return; + } + + let Ok(new_db) = ls_catch_unwind(|| { + let mut new_db = AnalysisDatabase::new(tricks); + ensure_exists_in_db(&mut new_db, db, open_files.iter()); + new_db + }) else { + error!("caught panic when preparing new db for swap"); + return; + }; + + *db = new_db; + + self.last_replace = SystemTime::now(); + } +} + +/// Makes sure that all open files exist in the new db, with their current changes. +#[tracing::instrument(level = "trace", skip_all)] +fn ensure_exists_in_db<'a>( + new_db: &mut AnalysisDatabase, + old_db: &AnalysisDatabase, + open_files: impl Iterator, +) { + let overrides = old_db.file_overrides(); + let mut new_overrides: OrderedHashMap> = Default::default(); + for uri in open_files { + let Some(file_id) = old_db.file_for_url(uri) else { + // This branch is hit for open files that have never been seen by the old db. + // This is a strange condition, but it is OK to just not think about such files here. + continue; + }; + let new_file_id = file_id.lookup_intern(old_db).intern(new_db); + if let Some(content) = overrides.get(&file_id) { + new_overrides.insert(new_file_id, content.clone()); + } + } + new_db.set_file_overrides(Arc::new(new_overrides)); +} diff --git a/crates/cairo-lang-language-server/src/lib.rs b/crates/cairo-lang-language-server/src/lib.rs index 4655d7e50e7..0901b7a580d 100644 --- a/crates/cairo-lang-language-server/src/lib.rs +++ b/crates/cairo-lang-language-server/src/lib.rs @@ -43,7 +43,6 @@ use std::io; use std::panic::{AssertUnwindSafe, RefUnwindSafe, catch_unwind}; use std::path::{Path, PathBuf}; use std::process::ExitCode; -use std::sync::Arc; use std::time::SystemTime; use anyhow::{Context, Result}; @@ -61,8 +60,7 @@ use cairo_lang_project::ProjectConfig; use cairo_lang_semantic::SemanticDiagnostic; use cairo_lang_semantic::db::SemanticGroup; use cairo_lang_semantic::plugin::PluginSuite; -use cairo_lang_utils::ordered_hash_map::OrderedHashMap; -use cairo_lang_utils::{Intern, LookupIntern, Upcast}; +use cairo_lang_utils::Upcast; use itertools::Itertools; use lsp_server::Message; use lsp_types::notification::PublishDiagnostics; @@ -85,7 +83,6 @@ use crate::project::scarb::update_crate_roots; use crate::project::unmanaged_core_crate::try_to_init_unmanaged_core; use crate::server::client::{Client, Notifier, Requester, Responder}; use crate::server::connection::{Connection, ConnectionInitializer}; -use crate::server::panic::ls_catch_unwind; use crate::server::schedule::{ JoinHandle, Scheduler, Task, bounded_available_parallelism, event_loop_thread, }; @@ -226,25 +223,6 @@ fn init_logging() -> Option { guard } -/// Makes sure that all open files exist in the new db, with their current changes. -#[tracing::instrument(level = "trace", skip_all)] -fn ensure_exists_in_db<'a>( - new_db: &mut AnalysisDatabase, - old_db: &AnalysisDatabase, - open_files: impl Iterator, -) { - let overrides = old_db.file_overrides(); - let mut new_overrides: OrderedHashMap> = Default::default(); - for uri in open_files { - let Some(file_id) = old_db.file_for_url(uri) else { continue }; - let new_file_id = file_id.lookup_intern(old_db).intern(new_db); - if let Some(content) = overrides.get(&file_id) { - new_overrides.insert(new_file_id, content.clone()); - } - } - new_db.set_file_overrides(Arc::new(new_overrides)); -} - struct Backend { connection: Connection, state: State, @@ -319,16 +297,16 @@ impl Backend { Self::dispatch_setup_tasks(&mut scheduler); + // Attempt to swap the database to reduce memory use. + // Because diagnostics are always refreshed afterwards, the fresh database state will + // be quickly repopulated. + scheduler.on_sync_task(Self::maybe_swap_database); + // Refresh diagnostics each time state changes. // Although it is possible to mutate state without affecting the analysis database, // we basically never hit such a case in CairoLS in happy paths. scheduler.on_sync_task(Self::refresh_diagnostics); - // After handling diagnostics, attempt to swap the database to reduce memory use. - // TODO(mkaput): This should be an independent cronjob when diagnostics are run as - // a background task. - scheduler.on_sync_task(Self::maybe_swap_database); - let result = Self::event_loop(&connection, scheduler); if let Err(err) = connection.close() { @@ -591,79 +569,9 @@ impl Backend { result } - /// Checks if enough time passed since last db swap, and if so, swaps the database. - #[tracing::instrument(level = "trace", skip_all)] - fn maybe_swap_database(state: &mut State, notifier: Notifier) { - if state.last_replace.elapsed().unwrap() <= state.db_replace_interval { - // Not enough time passed since last swap. - return; - } - - Backend::swap_database(state, ¬ifier); - - state.last_replace = SystemTime::now(); - } - - /// Perform database swap - #[tracing::instrument(level = "debug", skip_all)] - fn swap_database(state: &mut State, notifier: &Notifier) { - debug!("scheduled"); - let Ok(mut new_db) = ls_catch_unwind(|| { - let mut new_db = AnalysisDatabase::new(&state.tricks); - ensure_exists_in_db(&mut new_db, &state.db, state.open_files.iter()); - new_db - }) else { - return; - }; - debug!("initial setup done"); - Backend::ensure_diagnostics_queries_up_to_date( - &mut new_db, - &state.scarb_toolchain, - &state.config, - state.open_files.iter(), - notifier, - ); - debug!("initial compilation done"); - debug!("starting"); - - ensure_exists_in_db(&mut new_db, &state.db, state.open_files.iter()); - state.db = new_db; - - debug!("done"); - } - - /// Ensures that all diagnostics are up to date. - #[tracing::instrument(level = "trace", skip_all)] - fn ensure_diagnostics_queries_up_to_date<'a>( - db: &mut AnalysisDatabase, - scarb_toolchain: &ScarbToolchain, - config: &Config, - open_files: impl Iterator, - notifier: &Notifier, - ) { - let query_diags = |db: &AnalysisDatabase, file_id| { - db.file_syntax_diagnostics(file_id); - let _ = db.file_semantic_diagnostics(file_id); - let _ = db.file_lowering_diagnostics(file_id); - }; - for uri in open_files { - let Some(file_id) = db.file_for_url(uri) else { continue }; - if let FileLongId::OnDisk(file_path) = file_id.lookup_intern(db) { - Backend::detect_crate_for(db, scarb_toolchain, config, &file_path, notifier); - } - query_diags(db, file_id); - } - for crate_id in db.crates() { - for module_files in db - .crate_modules(crate_id) - .iter() - .filter_map(|module_id| db.module_files(*module_id).ok()) - { - for file_id in module_files.iter().copied() { - query_diags(db, file_id); - } - } - } + /// Calls [`lang::db::AnalysisDatabaseSwapper::maybe_swap`] to do its work. + fn maybe_swap_database(state: &mut State, _notifier: Notifier) { + state.db_swapper.maybe_swap(&mut state.db, &state.open_files, &state.tricks); } /// Tries to detect the crate root the config that contains a cairo file, and add it to the diff --git a/crates/cairo-lang-language-server/src/state.rs b/crates/cairo-lang-language-server/src/state.rs index a3c8be344d7..141d51f4c7a 100644 --- a/crates/cairo-lang-language-server/src/state.rs +++ b/crates/cairo-lang-language-server/src/state.rs @@ -1,7 +1,6 @@ use std::collections::{HashMap, HashSet}; use std::ops::{Deref, DerefMut}; use std::sync::Arc; -use std::time::{Duration, SystemTime}; use cairo_lang_diagnostics::Diagnostics; use cairo_lang_lowering::diagnostic::LoweringDiagnostic; @@ -10,10 +9,10 @@ use cairo_lang_semantic::SemanticDiagnostic; use lsp_types::{ClientCapabilities, Url}; use salsa::ParallelDatabase; +use crate::Tricks; use crate::config::Config; -use crate::lang::db::AnalysisDatabase; +use crate::lang::db::{AnalysisDatabase, AnalysisDatabaseSwapper}; use crate::toolchain::scarb::ScarbToolchain; -use crate::{Tricks, env_config}; /// State of Language server. pub struct State { @@ -23,8 +22,7 @@ pub struct State { pub config: Owned, pub client_capabilities: Owned, pub scarb_toolchain: ScarbToolchain, - pub last_replace: SystemTime, - pub db_replace_interval: Duration, + pub db_swapper: AnalysisDatabaseSwapper, pub tricks: Owned, } @@ -55,10 +53,9 @@ impl State { file_diagnostics: Default::default(), config: Default::default(), client_capabilities: Owned::new(client_capabilities.into()), - tricks: Owned::new(tricks.into()), scarb_toolchain, - last_replace: SystemTime::now(), - db_replace_interval: env_config::db_replace_interval(), + db_swapper: AnalysisDatabaseSwapper::new(), + tricks: Owned::new(tricks.into()), } }