Merge #9743

9743: internal: a bit of completion profiling r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

Author: bors[bot]

crates/ide_completion/src/render/function.rs

@@ -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},
@@ -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,
 }
 

crates/profile/src/hprof.rs

@@ -2,7 +2,7 @@
 use std::{
     cell::RefCell,
     collections::{BTreeMap, HashSet},
-    env,
+    env, fmt,
     io::{stderr, Write},
     sync::{
         atomic::{AtomicBool, Ordering},
@@ -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,
     )
@@ -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),
+        }
+    }
+}