Skip to content

Commit

Permalink
LS: Make database swapping use benefits of new architecture (#6495)
Browse files Browse the repository at this point in the history
  • Loading branch information
mkaput authored Oct 17, 2024
1 parent deddcc7 commit 78303aa
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 109 deletions.
2 changes: 2 additions & 0 deletions crates/cairo-lang-language-server/src/lang/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
104 changes: 104 additions & 0 deletions crates/cairo-lang-language-server/src/lang/db/swapper.rs
Original file line number Diff line number Diff line change
@@ -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<Url>,
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<Item = &'a Url>,
) {
let overrides = old_db.file_overrides();
let mut new_overrides: OrderedHashMap<FileId, Arc<str>> = 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));
}
110 changes: 9 additions & 101 deletions crates/cairo-lang-language-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand All @@ -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,
};
Expand Down Expand Up @@ -226,25 +223,6 @@ fn init_logging() -> Option<impl Drop> {
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<Item = &'a Url>,
) {
let overrides = old_db.file_overrides();
let mut new_overrides: OrderedHashMap<FileId, Arc<str>> = 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,
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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, &notifier);

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<Item = &'a Url>,
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
Expand Down
13 changes: 5 additions & 8 deletions crates/cairo-lang-language-server/src/state.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {
Expand All @@ -23,8 +22,7 @@ pub struct State {
pub config: Owned<Config>,
pub client_capabilities: Owned<ClientCapabilities>,
pub scarb_toolchain: ScarbToolchain,
pub last_replace: SystemTime,
pub db_replace_interval: Duration,
pub db_swapper: AnalysisDatabaseSwapper,
pub tricks: Owned<Tricks>,
}

Expand Down Expand Up @@ -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()),
}
}

Expand Down

0 comments on commit 78303aa

Please sign in to comment.