Skip to content

Commit

Permalink
chore: Optimize goto_definitions for workspace case (#3914)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves 

Slow performance on bigger workspace with goto-definitions #3915

## Summary\*

Optimises performance of a request from ~16 seconds to ~1s on sample
workspace
[noir-protocol-circuits](https://github.com/AztecProtocol/aztec-packages/tree/master/yarn-project/noir-protocol-circuits)

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
kobyhallx authored Dec 22, 2023
1 parent 65cbb3d commit 66af0e7
Showing 1 changed file with 40 additions and 66 deletions.
106 changes: 40 additions & 66 deletions tooling/lsp/src/requests/goto_definition.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use std::future::{self, Future};

use crate::resolve_workspace_for_source_path;
use crate::{types::GotoDefinitionResult, LspState};
use async_lsp::{ErrorCode, LanguageClient, ResponseError};
use async_lsp::{ErrorCode, ResponseError};
use fm::codespan_files::Error;
use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse, Location};
use lsp_types::{Position, Url};
use nargo::insert_all_files_for_workspace_into_file_manager;
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING};
use noirc_driver::file_manager_with_stdlib;

pub(crate) fn on_goto_definition_request(
state: &mut LspState,
Expand All @@ -18,81 +18,55 @@ pub(crate) fn on_goto_definition_request(
}

fn on_goto_definition_inner(
state: &mut LspState,
_state: &mut LspState,
params: GotoDefinitionParams,
) -> Result<GotoDefinitionResult, ResponseError> {
let root_path = state.root_path.as_deref().ok_or_else(|| {
ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find project root")
})?;

let file_path =
params.text_document_position_params.text_document.uri.to_file_path().map_err(|_| {
ResponseError::new(ErrorCode::REQUEST_FAILED, "URI is not a valid file path")
})?;

let toml_path = match find_package_manifest(root_path, &file_path) {
Ok(toml_path) => toml_path,
Err(err) => {
let _ = state.client.log_message(lsp_types::LogMessageParams {
typ: lsp_types::MessageType::WARNING,
message: err.to_string(),
});
return Ok(None);
}
};
let workspace = resolve_workspace_from_toml(
&toml_path,
PackageSelection::All,
Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
)
.map_err(|err| {
// If we found a manifest, but the workspace is invalid, we raise an error about it
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;

let mut definition_position = None;
let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap();
let package = workspace.members.first().unwrap();

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager);

for package in &workspace {
let (mut context, crate_id) = nargo::prepare_package(&workspace_file_manager, package);

// We ignore the warnings and errors produced by compilation while resolving the definition
let _ = noirc_driver::check_crate(&mut context, crate_id, false, false);

let files = context.file_manager.as_file_map();
let file_id = context.file_manager.name_to_id(file_path.clone());

if let Some(file_id) = file_id {
let byte_index = position_to_byte_index(
files,
file_id,
&params.text_document_position_params.position,
);

if let Ok(byte_index) = byte_index {
let search_for_location = noirc_errors::Location {
file: file_id,
span: noirc_errors::Span::single_char(byte_index as u32),
};
let found_location = context.get_definition_location_from(search_for_location);

if let Some(found_location) = found_location {
let file_id = found_location.file;
definition_position = to_lsp_location(files, file_id, found_location.span);
}
}
}
}
let (mut context, crate_id) = nargo::prepare_package(&workspace_file_manager, package);

// We ignore the warnings and errors produced by compilation while resolving the definition
let _ = noirc_driver::check_crate(&mut context, crate_id, false, false);

let files = context.file_manager.as_file_map();
let file_id = context.file_manager.name_to_id(file_path.clone()).ok_or(ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("Could not find file in file manager. File path: {:?}", file_path),
))?;

let byte_index =
position_to_byte_index(files, file_id, &params.text_document_position_params.position)
.map_err(|err| {
ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("Could not convert position to byte index. Error: {:?}", err),
)
})?;

let search_for_location = noirc_errors::Location {
file: file_id,
span: noirc_errors::Span::single_char(byte_index as u32),
};

if let Some(definition_position) = definition_position {
let response: GotoDefinitionResponse =
GotoDefinitionResponse::from(definition_position).to_owned();
Ok(Some(response))
} else {
Ok(None)
}
let goto_definition_response =
context.get_definition_location_from(search_for_location).and_then(|found_location| {
let file_id = found_location.file;
let definition_position = to_lsp_location(files, file_id, found_location.span)?;
let response: GotoDefinitionResponse =
GotoDefinitionResponse::from(definition_position).to_owned();
Some(response)
});

Ok(goto_definition_response)
}

fn to_lsp_location<'a, F>(
Expand Down

0 comments on commit 66af0e7

Please sign in to comment.