From 421caefe6c8241b75488f8e515ec41544a35d888 Mon Sep 17 00:00:00 2001 From: Joshua Batty Date: Mon, 11 Mar 2024 00:17:15 +1100 Subject: [PATCH] Enable LSP Garbage Collection test for CI (#5704) ## Description Enables a did_change stress test with random wait times between each trigger to emulate random key typing. This then internally kicks off compilation and triggers garbage collection every 3 keystrokes. I had intended to include this test when garbage collection was implemented in #5251. However, at that time, we were only performing GC every 10th keystroke and it was overloading the stack in CI. Now that we have reduced this to every 3rd keystroke it seems to be fine for CI. Unfortunately, this test wasn't running to catch a bug that slipped through in #5306. As such, this PR currently allows a way for us to reproduce this error but won't be able to pass CI until the underlying issue is resolved. P.S.: This also adds a temporary fix where we retain elements without a source id. This is tantamount to leaking them but seems preferable to disabling the GC altogether. --------- Co-authored-by: xunilrj Co-authored-by: IGI-111 --- sway-core/src/decl_engine/engine.rs | 20 +++++++++++++++----- sway-core/src/decl_engine/parsed_engine.rs | 4 +++- sway-core/src/type_system/engine.rs | 8 ++++++-- sway-lsp/tests/lib.rs | 14 +++++++------- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/sway-core/src/decl_engine/engine.rs b/sway-core/src/decl_engine/engine.rs index c170d8a1664..1aad8e09ef3 100644 --- a/sway-core/src/decl_engine/engine.rs +++ b/sway-core/src/decl_engine/engine.rs @@ -190,16 +190,24 @@ macro_rules! decl_engine_clear_module { self.parents.write().unwrap().retain(|key, _| { match key { AssociatedItemDeclId::TraitFn(decl_id) => { - self.get_trait_fn(decl_id).span().source_id().map_or(false, |src_id| &src_id.module_id() != module_id) + // WARNING: Setting to true disables garbage collection for these cases. + // This should be set back to false once this issue is solved: https://github.com/FuelLabs/sway/issues/5698 + self.get_trait_fn(decl_id).span().source_id().map_or(true, |src_id| &src_id.module_id() != module_id) }, AssociatedItemDeclId::Function(decl_id) => { - self.get_function(decl_id).span().source_id().map_or(false, |src_id| &src_id.module_id() != module_id) + // WARNING: Setting to true disables garbage collection for these cases. + // This should be set back to false once this issue is solved: https://github.com/FuelLabs/sway/issues/5698 + self.get_function(decl_id).span().source_id().map_or(true, |src_id| &src_id.module_id() != module_id) }, AssociatedItemDeclId::Type(decl_id) => { - self.get_type(decl_id).span().source_id().map_or(false, |src_id| &src_id.module_id() != module_id) + // WARNING: Setting to true disables garbage collection for these cases. + // This should be set back to false once this issue is solved: https://github.com/FuelLabs/sway/issues/5698 + self.get_type(decl_id).span().source_id().map_or(true, |src_id| &src_id.module_id() != module_id) }, AssociatedItemDeclId::Constant(decl_id) => { - self.get_constant(decl_id).span().source_id().map_or(false, |src_id| &src_id.module_id() != module_id) + // WARNING: Setting to true disables garbage collection for these cases. + // This should be set back to false once this issue is solved: https://github.com/FuelLabs/sway/issues/5698 + self.get_constant(decl_id).span().source_id().map_or(true, |src_id| &src_id.module_id() != module_id) }, } }); @@ -207,7 +215,9 @@ macro_rules! decl_engine_clear_module { $( self.$slab.retain(|_k, ty| match ty.span().source_id() { Some(source_id) => &source_id.module_id() != module_id, - None => false, + // WARNING: Setting to true disables garbage collection for these cases. + // This should be set back to false once this issue is solved: https://github.com/FuelLabs/sway/issues/5698 + None => true, }); )* } diff --git a/sway-core/src/decl_engine/parsed_engine.rs b/sway-core/src/decl_engine/parsed_engine.rs index c3c15fc0709..a1a2c49fbb0 100644 --- a/sway-core/src/decl_engine/parsed_engine.rs +++ b/sway-core/src/decl_engine/parsed_engine.rs @@ -142,7 +142,9 @@ macro_rules! decl_engine_clear_module { let span = $getter(item); match span.source_id() { Some(source_id) => &source_id.module_id() != module_id, - None => false, + // WARNING: Setting to true disables garbage collection for these cases. + // This should be set back to false once this issue is solved: https://github.com/FuelLabs/sway/issues/5698 + None => true, } }); )* diff --git a/sway-core/src/type_system/engine.rs b/sway-core/src/type_system/engine.rs index c6f316e3a78..a6ee204d6e5 100644 --- a/sway-core/src/type_system/engine.rs +++ b/sway-core/src/type_system/engine.rs @@ -72,14 +72,18 @@ impl TypeEngine { pub fn clear_module(&mut self, module_id: &ModuleId) { self.slab.retain(|_, tsi| match tsi.source_id { Some(source_id) => &source_id.module_id() != module_id, - None => false, + // WARNING: Setting to true disables garbage collection for these cases. + // This should be set back to false once this issue is solved: https://github.com/FuelLabs/sway/issues/5698 + None => true, }); self.id_map .write() .unwrap() .retain(|tsi, _| match tsi.source_id { Some(source_id) => &source_id.module_id() != module_id, - None => false, + // WARNING: Setting to true disables garbage collection for these cases. + // This should be set back to false once this issue is solved: https://github.com/FuelLabs/sway/issues/5698 + None => true, }); } diff --git a/sway-lsp/tests/lib.rs b/sway-lsp/tests/lib.rs index 4b6cc507b67..d660ed63ac7 100644 --- a/sway-lsp/tests/lib.rs +++ b/sway-lsp/tests/lib.rs @@ -138,7 +138,7 @@ async fn did_change_stress_test() { shutdown_and_exit(&mut service).await; } -// #[tokio::test] +#[tokio::test] #[allow(dead_code)] async fn did_change_stress_test_random_wait() { std::env::set_var("RUST_BACKTRACE", "1"); @@ -147,15 +147,16 @@ async fn did_change_stress_test_random_wait() { default_panic(panic_info); // Print the panic message std::process::exit(1); })); - let (mut service, _) = LspService::build(ServerState::new) .custom_method("sway/metrics", ServerState::metrics) .finish(); - let bench_dir = sway_workspace_dir().join("sway-lsp/tests/fixtures/benchmark"); - let uri = init_and_open(&mut service, bench_dir.join("src/main.sw")).await; - let times = 40000; + let example_dir = sway_workspace_dir() + .join(e2e_language_dir()) + .join("generics_in_contract"); + let uri = init_and_open(&mut service, example_dir.join("src/main.sw")).await; + let times = 400; for version in 0..times { - eprintln!("version: {}", version); + //eprintln!("version: {}", version); let _ = lsp::did_change_request(&mut service, &uri, version + 1).await; if version == 0 { service.inner().wait_for_parsing().await; @@ -165,7 +166,6 @@ async fn did_change_stress_test_random_wait() { rand::random::() % 30 + 1, )) .await; - // there is a 10% chance that a longer 300-1000ms wait will be added if rand::random::() % 10 < 1 { tokio::time::sleep(tokio::time::Duration::from_millis(