perf: instrument more code with tracing and optimize it (#18)

benches/src/lib.rs

@@ -93,7 +93,7 @@ impl Parser for Sulk {
             let sess = Session::with_tty_emitter(source_map.into());
             let filename = PathBuf::from("test.sol");
             let mut parser =
-                sulk_parse::Parser::from_source_code(&sess, filename.into(), src.into())?;
+                sulk_parse::Parser::from_source_code(&sess, filename.into(), || Ok(src.into()))?;
             let result = parser.parse_file().map_err(|e| e.emit())?;
             sess.dcx.has_errors()?;
             black_box(result);

crates/interface/src/lib.rs

@@ -52,31 +52,3 @@ pub type Result<T = (), E = ErrorGuaranteed> = std::result::Result<T, E>;
 pub fn enter<R>(f: impl FnOnce() -> R) -> R {
     SessionGlobals::with_or_default(|_| f())
 }
-
-#[macro_export]
-macro_rules! time {
-    ($level:expr, $what:literal, || $e:expr) => {
-        if enabled!($level) {
-            let timer = std::time::Instant::now();
-            let res = $e;
-            event!($level, elapsed=?timer.elapsed(), $what);
-            res
-        } else {
-            $e
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! debug_time {
-    ($what:literal, || $e:expr) => {
-        $crate::time!(tracing::Level::DEBUG, $what, || $e)
-    };
-}
-
-#[macro_export]
-macro_rules! trace_time {
-    ($what:literal, || $e:expr) => {
-        $crate::time!(tracing::Level::TRACE, $what, || $e)
-    };
-}

crates/interface/src/source_map/file.rs

@@ -221,7 +221,7 @@ pub struct SourceFile {
 impl SourceFile {
     pub fn new(
         name: FileName,
-        src: String,
+        mut src: String,
         hash_kind: SourceFileHashAlgorithm,
     ) -> Result<Self, OffsetOverflowError> {
         // Compute the file hash before any normalization.
@@ -234,6 +234,7 @@ impl SourceFile {
 
         let (lines, multibyte_chars, non_narrow_chars) = super::analyze::analyze_source_file(&src);
 
+        src.shrink_to_fit();
         Ok(Self {
             name,
             src: Lrc::new(src),

crates/interface/src/source_map/file_resolver.rs

@@ -78,6 +78,7 @@ impl<'a> FileResolver<'a> {
     }
 
     /// Resolves an import path. `parent` is the path of the file that contains the import, if any.
+    #[instrument(level = "debug", skip_all, fields(path = %path.display()))]
     pub fn resolve_file(
         &self,
         path: &Path,
@@ -140,7 +141,7 @@ impl<'a> FileResolver<'a> {
     }
 
     /// Applies the import path mappings to `path`.
-    #[instrument(level = "trace", skip(self), ret)]
+    #[instrument(level = "trace", skip_all, ret)]
     pub fn remap_path<'b>(&self, path: &'b Path) -> Cow<'b, Path> {
         let orig = path;
         let mut remapped = Cow::Borrowed(path);
@@ -160,7 +161,7 @@ impl<'a> FileResolver<'a> {
     }
 
     /// Loads `path` into the source map. Returns `None` if the file doesn't exist.
-    #[instrument(level = "trace", skip(self))]
+    #[instrument(level = "debug", skip_all)]
     pub fn try_file(&self, path: &Path) -> Result<Option<Lrc<SourceFile>>, ResolveError> {
         let cache_path = path.normalize();
         if let Ok(file) = self.source_map().load_file(&cache_path) {

crates/interface/src/source_map/mod.rs

@@ -3,7 +3,7 @@
 use crate::{BytePos, CharPos, Pos, Span};
 use std::{
     io::{self, Read},
-    path::Path,
+    path::{Path, PathBuf},
 };
 use sulk_data_structures::{
     map::FxHashMap,
@@ -124,16 +124,52 @@ impl SourceMap {
 
     /// Loads a file from the given path.
     pub fn load_file(&self, path: &Path) -> io::Result<Lrc<SourceFile>> {
-        let src = std::fs::read_to_string(path)?;
         let filename = path.to_owned().into();
-        self.new_source_file(filename, src).map_err(Into::into)
+        self.new_source_file(filename, || std::fs::read_to_string(path))
     }
 
     /// Loads `stdin`.
     pub fn load_stdin(&self) -> io::Result<Lrc<SourceFile>> {
-        let mut src = String::new();
-        io::stdin().read_to_string(&mut src)?;
-        self.new_source_file(FileName::Stdin, src).map_err(Into::into)
+        self.new_source_file(FileName::Stdin, || {
+            let mut src = String::new();
+            io::stdin().read_to_string(&mut src)?;
+            Ok(src)
+        })
+    }
+
+    /// Loads a file with the given source string.
+    ///
+    /// This is useful for testing.
+    pub fn new_dummy_source_file(&self, path: PathBuf, src: String) -> io::Result<Lrc<SourceFile>> {
+        self.new_source_file(path.into(), || Ok(src))
+    }
+
+    /// Creates a new `SourceFile`.
+    ///
+    /// If a file already exists in the `SourceMap` with the same ID, that file is returned
+    /// unmodified.
+    ///
+    /// Returns an error if the file is larger than 4GiB or other errors occur while creating the
+    /// `SourceFile`.
+    #[instrument(level = "debug", skip_all, fields(filename = %filename.display()))]
+    pub fn new_source_file(
+        &self,
+        filename: FileName,
+        get_src: impl FnOnce() -> io::Result<String>,
+    ) -> io::Result<Lrc<SourceFile>> {
+        let stable_id = StableSourceFileId::from_filename_in_current_crate(&filename);
+        match self.source_file_by_stable_id(stable_id) {
+            Some(lrc_sf) => Ok(lrc_sf),
+            None => {
+                let source_file = SourceFile::new(filename, get_src()?, self.hash_kind)?;
+
+                // Let's make sure the file_id we generated above actually matches
+                // the ID we generate for the SourceFile we just created.
+                debug_assert_eq!(source_file.stable_id, stable_id);
+
+                self.register_source_file(stable_id, source_file).map_err(Into::into)
+            }
+        }
     }
 
     pub fn files(&self) -> MappedReadGuard<'_, Vec<Lrc<SourceFile>>> {
@@ -174,33 +210,6 @@ impl SourceMap {
         Ok(file)
     }
 
-    /// Creates a new `SourceFile`.
-    ///
-    /// If a file already exists in the `SourceMap` with the same ID, that file is returned
-    /// unmodified.
-    ///
-    /// Returns an error if the file is larger than 4GiB or other errors occur while creating the
-    /// `SourceFile`.
-    pub fn new_source_file(
-        &self,
-        filename: FileName,
-        src: String,
-    ) -> Result<Lrc<SourceFile>, OffsetOverflowError> {
-        let stable_id = StableSourceFileId::from_filename_in_current_crate(&filename);
-        match self.source_file_by_stable_id(stable_id) {
-            Some(lrc_sf) => Ok(lrc_sf),
-            None => {
-                let source_file = SourceFile::new(filename, src, self.hash_kind)?;
-
-                // Let's make sure the file_id we generated above actually matches
-                // the ID we generate for the SourceFile we just created.
-                debug_assert_eq!(source_file.stable_id, stable_id);
-
-                self.register_source_file(stable_id, source_file)
-            }
-        }
-    }
-
     pub fn filename_for_diagnostics<'a>(&self, filename: &'a FileName) -> FileNameDisplay<'a> {
         filename.display()
     }

crates/interface/src/source_map/tests.rs

@@ -3,10 +3,10 @@ use std::path::PathBuf;
 
 fn init_source_map() -> SourceMap {
     let sm = SourceMap::empty();
-    sm.new_source_file(PathBuf::from("blork.rs").into(), "first line.\nsecond line".to_string())
+    sm.new_dummy_source_file(PathBuf::from("blork.rs"), "first line.\nsecond line".to_string())
         .unwrap();
-    sm.new_source_file(PathBuf::from("empty.rs").into(), String::new()).unwrap();
-    sm.new_source_file(PathBuf::from("blork2.rs").into(), "first line.\nsecond line".to_string())
+    sm.new_dummy_source_file(PathBuf::from("empty.rs"), String::new()).unwrap();
+    sm.new_dummy_source_file(PathBuf::from("blork2.rs"), "first line.\nsecond line".to_string())
         .unwrap();
     sm
 }
@@ -105,13 +105,13 @@ fn t5() {
 fn init_source_map_mbc() -> SourceMap {
     let sm = SourceMap::empty();
     // "€" is a three-byte UTF8 char.
-    sm.new_source_file(
-        PathBuf::from("blork.rs").into(),
+    sm.new_dummy_source_file(
+        PathBuf::from("blork.rs"),
         "fir€st €€€€ line.\nsecond line".to_string(),
     )
     .unwrap();
-    sm.new_source_file(
-        PathBuf::from("blork2.rs").into(),
+    sm.new_dummy_source_file(
+        PathBuf::from("blork2.rs"),
         "first line€€.\n€ second line".to_string(),
     )
     .unwrap();
@@ -166,7 +166,7 @@ fn span_to_snippet_and_lines_spanning_multiple_lines() {
     let sm = SourceMap::empty();
     let inputtext = "aaaaa\nbbbbBB\nCCC\nDDDDDddddd\neee\n";
     let selection = "     \n    ~~\n~~~\n~~~~~     \n   \n";
-    sm.new_source_file(Path::new("blork.rs").to_owned().into(), inputtext.to_string()).unwrap();
+    sm.new_dummy_source_file(Path::new("blork.rs").to_owned(), inputtext.to_string()).unwrap();
     let span = span_from_selection(inputtext, selection);
 
     // Check that we are extracting the text we thought we were extracting.
@@ -209,7 +209,7 @@ fn span_merging_fail() {
     let inputtext = "bbbb BB\ncc CCC\n";
     let selection1 = "     ~~\n      \n";
     let selection2 = "       \n   ~~~\n";
-    sm.new_source_file(Path::new("blork.rs").to_owned().into(), inputtext.to_owned()).unwrap();
+    sm.new_dummy_source_file(Path::new("blork.rs").to_owned(), inputtext.to_owned()).unwrap();
     let span1 = span_from_selection(inputtext, selection1);
     let span2 = span_from_selection(inputtext, selection2);
 
@@ -224,7 +224,8 @@ fn t10() {
     let unnormalized = "first line.\r\nsecond line";
     let normalized = "first line.\nsecond line";
 
-    let src_file = sm.new_source_file(PathBuf::from("blork.rs").into(), unnormalized.to_string());
+    let src_file =
+        sm.new_dummy_source_file(PathBuf::from("blork.rs").into(), unnormalized.to_string());
 
     assert_eq!(src_file.src.as_ref().unwrap().as_ref(), normalized);
     assert!(
@@ -554,7 +555,7 @@ fn path_prefix_remapping_reverse() {
 #[test]
 fn test_next_point() {
     let sm = SourceMap::empty();
-    sm.new_source_file(PathBuf::from("example.rs").into(), "a…b".to_string());
+    sm.new_dummy_source_file(PathBuf::from("example.rs").into(), "a…b".to_string());
 
     // Dummy spans don't advance.
     let span = DUMMY_SP;

crates/parse/src/lexer/mod.rs

@@ -85,7 +85,9 @@ impl<'sess, 'src> Lexer<'sess, 'src> {
     /// Note that this skips comments, as [required by the parser](crate::Parser::new).
     ///
     /// Prefer using this method instead of manually collecting tokens using [`Iterator`].
+    #[instrument(name = "lex", level = "debug", skip_all)]
     pub fn into_tokens(mut self) -> Vec<Token> {
+        // `src.len() / 8` is an estimate of the number of tokens in the source.
         let mut tokens = Vec::with_capacity(self.src.len() / 8);
         loop {
             let token = self.next_token();
@@ -387,11 +389,11 @@ impl<'sess, 'src> Lexer<'sess, 'src> {
         let content_end = end - 1;
         let lit_content = self.str_from_to(content_start, content_end);
 
-        let mut has_fatal_err = false;
+        let mut has_err = false;
         unescape::unescape_literal(lit_content, mode, |range, result| {
             // Here we only check for errors. The actual unescaping is done later.
             if let Err(err) = result {
-                has_fatal_err = true;
+                has_err = true;
                 let (start, end) = (range.start as u32, range.end as u32);
                 let lo = content_start + BytePos(start);
                 let hi = lo + BytePos(end - start);
@@ -402,11 +404,9 @@ impl<'sess, 'src> Lexer<'sess, 'src> {
 
         // We normally exclude the quotes for the symbol, but for errors we
         // include it because it results in clearer error messages.
-        if has_fatal_err {
-            (TokenLitKind::Err, self.symbol_from_to(start, end))
-        } else {
-            (kind, Symbol::intern(lit_content))
-        }
+        let symbol =
+            if has_err { self.symbol_from_to(start, end) } else { Symbol::intern(lit_content) };
+        (kind, symbol)
     }
 
     #[inline]

crates/parse/src/lexer/unescape/mod.rs

@@ -1,6 +1,6 @@
 //! Utilities for validating string and char literals and turning them into values they represent.
 
-use std::{ops::Range, slice, str::Chars};
+use std::{borrow::Cow, ops::Range, slice, str::Chars};
 
 mod errors;
 pub(crate) use errors::emit_unescape_error;
@@ -18,33 +18,43 @@ pub enum Mode {
 }
 
 /// Parses a string literal (without quotes) into a byte array.
-pub fn parse_literal<F>(src: &str, mode: Mode, f: F) -> Vec<u8>
+#[instrument(level = "debug", skip_all)]
+pub fn parse_string_literal<F>(src: &str, mode: Mode, f: F) -> Vec<u8>
 where
     F: FnMut(Range<usize>, EscapeError),
 {
     let mut bytes = if needs_unescape(src, mode) {
-        let mut bytes = Vec::with_capacity(src.len());
-        parse_literal_unescape(src, mode, f, &mut bytes);
-        bytes
+        Cow::Owned(parse_literal_unescape(src, mode, f))
     } else {
-        src.as_bytes().to_vec()
+        Cow::Borrowed(src.as_bytes())
     };
     if mode == Mode::HexStr {
         // Currently this should never fail, but it's a good idea to check anyway.
         if let Ok(decoded) = hex::decode(&bytes) {
-            bytes = decoded;
+            bytes = Cow::Owned(decoded);
         }
     }
+    bytes.into_owned()
+}
+
+#[cold]
+fn parse_literal_unescape<F>(src: &str, mode: Mode, f: F) -> Vec<u8>
+where
+    F: FnMut(Range<usize>, EscapeError),
+{
+    let mut bytes = Vec::with_capacity(src.len());
+    parse_literal_unescape_into(src, mode, f, &mut bytes);
     bytes
 }
 
-#[inline]
-fn parse_literal_unescape<F>(src: &str, mode: Mode, mut f: F, dst_buf: &mut Vec<u8>)
+fn parse_literal_unescape_into<F>(src: &str, mode: Mode, mut f: F, dst_buf: &mut Vec<u8>)
 where
     F: FnMut(Range<usize>, EscapeError),
 {
     // `src.len()` is enough capacity for the unescaped string, so we can just use a slice.
     // SAFETY: The buffer is never read from.
+    debug_assert!(dst_buf.is_empty());
+    debug_assert!(dst_buf.capacity() >= src.len());
     let mut dst = unsafe { slice::from_raw_parts_mut(dst_buf.as_mut_ptr(), dst_buf.capacity()) };
     unescape_literal(src, mode, |range, res| match res {
         Ok(c) => {
@@ -67,6 +77,7 @@ where
 /// Unescapes the contents of a string literal (without quotes).
 ///
 /// The callback is invoked with a range and either a unicode code point or an error.
+#[instrument(level = "debug", skip_all)]
 pub fn unescape_literal<F>(src: &str, mode: Mode, mut callback: F)
 where
     F: FnMut(Range<usize>, Result<u32, EscapeError>),
@@ -297,7 +308,7 @@ mod tests {
         assert_eq!(ok, expected_str, "{panic_str}");
 
         let mut errs2 = Vec::with_capacity(errs.len());
-        let out = parse_literal(src, mode, |range, e| {
+        let out = parse_string_literal(src, mode, |range, e| {
             errs2.push((range, e));
         });
         assert_eq!(errs2, errs, "{panic_str}");

crates/parse/src/parser/expr.rs

@@ -9,6 +9,7 @@ impl<'a> Parser<'a> {
         self.parse_expr_with(None)
     }
 
+    #[instrument(name = "parse_expr", level = "debug", skip_all)]
     pub(super) fn parse_expr_with(&mut self, with: Option<Box<Expr>>) -> PResult<'a, Box<Expr>> {
         let expr = self.parse_binary_expr(4, with)?;
         if self.eat(&TokenKind::Question) {