Skip to content

Commit f702c41

Browse files
authored
Disable DCA and writing diagnostics on did_change events (#5555)
## Description This PR does 3 optimizations. The timings below are measured against the LSP benchmarking project. 1. Disable running DCA & control flow analysis, for did_change events `39.544ms` 2. Disable running collect_types_metadata for did_change events `3.522ms` 3. Only write the diagnostics res to self.diagnostics.write() on did_open & did_save `21.135ms` I also had to increase the frequency that we are calling GC to every 3rd keystroke as during stress tests I was occasionally getting stack overflows otherwise. related to #5445 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers.
1 parent c47a885 commit f702c41

File tree

15 files changed

+168
-80
lines changed

15 files changed

+168
-80
lines changed

Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

forc-pkg/src/manifest.rs

-3
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,6 @@ pub struct BuildProfile {
238238
pub optimization_level: OptLevel,
239239
#[serde(default)]
240240
pub experimental: ExperimentalFlags,
241-
pub lsp_mode: bool,
242241
}
243242

244243
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, Default)]
@@ -734,7 +733,6 @@ impl BuildProfile {
734733
experimental: ExperimentalFlags {
735734
new_encoding: false,
736735
},
737-
lsp_mode: false,
738736
}
739737
}
740738

@@ -757,7 +755,6 @@ impl BuildProfile {
757755
experimental: ExperimentalFlags {
758756
new_encoding: false,
759757
},
760-
lsp_mode: false,
761758
}
762759
}
763760
}

forc-pkg/src/pkg.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use sway_core::{
4444
semantic_analysis::namespace,
4545
source_map::SourceMap,
4646
transform::AttributeKind,
47-
BuildTarget, Engines, FinalizedEntry,
47+
BuildTarget, Engines, FinalizedEntry, LspConfig,
4848
};
4949
use sway_error::{error::CompileError, handler::Handler, warning::CompileWarning};
5050
use sway_types::constants::{CORE, PRELUDE, STD};
@@ -1567,8 +1567,7 @@ pub fn sway_build_config(
15671567
.with_optimization_level(build_profile.optimization_level)
15681568
.with_experimental(sway_core::ExperimentalFlags {
15691569
new_encoding: build_profile.experimental.new_encoding,
1570-
})
1571-
.with_lsp_mode(build_profile.lsp_mode);
1570+
});
15721571
Ok(build_config)
15731572
}
15741573

