Skip to content

Commit

Permalink
Enable LSP Garbage Collection test for CI (#5704)
Browse files Browse the repository at this point in the history 
## 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 <[email protected]>
Co-authored-by: IGI-111 <[email protected]>
  • Loading branch information
@JoshuaBatty @xunilrj @IGI-111
3 people authored Mar 10, 2024
1 parent 13731d9 commit 421caef
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 15 deletions.
20 changes: 15 additions & 5 deletions sway-core/src/decl_engine/engine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,24 +190,34 @@ 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)
},
}
});

$(
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,
});
)*
}
Expand Down
4 changes: 3 additions & 1 deletion sway-core/src/decl_engine/parsed_engine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
});
)*
Expand Down
8 changes: 6 additions & 2 deletions sway-core/src/type_system/engine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}

Expand Down
14 changes: 7 additions & 7 deletions sway-lsp/tests/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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;
Expand All @@ -165,7 +166,6 @@ async fn did_change_stress_test_random_wait() {
rand::random::<u64>() % 30 + 1,
))
.await;

// there is a 10% chance that a longer 300-1000ms wait will be added
if rand::random::<u64>() % 10 < 1 {
tokio::time::sleep(tokio::time::Duration::from_millis(
Expand Down

0 comments on commit 421caef

Please sign in to comment.