From 0b440eacab9a6fedd1a6b25a1111f41cc6fee9ef Mon Sep 17 00:00:00 2001 From: Joshua Batty Date: Mon, 22 Jul 2024 12:23:34 +0200 Subject: [PATCH] Cache Build Plans in language server (#6281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description closes #5462 related #5445 before we were recreating the build plan on every keystroke. We now cache the result and reuse on subsequent did change events. We invalidate the cache if there has been any modifications to the `Forc.toml` file since the cache was created. Here are the timings when triggering this with a did_change event on the FUSD repo. previous: `80.146ms` with this change: `56µs` --- sway-lsp/benches/lsp_benchmarks/compile.rs | 28 +++++++--- sway-lsp/src/core/session.rs | 65 ++++++++++++++++++++-- sway-lsp/src/core/sync.rs | 2 +- sway-lsp/src/error.rs | 2 + 4 files changed, 82 insertions(+), 15 deletions(-) diff --git a/sway-lsp/benches/lsp_benchmarks/compile.rs b/sway-lsp/benches/lsp_benchmarks/compile.rs index 6916582e271..fdab8f01094 100644 --- a/sway-lsp/benches/lsp_benchmarks/compile.rs +++ b/sway-lsp/benches/lsp_benchmarks/compile.rs @@ -1,27 +1,36 @@ use criterion::{black_box, criterion_group, Criterion}; -use lsp_types::Url; use std::sync::Arc; use sway_core::{Engines, ExperimentalFlags}; use sway_lsp::core::session; +use tokio::runtime::Runtime; const NUM_DID_CHANGE_ITERATIONS: usize = 10; fn benchmarks(c: &mut Criterion) { - let experimental = ExperimentalFlags { - new_encoding: false, - }; + let (uri, session, _) = Runtime::new() + .unwrap() + .block_on(async { black_box(super::compile_test_project().await) }); + + let build_plan = session + .build_plan_cache + .get_or_update(&session.sync.manifest_path(), || session::build_plan(&uri)) + .unwrap(); - // Load the test project - let uri = Url::from_file_path(super::benchmark_dir().join("src/main.sw")).unwrap(); let mut lsp_mode = Some(sway_core::LspConfig { optimized_build: false, file_versions: Default::default(), }); + + let experimental = ExperimentalFlags { + new_encoding: false, + }; + c.bench_function("compile", |b| { b.iter(|| { let engines = Engines::default(); let _ = black_box( - session::compile(&uri, &engines, None, lsp_mode.clone(), experimental).unwrap(), + session::compile(&build_plan, &engines, None, lsp_mode.clone(), experimental) + .unwrap(), ); }) }); @@ -29,7 +38,7 @@ fn benchmarks(c: &mut Criterion) { c.bench_function("traverse", |b| { let engines = Engines::default(); let results = black_box( - session::compile(&uri, &engines, None, lsp_mode.clone(), experimental).unwrap(), + session::compile(&build_plan, &engines, None, lsp_mode.clone(), experimental).unwrap(), ); let session = Arc::new(session::Session::new()); b.iter(|| { @@ -44,7 +53,8 @@ fn benchmarks(c: &mut Criterion) { b.iter(|| { for _ in 0..NUM_DID_CHANGE_ITERATIONS { let _ = black_box( - session::compile(&uri, &engines, None, lsp_mode.clone(), experimental).unwrap(), + session::compile(&build_plan, &engines, None, lsp_mode.clone(), experimental) + .unwrap(), ); } }) diff --git a/sway-lsp/src/core/session.rs b/sway-lsp/src/core/session.rs index e0fe109c43a..6dab2952254 100644 --- a/sway-lsp/src/core/session.rs +++ b/sway-lsp/src/core/session.rs @@ -29,6 +29,7 @@ use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use std::{ path::PathBuf, sync::{atomic::AtomicBool, Arc}, + time::SystemTime, }; use sway_core::{ decl_engine::DeclEngine, @@ -61,6 +62,7 @@ pub struct CompiledProgram { pub struct Session { token_map: TokenMap, pub runnables: RunnableMap, + pub build_plan_cache: BuildPlanCache, pub compiled_program: RwLock, pub engines: RwLock, pub sync: SyncWorkspace, @@ -80,6 +82,7 @@ impl Session { Session { token_map: TokenMap::new(), runnables: DashMap::new(), + build_plan_cache: BuildPlanCache::default(), metrics: DashMap::new(), compiled_program: RwLock::new(CompiledProgram::default()), engines: <_>::default(), @@ -228,7 +231,7 @@ impl Session { } /// Create a [BuildPlan] from the given [Url] appropriate for the language server. -pub(crate) fn build_plan(uri: &Url) -> Result { +pub fn build_plan(uri: &Url) -> Result { let _p = tracing::trace_span!("build_plan").entered(); let manifest_dir = PathBuf::from(uri.path()); let manifest = @@ -254,17 +257,16 @@ pub(crate) fn build_plan(uri: &Url) -> Result { } pub fn compile( - uri: &Url, + build_plan: &BuildPlan, engines: &Engines, retrigger_compilation: Option>, lsp_mode: Option, experimental: sway_core::ExperimentalFlags, ) -> Result, Handler)>, LanguageServerError> { let _p = tracing::trace_span!("compile").entered(); - let build_plan = build_plan(uri)?; let tests_enabled = true; pkg::check( - &build_plan, + build_plan, BuildTarget::default(), true, lsp_mode, @@ -382,8 +384,11 @@ pub fn parse_project( experimental: sway_core::ExperimentalFlags, ) -> Result<(), LanguageServerError> { let _p = tracing::trace_span!("parse_project").entered(); + let build_plan = session + .build_plan_cache + .get_or_update(&session.sync.manifest_path(), || build_plan(uri))?; let results = compile( - uri, + &build_plan, engines, retrigger_compilation, lsp_mode.clone(), @@ -508,6 +513,56 @@ pub(crate) fn program_id_from_path( Ok(program_id) } +/// A cache for storing and retrieving BuildPlan objects. +#[derive(Debug, Clone)] +pub struct BuildPlanCache { + /// The cached BuildPlan and its last update time + cache: Arc>>, +} + +impl Default for BuildPlanCache { + fn default() -> Self { + Self { + cache: Arc::new(RwLock::new(None)), + } + } +} + +impl BuildPlanCache { + /// Retrieves a BuildPlan from the cache or updates it if necessary. + pub fn get_or_update( + &self, + manifest_path: &Option, + update_fn: F, + ) -> Result + where + F: FnOnce() -> Result, + { + let should_update = { + let cache = self.cache.read(); + manifest_path + .as_ref() + .and_then(|path| path.metadata().ok()?.modified().ok()) + .map_or(cache.is_none(), |time| { + cache.as_ref().map_or(true, |&(_, last)| time > last) + }) + }; + + if should_update { + let new_plan = update_fn()?; + let mut cache = self.cache.write(); + *cache = Some((new_plan.clone(), SystemTime::now())); + Ok(new_plan) + } else { + let cache = self.cache.read(); + cache + .as_ref() + .map(|(plan, _)| plan.clone()) + .ok_or(LanguageServerError::BuildPlanCacheIsEmpty) + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/sway-lsp/src/core/sync.rs b/sway-lsp/src/core/sync.rs index bf6c9fa73a7..f00189880fe 100644 --- a/sway-lsp/src/core/sync.rs +++ b/sway-lsp/src/core/sync.rs @@ -175,7 +175,7 @@ impl SyncWorkspace { .ok() } - pub(crate) fn manifest_path(&self) -> Option { + pub fn manifest_path(&self) -> Option { self.manifest_dir() .map(|dir| dir.join(sway_utils::constants::MANIFEST_FILE_NAME)) .ok() diff --git a/sway-lsp/src/error.rs b/sway-lsp/src/error.rs index 740a1127d46..e11fd90d129 100644 --- a/sway-lsp/src/error.rs +++ b/sway-lsp/src/error.rs @@ -14,6 +14,8 @@ pub enum LanguageServerError { // Top level errors #[error("Failed to create build plan. {0}")] BuildPlanFailed(anyhow::Error), + #[error("Build Plan Cache is empty")] + BuildPlanCacheIsEmpty, #[error("Failed to compile. {0}")] FailedToCompile(anyhow::Error), #[error("Failed to parse document")]