@@ -2598,7 +2597,7 @@ pub fn check(
25982597
plan: &BuildPlan,
25992598
build_target: BuildTarget,
26002599
terse_mode: bool,
2601-
lsp_mode: bool,
2600+
lsp_mode: Option<LspConfig>,
26022601
include_tests: bool,
26032602
engines: &Engines,
26042603
retrigger_compilation: Option<Arc<AtomicBool>>,
@@ -2647,7 +2646,7 @@ pub fn check(
26472646
&profile,
26482647
)?
26492648
.with_include_tests(include_tests)
2650-
.with_lsp_mode(lsp_mode);
2649+
.with_lsp_mode(lsp_mode.clone());
26512650

26522651
let input = manifest.entry_string()?;
26532652
let handler = Handler::default();

forc-plugins/forc-doc/src/main.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,8 @@ pub fn compile_html(
216216
&plan,
217217
BuildTarget::default(),
218218
build_instructions.silent,
219+
None,
219220
tests_enabled,
220-
false,
221221
&engines,
222222
None,
223223
)?;

forc/src/ops/forc_check.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ pub fn check(command: CheckCommand, engines: &Engines) -> Result<(Option<ty::TyP
3838
&plan,
3939
build_target,
4040
terse_mode,
41-
false,
41+
None,
4242
tests_enabled,
4343
engines,
4444
None,

sway-core/src/build_config.rs

+12-3
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ pub struct BuildConfig {
5656
pub time_phases: bool,
5757
pub metrics_outfile: Option<String>,
5858
pub experimental: ExperimentalFlags,
59-
pub lsp_mode: bool,
59+
pub lsp_mode: Option<LspConfig>,
6060
}
6161

6262
impl BuildConfig {
@@ -103,7 +103,7 @@ impl BuildConfig {
103103
metrics_outfile: None,
104104
optimization_level: OptLevel::Opt0,
105105
experimental: ExperimentalFlags::default(),
106-
lsp_mode: false,
106+
lsp_mode: None,
107107
}
108108
}
109109

@@ -182,7 +182,7 @@ impl BuildConfig {
182182
}
183183
}
184184

185-
pub fn with_lsp_mode(self, lsp_mode: bool) -> Self {
185+
pub fn with_lsp_mode(self, lsp_mode: Option<LspConfig>) -> Self {
186186
Self { lsp_mode, ..self }
187187
}
188188

@@ -196,6 +196,15 @@ pub struct ExperimentalFlags {
196196
pub new_encoding: bool,
197197
}
198198

199+
#[derive(Clone, Debug, Default)]
200+
pub struct LspConfig {
201+
// This is set to true if compilation was triggered by a didChange LSP event. In this case, we
202+
// bypass collecting type metadata and skip DCA.
203+
//
204+
// This is set to false if compilation was triggered by a didSave or didOpen LSP event.
205+
pub optimized_build: bool,
206+
}
207+
199208
#[cfg(test)]
200209
mod test {
201210
use super::*;

sway-core/src/lib.rs

+60-47
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::source_map::SourceMap;
2727
pub use asm_generation::from_ir::compile_ir_to_asm;
2828
use asm_generation::FinalizedAsm;
2929
pub use asm_generation::{CompiledBytecode, FinalizedEntry};
30-
pub use build_config::{BuildConfig, BuildTarget, OptLevel};
30+
pub use build_config::{BuildConfig, BuildTarget, LspConfig, OptLevel};
3131
use control_flow_analysis::ControlFlowGraph;
3232
use metadata::MetadataManager;
3333
use query_engine::{ModuleCacheKey, ModulePath, ProgramsCacheEntry};
@@ -480,6 +480,7 @@ pub fn parsed_to_ast(
480480
retrigger_compilation: Option<Arc<AtomicBool>>,
481481
) -> Result<ty::TyProgram, ErrorEmitted> {
482482
let experimental = build_config.map(|x| x.experimental).unwrap_or_default();
483+
let lsp_config = build_config.map(|x| x.lsp_mode.clone()).unwrap_or_default();
483484

484485
// Type check the program.
485486
let typed_program_opt = ty::TyProgram::type_check(
@@ -491,16 +492,15 @@ pub fn parsed_to_ast(
491492
build_config,
492493
);
493494

495+
check_should_abort(handler, retrigger_compilation.clone())?;
496+
494497
// Only clear the parsed AST nodes if we are running a regular compilation pipeline.
495498
// LSP needs these to build its token map, and they are cleared by `clear_module` as
496499
// part of the LSP garbage collection functionality instead.
497-
let lsp_mode = build_config.map(|x| x.lsp_mode).unwrap_or_default();
498-
if !lsp_mode {
500+
if lsp_config.is_none() {
499501
engines.pe().clear();
500502
}
501503

502-
check_should_abort(handler, retrigger_compilation.clone())?;
503-
504504
let mut typed_program = match typed_program_opt {
505505
Ok(typed_program) => typed_program,
506506
Err(e) => return Err(e),
@@ -516,51 +516,62 @@ pub fn parsed_to_ast(
516516
}
517517
};
518518

519-
// Collect information about the types used in this program
520-
let types_metadata_result = typed_program.collect_types_metadata(
521-
handler,
522-
&mut CollectTypesMetadataContext::new(engines, experimental),
523-
);
524-
let types_metadata = match types_metadata_result {
525-
Ok(types_metadata) => types_metadata,
526-
Err(e) => {
527-
handler.dedup();
528-
return Err(e);
529-
}
530-
};
519+
// Skip collecting metadata if we triggered an optimised build from LSP.
520+
let types_metadata = if !lsp_config
521+
.as_ref()
522+
.map(|lsp| lsp.optimized_build)
523+
.unwrap_or(false)
524+
{
525+
// Collect information about the types used in this program
526+
let types_metadata_result = typed_program.collect_types_metadata(
527+
handler,
528+
&mut CollectTypesMetadataContext::new(engines, experimental),
529+
);
530+
let types_metadata = match types_metadata_result {
531+
Ok(types_metadata) => types_metadata,
532+
Err(e) => {
533+
handler.dedup();
534+
return Err(e);
535+
}
536+
};
531537

532-
typed_program
533-
.logged_types
534-
.extend(types_metadata.iter().filter_map(|m| match m {
535-
TypeMetadata::LoggedType(log_id, type_id) => Some((*log_id, *type_id)),
536-
_ => None,
537-
}));
538-
539-
typed_program
540-
.messages_types
541-
.extend(types_metadata.iter().filter_map(|m| match m {
542-
TypeMetadata::MessageType(message_id, type_id) => Some((*message_id, *type_id)),
543-
_ => None,
544-
}));
545-
546-
let (print_graph, print_graph_url_format) = match build_config {
547-
Some(cfg) => (
548-
cfg.print_dca_graph.clone(),
549-
cfg.print_dca_graph_url_format.clone(),
550-
),
551-
None => (None, None),
552-
};
538+
typed_program
539+
.logged_types
540+
.extend(types_metadata.iter().filter_map(|m| match m {
541+
TypeMetadata::LoggedType(log_id, type_id) => Some((*log_id, *type_id)),
542+
_ => None,
543+
}));
553544

554-
check_should_abort(handler, retrigger_compilation.clone())?;
545+
typed_program
546+
.messages_types
547+
.extend(types_metadata.iter().filter_map(|m| match m {
548+
TypeMetadata::MessageType(message_id, type_id) => Some((*message_id, *type_id)),
549+
_ => None,
550+
}));
551+
552+
let (print_graph, print_graph_url_format) = match build_config {
553+
Some(cfg) => (
554+
cfg.print_dca_graph.clone(),
555+
cfg.print_dca_graph_url_format.clone(),
556+
),
557+
None => (None, None),
558+
};
555559

556-
// Perform control flow analysis and extend with any errors.
557-
let _ = perform_control_flow_analysis(
558-
handler,
559-
engines,
560-
&typed_program,
561-
print_graph,
562-
print_graph_url_format,
563-
);
560+
check_should_abort(handler, retrigger_compilation.clone())?;
561+
562+
// Perform control flow analysis and extend with any errors.
563+
let _ = perform_control_flow_analysis(
564+
handler,
565+
engines,
566+
&typed_program,
567+
print_graph,
568+
print_graph_url_format,
569+
);
570+
571+
types_metadata
572+
} else {
573+
vec![]
574+
};
564575

565576
// Evaluate const declarations, to allow storage slots initialization with consts.
566577
let mut ctx = Context::new(
@@ -713,6 +724,8 @@ pub fn compile_to_ast(
713724
query_engine.insert_programs_cache_entry(cache_entry);
714725
}
715726

727+
check_should_abort(handler, retrigger_compilation.clone())?;
728+
716729
Ok(programs)
717730
}
718731

sway-lsp/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ futures = { version = "0.3", default-features = false, features = [
6060
"async-await",
6161
] }
6262
pretty_assertions = "1.4.0"
63+
rand = "0.8"
6364
regex = "^1.10.2"
6465
sway-lsp-test-utils = { path = "tests/utils" }
6566
tikv-jemallocator = "0.5"

sway-lsp/benches/lsp_benchmarks/compile.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,33 @@ const NUM_DID_CHANGE_ITERATIONS: usize = 10;
99
fn benchmarks(c: &mut Criterion) {
1010
// Load the test project
1111
let uri = Url::from_file_path(super::benchmark_dir().join("src/main.sw")).unwrap();
12+
let mut lsp_mode = Some(sway_core::LspConfig {
13+
optimized_build: false,
14+
});
1215
c.bench_function("compile", |b| {
1316
b.iter(|| {
1417
let engines = Engines::default();
15-
let _ = black_box(session::compile(&uri, &engines, None).unwrap());
18+
let _ = black_box(session::compile(&uri, &engines, None, lsp_mode.clone()).unwrap());
1619
})
1720
});
1821

1922
c.bench_function("traverse", |b| {
2023
let engines = Engines::default();
21-
let results = black_box(session::compile(&uri, &engines, None).unwrap());
24+
let results = black_box(session::compile(&uri, &engines, None, lsp_mode.clone()).unwrap());
2225
let session = Arc::new(session::Session::new());
2326
b.iter(|| {
2427
let _ =
2528
black_box(session::traverse(results.clone(), &engines, session.clone()).unwrap());
2629
})
2730
});
2831

32+
lsp_mode.as_mut().unwrap().optimized_build = true;
2933
c.bench_function("did_change_with_caching", |b| {
3034
let engines = Engines::default();
3135
b.iter(|| {
3236
for _ in 0..NUM_DID_CHANGE_ITERATIONS {
33-
let _ = black_box(session::compile(&uri, &engines, None).unwrap());
37+
let _ =
38+
black_box(session::compile(&uri, &engines, None, lsp_mode.clone()).unwrap());
3439
}
3540
})
3641
});

sway-lsp/benches/lsp_benchmarks/mod.rs

+11-1
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,21 @@ use sway_lsp::core::session::{self, Session};
88

99
pub async fn compile_test_project() -> (Url, Arc<Session>) {
1010
let session = Arc::new(Session::new());
11+
let lsp_mode = Some(sway_core::LspConfig {
12+
optimized_build: false,
13+
});
1114
// Load the test project
1215
let uri = Url::from_file_path(benchmark_dir().join("src/main.sw")).unwrap();
1316
session.handle_open_file(&uri).await;
1417
// Compile the project
15-
session::parse_project(&uri, &session.engines.read(), None, session.clone()).unwrap();
18+
session::parse_project(
19+
&uri,
20+
&session.engines.read(),
21+
None,
22+
lsp_mode,
23+
session.clone(),
24+
)
25+
.unwrap();
1626
(uri, session)
1727
}
1828

0 commit comments

Comments
 (0)