Skip to content

Commit

Permalink
[move-ide] Avoid analyzing unmodified dependencies (MystenLabs#20046)
Browse files Browse the repository at this point in the history
## Description 

This PR implements an optimization to the symbolication algorithm that
avoids analyzing dependencies if they have not changed. Previously we
were avoiding re-compiling unchanged dependencies but we were still
analyzing them which introduces unnecessary overhead.

The implementation involved separating analysis of the main program and
the dependencies, and merging the results of the two together in the end
(whether the dependencies are computed fresh or obtained from the
cache). In particular, we now create two analysis visitors per analysis
phase, one for the main program and one for the dependencies.

For a simple package with Sui framework as a dependency we observe a
significant reduction in analysis time.

Before:

![image](https://github.com/user-attachments/assets/06148508-9b22-4f52-b8c9-21968dcc4b1f)

After:

![image](https://github.com/user-attachments/assets/d14f6b5e-f2fa-4312-a1c2-81f68ce210c2)


## Test plan 

All existing test must pass. I also tested manually to verify that
reporting references (now merged between the main program and the
dependencies) works correctly.
  • Loading branch information
awelc authored Oct 28, 2024
1 parent 1837a5e commit 495dbb7
Show file tree
Hide file tree
Showing 4 changed files with 326 additions and 135 deletions.
12 changes: 7 additions & 5 deletions external-crates/move/crates/move-analyzer/src/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ pub fn run() {

let (connection, io_threads) = Connection::stdio();
let symbols_map = Arc::new(Mutex::new(BTreeMap::new()));
let pkg_deps = Arc::new(Mutex::new(
BTreeMap::<PathBuf, symbols::PrecompiledPkgDeps>::new(),
));
let pkg_deps = Arc::new(Mutex::new(BTreeMap::<
PathBuf,
symbols::PrecomputedPkgDepsInfo,
>::new()));
let ide_files_root: VfsPath = MemoryFS::new().into();

let (id, client_response) = connection
Expand Down Expand Up @@ -147,7 +148,8 @@ pub fn run() {
// main reason for this is to enable unit tests that rely on the symbolication information
// to be available right after the client is initialized.
if let Some(uri) = initialize_params.root_uri {
if let Some(p) = symbols::SymbolicatorRunner::root_dir(&uri.to_file_path().unwrap()) {
let build_path = uri.to_file_path().unwrap();
if let Some(p) = symbols::SymbolicatorRunner::root_dir(&build_path) {
if let Ok((Some(new_symbols), _)) = symbols::get_symbols(
Arc::new(Mutex::new(BTreeMap::new())),
ide_files_root.clone(),
Expand Down Expand Up @@ -277,7 +279,7 @@ fn on_request(
context: &Context,
request: &Request,
ide_files_root: VfsPath,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, symbols::PrecompiledPkgDeps>>>,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, symbols::PrecomputedPkgDepsInfo>>>,
shutdown_request_received: bool,
) -> bool {
if shutdown_request_received {
Expand Down
10 changes: 5 additions & 5 deletions external-crates/move/crates/move-analyzer/src/completions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
utils::{completion_item, PRIMITIVE_TYPE_COMPLETIONS},
},
context::Context,
symbols::{self, CursorContext, PrecompiledPkgDeps, SymbolicatorRunner, Symbols},
symbols::{self, CursorContext, PrecomputedPkgDepsInfo, SymbolicatorRunner, Symbols},
};
use lsp_server::Request;
use lsp_types::{CompletionItem, CompletionItemKind, CompletionParams, Position};
Expand Down Expand Up @@ -78,7 +78,7 @@ pub fn on_completion_request(
context: &Context,
request: &Request,
ide_files_root: VfsPath,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, PrecompiledPkgDeps>>>,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, PrecomputedPkgDepsInfo>>>,
) {
eprintln!("handling completion request");
let parameters = serde_json::from_value::<CompletionParams>(request.params.clone())
Expand Down Expand Up @@ -119,7 +119,7 @@ pub fn on_completion_request(
fn completions(
context: &Context,
ide_files_root: VfsPath,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, PrecompiledPkgDeps>>>,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, PrecomputedPkgDepsInfo>>>,
path: &Path,
pos: Position,
) -> Option<Vec<CompletionItem>> {
Expand All @@ -143,7 +143,7 @@ fn completions(
pub fn compute_completions(
current_symbols: &Symbols,
ide_files_root: VfsPath,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, PrecompiledPkgDeps>>>,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, PrecomputedPkgDepsInfo>>>,
path: &Path,
pos: Position,
) -> Vec<CompletionItem> {
Expand All @@ -156,7 +156,7 @@ pub fn compute_completions(
/// view of the code (returns `None` if the symbols could not be re-computed).
fn compute_completions_new_symbols(
ide_files_root: VfsPath,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, PrecompiledPkgDeps>>>,
pkg_dependencies: Arc<Mutex<BTreeMap<PathBuf, PrecomputedPkgDepsInfo>>>,
path: &Path,
cursor_position: Position,
) -> Option<Vec<CompletionItem>> {
Expand Down
Loading

0 comments on commit 495dbb7

Please sign in to comment.