Skip to content

Commit

Permalink
LS: Reduce noise in trace profiles' slices (#6517)
Browse files Browse the repository at this point in the history
  • Loading branch information
mkaput authored Oct 25, 2024
1 parent 3ea3412 commit 02fca5c
Show file tree
Hide file tree
Showing 21 changed files with 64 additions and 93 deletions.
1 change: 0 additions & 1 deletion crates/cairo-lang-language-server/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ pub struct Config {

impl Config {
/// Reloads the configuration from the language client.
#[tracing::instrument(name = "reload_config", level = "trace", skip_all)]
pub fn reload(
&mut self,
requester: &mut Requester<'_>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use crate::lang::db::{AnalysisDatabase, LsSemanticGroup};
use crate::lang::lsp::{LsProtoGroup, ToLsp};

/// Create a Quick Fix code action to add a missing trait given a `CannotCallMethod` diagnostic.
#[tracing::instrument(level = "trace", skip_all)]
pub fn add_missing_trait(db: &AnalysisDatabase, node: &SyntaxNode, uri: Url) -> Vec<CodeAction> {
let file_id = db.file_for_url(&uri).unwrap();
let lookup_items = db.collect_lookup_items_stack(node).unwrap();
Expand Down
5 changes: 0 additions & 5 deletions crates/cairo-lang-language-server/src/ide/code_actions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ mod rename_unused_variable;

/// Compute commands for a given text document and range. These commands are typically code fixes to
/// either fix problems or to beautify/refactor code.
#[tracing::instrument(
level = "debug",
skip_all,
fields(uri = %params.text_document.uri)
)]
pub fn code_actions(params: CodeActionParams, db: &AnalysisDatabase) -> Option<CodeActionResponse> {
let mut actions = Vec::with_capacity(params.context.diagnostics.len());
let file_id = db.file_for_url(&params.text_document.uri)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use lsp_types::{CodeAction, Diagnostic, TextEdit, Url, WorkspaceEdit};
use crate::lang::db::AnalysisDatabase;

/// Create a code action that prefixes an unused variable with an `_`.
#[tracing::instrument(level = "trace", skip_all)]
pub fn rename_unused_variable(
db: &AnalysisDatabase,
node: &SyntaxNode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use crate::ide::utils::find_methods_for_type;
use crate::lang::db::{AnalysisDatabase, LsSemanticGroup};
use crate::lang::lsp::ToLsp;

#[tracing::instrument(level = "trace", skip_all)]
pub fn generic_completions(
db: &AnalysisDatabase,
module_file_id: ModuleFileId,
Expand Down Expand Up @@ -106,7 +105,6 @@ fn resolved_generic_item_completion_kind(item: ResolvedGenericItem) -> Completio
}
}

#[tracing::instrument(level = "trace", skip_all)]
pub fn colon_colon_completions(
db: &AnalysisDatabase,
module_file_id: ModuleFileId,
Expand Down Expand Up @@ -181,7 +179,6 @@ pub fn colon_colon_completions(
})
}

#[tracing::instrument(level = "trace", skip_all)]
pub fn dot_completions(
db: &AnalysisDatabase,
file_id: FileId,
Expand Down Expand Up @@ -254,7 +251,6 @@ pub fn dot_completions(
}

/// Returns a completion item for a method.
#[tracing::instrument(level = "trace", skip_all)]
pub fn completion_for_method(
db: &AnalysisDatabase,
module_id: ModuleId,
Expand Down Expand Up @@ -292,7 +288,6 @@ pub fn completion_for_method(
}

/// Checks if a module has a trait in scope.
#[tracing::instrument(level = "trace", skip_all)]
fn module_has_trait(
db: &AnalysisDatabase,
module_id: ModuleId,
Expand Down
7 changes: 0 additions & 7 deletions crates/cairo-lang-language-server/src/ide/completion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ use crate::lang::lsp::{LsProtoGroup, ToCairo};
mod completions;

/// Compute completion items at a given cursor position.
#[tracing::instrument(
level = "debug",
skip_all,
fields(uri = %params.text_document_position.text_document.uri)
)]
pub fn complete(params: CompletionParams, db: &AnalysisDatabase) -> Option<CompletionResponse> {
let text_document_position = params.text_document_position;
let file_id = db.file_for_url(&text_document_position.text_document.uri)?;
Expand Down Expand Up @@ -61,7 +56,6 @@ enum CompletionKind {
ColonColon(Vec<PathSegment>),
}

#[tracing::instrument(level = "trace", skip_all)]
fn completion_kind(db: &AnalysisDatabase, node: SyntaxNode) -> CompletionKind {
debug!("node.kind: {:#?}", node.kind(db));
match node.kind(db) {
Expand Down Expand Up @@ -148,7 +142,6 @@ fn completion_kind(db: &AnalysisDatabase, node: SyntaxNode) -> CompletionKind {
CompletionKind::ColonColon(vec![])
}

#[tracing::instrument(level = "trace", skip_all)]
fn completion_kind_from_path_node(db: &AnalysisDatabase, parent: SyntaxNode) -> CompletionKind {
debug!("completion_kind_from_path_node: {}", parent.clone().get_text_without_trivia(db));
let expr = ast::ExprPath::from_syntax_node(db, parent);
Expand Down
5 changes: 0 additions & 5 deletions crates/cairo-lang-language-server/src/ide/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ use crate::lang::db::AnalysisDatabase;
use crate::lang::lsp::LsProtoGroup;

/// Format a whole document.
#[tracing::instrument(
level = "debug",
skip_all,
fields(uri = %params.text_document.uri)
)]
pub fn format(params: DocumentFormattingParams, db: &AnalysisDatabase) -> Option<Vec<TextEdit>> {
let file_uri = params.text_document.uri;
let file = db.file_for_url(&file_uri)?;
Expand Down
5 changes: 0 additions & 5 deletions crates/cairo-lang-language-server/src/ide/hover/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@ use crate::lang::lsp::{LsProtoGroup, ToCairo};
mod render;

/// Get hover information at a given text document position.
#[tracing::instrument(
level = "debug",
skip_all,
fields(uri = %params.text_document_position_params.text_document.uri)
)]
pub fn hover(params: HoverParams, db: &AnalysisDatabase) -> Option<Hover> {
let file_id = db.file_for_url(&params.text_document_position_params.text_document.uri)?;
let position = params.text_document_position_params.position.to_cairo();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use crate::lang::inspect::defs::{MemberDef, SymbolDef};
use crate::lang::lsp::ToLsp;

/// Get declaration and documentation "definition" of an item referred by the given identifier.
#[tracing::instrument(level = "trace", skip_all)]
pub fn definition(
db: &AnalysisDatabase,
identifier: &TerminalIdentifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ use crate::lang::inspect::defs::find_definition;
use crate::lang::lsp::{LsProtoGroup, ToCairo, ToLsp};

/// Get the definition location of a symbol at a given text document position.
#[tracing::instrument(
level = "debug",
skip_all,
fields(uri = %params.text_document_position_params.text_document.uri)
)]
pub fn goto_definition(
params: GotoDefinitionParams,
db: &AnalysisDatabase,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ mod encoder;
mod token_kind;

/// Resolve the semantic tokens of a given file.
#[tracing::instrument(
level = "debug",
skip_all,
fields(uri = %params.text_document.uri)
)]
pub fn semantic_highlight_full(
params: SemanticTokensParams,
db: &AnalysisDatabase,
Expand Down
1 change: 0 additions & 1 deletion crates/cairo-lang-language-server/src/ide/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use tracing::debug;
use crate::lang::db::AnalysisDatabase;

/// Finds all methods that can be called on a type.
#[tracing::instrument(level = "trace", skip_all)]
pub fn find_methods_for_type(
db: &AnalysisDatabase,
mut resolver: Resolver<'_>,
Expand Down
1 change: 0 additions & 1 deletion crates/cairo-lang-language-server/src/lang/db/semantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ impl<T> LsSemanticGroup for T where T: Upcast<dyn SemanticGroup> + ?Sized {}

/// If the ast node is a lookup item, return corresponding ids. Otherwise, returns `None`.
/// See [LookupItemId].
#[tracing::instrument(level = "trace", skip_all)]
fn lookup_item_from_ast(
db: &dyn SemanticGroup,
module_file_id: ModuleFileId,
Expand Down
8 changes: 6 additions & 2 deletions crates/cairo-lang-language-server/src/lang/db/swapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ impl AnalysisDatabaseSwapper {
}

/// Checks if enough time has passed since last db swap, and if so, swaps the database.
#[tracing::instrument(level = "trace", skip_all)]
pub fn maybe_swap(
&mut self,
db: &mut AnalysisDatabase,
Expand All @@ -65,6 +64,12 @@ impl AnalysisDatabaseSwapper {
return;
}

self.swap(db, open_files, tricks)
}

/// Swaps the database.
#[tracing::instrument(skip_all)]
fn swap(&mut self, db: &mut AnalysisDatabase, open_files: &HashSet<Url>, tricks: &Tricks) {
let Ok(new_db) = catch_unwind(AssertUnwindSafe(|| {
let mut new_db = AnalysisDatabase::new(tricks);
ensure_exists_in_db(&mut new_db, db, open_files.iter());
Expand All @@ -81,7 +86,6 @@ impl AnalysisDatabaseSwapper {
}

/// Makes sure that all open files exist in the new db, with their current changes.
#[tracing::instrument(level = "trace", skip_all)]
fn ensure_exists_in_db<'a>(
new_db: &mut AnalysisDatabase,
old_db: &AnalysisDatabase,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use tracing::error;
use crate::lang::lsp::{LsProtoGroup, ToLsp};

/// Converts internal diagnostics to LSP format.
#[tracing::instrument(level = "trace", skip_all)]
pub fn map_cairo_diagnostics_to_lsp<T: DiagnosticEntry>(
db: &T::DbType,
diags: &mut Vec<Diagnostic>,
Expand Down
39 changes: 18 additions & 21 deletions crates/cairo-lang-language-server/src/lang/diagnostics/refresh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use cairo_lang_semantic::db::SemanticGroup;
use cairo_lang_utils::Upcast;
use lsp_types::notification::PublishDiagnostics;
use lsp_types::{PublishDiagnosticsParams, Url};
use tracing::{error, trace_span};
use tracing::{error, info_span};

use crate::lang::db::AnalysisDatabase;
use crate::lang::diagnostics::lsp::map_cairo_diagnostics_to_lsp;
Expand All @@ -24,7 +24,7 @@ use crate::server::panic::is_cancelled;
use crate::state::FileDiagnostics;

/// Refresh diagnostics and send diffs to the client.
#[tracing::instrument(level = "debug", skip_all)]
#[tracing::instrument(skip_all)]
pub fn refresh_diagnostics(
db: &AnalysisDatabase,
open_files: &HashSet<Url>,
Expand All @@ -35,14 +35,14 @@ pub fn refresh_diagnostics(
let mut files_with_set_diagnostics: HashSet<Url> = HashSet::default();
let mut processed_modules: HashSet<ModuleId> = HashSet::default();

let open_files_ids = trace_span!("get_open_files_ids").in_scope(|| {
let open_files_ids = info_span!("get_open_files_ids").in_scope(|| {
open_files.iter().filter_map(|uri| db.file_for_url(uri)).collect::<HashSet<FileId>>()
});

let open_files_modules = get_files_modules(db, open_files_ids.iter().copied());

// Refresh open files modules first for better UX
trace_span!("refresh_open_files_modules").in_scope(|| {
info_span!("refresh_open_files_modules").in_scope(|| {
for (file, file_modules_ids) in open_files_modules {
refresh_file_diagnostics(
db,
Expand All @@ -57,7 +57,7 @@ pub fn refresh_diagnostics(
}
});

let rest_of_files = trace_span!("get_rest_of_files").in_scope(|| {
let rest_of_files = info_span!("get_rest_of_files").in_scope(|| {
let mut rest_of_files: HashSet<FileId> = HashSet::default();
for crate_id in db.crates() {
for module_id in db.crate_modules(crate_id).iter() {
Expand All @@ -74,7 +74,7 @@ pub fn refresh_diagnostics(
let rest_of_files_modules = get_files_modules(db, rest_of_files.iter().copied());

// Refresh the rest of files after, since they are not viewed currently
trace_span!("refresh_other_files_modules").in_scope(|| {
info_span!("refresh_other_files_modules").in_scope(|| {
for (file, file_modules_ids) in rest_of_files_modules {
refresh_file_diagnostics(
db,
Expand All @@ -90,7 +90,7 @@ pub fn refresh_diagnostics(
});

// Clear old diagnostics
trace_span!("clear_old_diagnostics").in_scope(|| {
info_span!("clear_old_diagnostics").in_scope(|| {
let mut removed_files = Vec::new();

file_diagnostics.retain(|uri, _| {
Expand All @@ -102,12 +102,10 @@ pub fn refresh_diagnostics(
});

for file in removed_files {
trace_span!("publish_diagnostics").in_scope(|| {
notifier.notify::<PublishDiagnostics>(PublishDiagnosticsParams {
uri: file,
diagnostics: vec![],
version: None,
});
notifier.notify::<PublishDiagnostics>(PublishDiagnosticsParams {
uri: file,
diagnostics: vec![],
version: None,
});
}
});
Expand All @@ -131,7 +129,7 @@ fn refresh_file_diagnostics(

macro_rules! diags {
($db:ident. $query:ident($file_id:expr), $f:expr) => {
trace_span!(stringify!($query)).in_scope(|| {
info_span!(stringify!($query)).in_scope(|| {
catch_unwind(AssertUnwindSafe(|| $db.$query($file_id)))
.map($f)
.map_err(|err| {
Expand Down Expand Up @@ -206,16 +204,15 @@ fn refresh_file_diagnostics(
trace_macro_diagnostics,
);

trace_span!("publish_diagnostics").in_scope(|| {
notifier.notify::<PublishDiagnostics>(PublishDiagnosticsParams {
uri: file_uri,
diagnostics: diags,
version: None,
});
})
notifier.notify::<PublishDiagnostics>(PublishDiagnosticsParams {
uri: file_uri,
diagnostics: diags,
version: None,
});
}

/// Gets the mapping of files to their respective modules.
#[tracing::instrument(skip_all)]
fn get_files_modules(
db: &AnalysisDatabase,
files_ids: impl Iterator<Item = FileId>,
Expand Down
4 changes: 0 additions & 4 deletions crates/cairo-lang-language-server/src/lang/inspect/defs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ pub enum ResolvedItem {

impl SymbolDef {
/// Finds definition of the symbol referred by the given identifier.
#[tracing::instrument(name = "SymbolDef::find", level = "trace", skip_all)]
pub fn find(db: &AnalysisDatabase, identifier: &TerminalIdentifier) -> Option<Self> {
if let Some(parent) = identifier.as_syntax_node().parent() {
if parent.kind(db.upcast()) == SyntaxKind::PathSegmentSimple
Expand Down Expand Up @@ -292,7 +291,6 @@ impl VariableDef {
}

// TODO(mkaput): make private.
#[tracing::instrument(level = "trace", skip_all)]
pub fn find_definition(
db: &AnalysisDatabase,
identifier: &ast::TerminalIdentifier,
Expand Down Expand Up @@ -403,7 +401,6 @@ fn try_extract_member(
}
}

#[tracing::instrument(level = "trace", skip_all)]
fn resolved_concrete_item_def(
db: &AnalysisDatabase,
item: ResolvedConcreteItem,
Expand All @@ -427,7 +424,6 @@ fn resolved_concrete_item_def(
}
}

#[tracing::instrument(level = "trace", skip_all)]
fn resolved_generic_item_def(
db: &AnalysisDatabase,
item: ResolvedGenericItem,
Expand Down
3 changes: 1 addition & 2 deletions crates/cairo-lang-language-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ impl Backend {

/// Tries to detect the crate root the config that contains a cairo file, and add it to the
/// system.
#[tracing::instrument(level = "trace", skip_all)]
#[tracing::instrument(skip_all)]
fn detect_crate_for(
db: &mut AnalysisDatabase,
scarb_toolchain: &ScarbToolchain,
Expand Down Expand Up @@ -426,7 +426,6 @@ impl Backend {
}

/// Reload crate detection for all open files.
#[tracing::instrument(level = "trace", skip_all)]
fn reload(
state: &mut State,
notifier: &Notifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn find_core_at_path(root_path: &Path) -> Option<PathBuf> {
///
/// Because CairoLS is tightly bound to Scarb (due to hard compiler version dependency),
/// we can safely make this lookup once and keep it cached for the entire LS lifetime.
#[tracing::instrument(level = "trace", skip_all)]
#[tracing::instrument(skip_all)]
fn find_scarb_managed_core(scarb: &ScarbToolchain) -> Option<PathBuf> {
let lookup = || {
let workspace = tempdir()
Expand Down
Loading

0 comments on commit 02fca5c

Please sign in to comment.