Skip to content

Commit

Permalink
Merge #9743
Browse files Browse the repository at this point in the history
9743: internal: a bit of completion profiling r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>
  • Loading branch information
bors[bot] and matklad authored Jul 31, 2021
2 parents 3236845 + a5049e1 commit 6b733ea
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 8 deletions.
18 changes: 16 additions & 2 deletions crates/ide_completion/src/render/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use hir::{AsAssocItem, HasSource, HirDisplay};
use ide_db::SymbolKind;
use itertools::Itertools;
use syntax::ast::Fn;
use syntax::ast;

use crate::{
item::{CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance, ImportEdit},
Expand Down Expand Up @@ -40,7 +40,21 @@ struct FunctionRender<'a> {
name: String,
receiver: Option<hir::Name>,
func: hir::Function,
ast_node: Fn,
/// NB: having `ast::Fn` here might or might not be a good idea. The problem
/// with it is that, to get an `ast::`, you want to parse the corresponding
/// source file. So, when flyimport completions suggest a bunch of
/// functions, we spend quite some time parsing many files.
///
/// We need ast because we want to access parameter names (patterns). We can
/// add them to the hir of the function itself, but parameter names are not
/// something hir cares otherwise.
///
/// Alternatively we can reconstruct params from the function body, but that
/// would require parsing anyway.
///
/// It seems that just using `ast` is the best choice -- most of parses
/// should be cached anyway.
ast_node: ast::Fn,
is_method: bool,
}

Expand Down
23 changes: 17 additions & 6 deletions crates/profile/src/hprof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use std::{
cell::RefCell,
collections::{BTreeMap, HashSet},
env,
env, fmt,
io::{stderr, Write},
sync::{
atomic::{AtomicBool, Ordering},
Expand Down Expand Up @@ -278,9 +278,9 @@ fn print(
let detail = tree[curr].detail.as_ref().map(|it| format!(" @ {}", it)).unwrap_or_default();
writeln!(
out,
"{}{:5}ms - {}{}",
"{}{} - {}{}",
current_indent,
tree[curr].duration.as_millis(),
ms(tree[curr].duration),
tree[curr].label,
detail,
)
Expand All @@ -302,14 +302,25 @@ fn print(
}

for (child_msg, (duration, count)) in short_children.iter() {
let millis = duration.as_millis();
writeln!(out, " {}{:5}ms - {} ({} calls)", current_indent, millis, child_msg, count)
writeln!(out, " {}{} - {} ({} calls)", current_indent, ms(*duration), child_msg, count)
.expect("printing profiling info");
}

let unaccounted = tree[curr].duration - accounted_for;
if tree.children(curr).next().is_some() && unaccounted > longer_than {
writeln!(out, " {}{:5}ms - ???", current_indent, unaccounted.as_millis())
writeln!(out, " {}{} - ???", current_indent, ms(unaccounted))
.expect("printing profiling info");
}
}

#[allow(non_camel_case_types)]
struct ms(Duration);

impl fmt::Display for ms {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.0.as_millis() {
0 => f.write_str(" 0 "),
n => write!(f, "{:5}ms", n),
}
}
}

0 comments on commit 6b733ea

Please sign in to comment.