Merge pull request #1 from tazjin/performance-improvements

Significant performance improvements for large projects:
  - removed usage of tokio for now (not too much concurency since canonical path building seems to take the bulk of the time and that operation is not async)
  - made canonical path resolution be cached as it is expensive

Author: andy31415

Cargo.lock

@@ -14,7 +14,6 @@ path="src/lib.rs"
 crate-type = ["lib"]
 
 [dependencies]
-tokio = { version = "1", features = ["full"]}
 thiserror = "1"
 tracing = { version = "0.1"}
 shlex = { version = "1.3.0"}
@@ -38,3 +37,10 @@ camino = "1.1.6"
 tera = "1.19.1"
 eyre = "0.6.12"
 color-eyre = "0.6.2"
+
+# Add a profile to all targets that enables release optimisations, but
+# retains debug symbols. This is great for use with
+# benchmarking/profiling tools.
+[profile.release-with-debug]
+inherits = "release"
+debug = true

Cargo.toml

@@ -1,3 +1,4 @@
+pub mod canonicalize;
 pub mod compiledb;
 pub mod configfile;
 pub mod cparse;

src/dependencies.rs

@@ -0,0 +1,42 @@
+use std::borrow::Borrow;
+use std::collections::HashMap;
+use std::fs::canonicalize;
+use std::hash::Hash;
+use std::path::{Path, PathBuf};
+use std::sync::{Arc, LazyLock, RwLock};
+
+static PATH_CACHE: LazyLock<Arc<RwLock<HashMap<PathBuf, Option<PathBuf>>>>> =
+    LazyLock::new(|| Default::default());
+
+/// Wrapper around [`std::fs::canonicalize`] that caches already canonicalized
+/// entries globally (often the case when dealing with includes).
+///
+/// This is done due to the operation being inherently slow: It has to walk the
+/// entire parent directory tree (especially expensive on FUSE-mounted virtual
+/// repository filesystems).
+pub fn canonicalize_cached<P>(path: P) -> Result<Option<PathBuf>, std::io::Error>
+where
+    P: AsRef<Path>,
+    PathBuf: Borrow<P>,
+    P: Hash + Eq,
+{
+    {
+        // First, try the cache ...
+        let cache = PATH_CACHE.read().unwrap();
+        if let Some(cached) = cache.get(&path) {
+            return Ok(cached.clone());
+        }
+    }
+
+    // ... then look it up ourselves.
+    let result = if path.as_ref().exists() {
+        Some(canonicalize(&path)?)
+    } else {
+        None
+    };
+
+    let mut cache = PATH_CACHE.write().unwrap();
+    cache.insert(path.as_ref().to_path_buf(), result.clone());
+
+    Ok(result)
+}

src/dependencies/canonicalize.rs

@@ -1,10 +1,11 @@
 use serde::{Deserialize, Serialize};
-use tokio::{fs::File, io::AsyncReadExt as _};
-use tracing::debug;
-
+use std::fs::File;
+use std::io::Read;
 use std::path::PathBuf;
+use tracing::debug;
 
-use crate::dependencies::error::Error;
+use super::canonicalize::canonicalize_cached;
+use super::error::Error;
 
 #[derive(Debug, PartialEq, PartialOrd, Hash, Serialize, Deserialize)]
 pub struct SourceFileEntry {
@@ -45,11 +46,12 @@ impl TryFrom<CompileCommandsEntry> for SourceFileEntry {
             source_file
         };
 
-        let file_path = file_path.canonicalize().map_err(|source| Error::IOError {
-            source,
-            path: file_path.clone(),
-            message: "canonicalize",
-        })?;
+        let file_path = canonicalize_cached(file_path)
+            .map_err(|source| Error::IOError {
+                source,
+                message: "canonicalize",
+            })?
+            .ok_or(Error::FileNotFound)?;
 
         let args = value
             .arguments
@@ -61,7 +63,7 @@ impl TryFrom<CompileCommandsEntry> for SourceFileEntry {
             .map(PathBuf::from)
             .filter_map(|p| {
                 if p.is_relative() {
-                    start_dir.join(p).canonicalize().ok()
+                    canonicalize_cached(start_dir.join(p)).ok()?
                 } else {
                     Some(p)
                 }
@@ -75,17 +77,16 @@ impl TryFrom<CompileCommandsEntry> for SourceFileEntry {
     }
 }
 
-pub async fn parse_compile_database(path: &str) -> Result<Vec<SourceFileEntry>, Error> {
-    let mut file = File::open(path).await.map_err(|source| Error::IOError {
+pub fn parse_compile_database(path: &str) -> Result<Vec<SourceFileEntry>, Error> {
+    let mut file = File::open(path).map_err(|source| Error::FileIOError {
         source,
         path: path.into(),
         message: "open",
     })?;
     let mut json_string = String::new();
 
     file.read_to_string(&mut json_string)
-        .await
-        .map_err(|source| Error::IOError {
+        .map_err(|source| Error::FileIOError {
             source,
             path: path.into(),
             message: "read_to_string",

src/dependencies/compiledb.rs

@@ -127,6 +127,9 @@ struct ConfigurationFile {
     /// What inputs are to be processed
     input_commands: Vec<InputCommand>,
 
+    /// Should symlinks be resolved, or left alone? Enabling symlink resolution
+    /// can be significantly slower on large code bases.
+
     /// Instructions to build a braph
     graph: GraphInstructions,
 }
@@ -756,7 +759,7 @@ fn parse_config(input: &str) -> IResult<&str, ConfigurationFile> {
         .parse(input)
 }
 
-pub async fn build_graph(input: &str) -> Result<Graph, Report> {
+pub fn build_graph(input: &str) -> Result<Graph, Report> {
     let (input, config) = parse_config(input)
         .map_err(|e| Error::ConfigParseError {
             message: format!("Nom error: {:?}", e),
@@ -783,7 +786,7 @@ pub async fn build_graph(input: &str) -> Result<Graph, Report> {
                 load_include_directories,
                 load_sources,
             } => {
-                let entries = match parse_compile_database(&path).await {
+                let entries = match parse_compile_database(&path) {
                     Ok(entries) => entries,
                     Err(err) => {
                         error!("Error parsing compile database {}: {:?}", path, err);
@@ -817,7 +820,7 @@ pub async fn build_graph(input: &str) -> Result<Graph, Report> {
                         .into_iter()
                         .collect::<Vec<_>>();
                     for entry in entries {
-                        match extract_includes(&entry.file_path, &includes_array).await {
+                        match extract_includes(&entry.file_path, &includes_array) {
                             Ok(includes) => {
                                 info!(target: "compile-db", "Loaded {:?} with includes {:#?}", &entry.file_path, includes);
                                 dependency_data.files.push(SourceWithIncludes {
@@ -851,7 +854,7 @@ pub async fn build_graph(input: &str) -> Result<Graph, Report> {
                     .clone()
                     .into_iter()
                     .collect::<Vec<_>>();
-                match all_sources_and_includes(glob, &includes_array).await {
+                match all_sources_and_includes(glob, &includes_array) {
                     Ok(data) => {
                         if data.is_empty() {
                             error!("GLOB {:?} resulted in EMPTY file list!", g);
@@ -926,12 +929,7 @@ pub async fn build_graph(input: &str) -> Result<Graph, Report> {
                 target,
                 source_root,
                 ignore_targets,
-            } => match load_gn_targets(
-                &PathBuf::from(gn_root),
-                &PathBuf::from(source_root),
-                &target,
-            )
-            .await
+            } => match load_gn_targets(PathBuf::from(gn_root), PathBuf::from(source_root), &target)
             {
                 Ok(targets) => g.add_groups_from_gn(targets, ignore_targets),
                 Err(e) => error!("Failed to load GN targets: {:?}", e),
@@ -1111,7 +1109,7 @@ mod tests {
         graph {
               map {
               }
-   
+
               group {
                 gn root test1 target //my/target/* sources srcs1
                 gn root test/${Foo}/blah target //* sources ${Foo} ignore targets {

src/dependencies/configfile.rs

@@ -1,22 +1,21 @@
+use super::canonicalize::canonicalize_cached;
 use super::error::Error;
 
 use regex::Regex;
 use std::{
     fmt::Debug,
-    path::{Path, PathBuf},
-    sync::Arc,
-};
-use tokio::{
     fs::File,
-    io::{AsyncBufReadExt as _, BufReader},
-    task::JoinSet,
+    io::{BufRead, BufReader},
+    path::{Path, PathBuf},
+    sync::{Arc, LazyLock},
+    thread,
 };
 use tracing::{error, info, trace};
 
 /// Attempt to make the full path of head::tail
 /// returns None if that fails (e.g. path does not exist)
-fn try_resolve(head: &Path, tail: &PathBuf) -> Option<PathBuf> {
-    head.join(tail).canonicalize().ok()
+fn try_resolve(head: &Path, tail: &Path) -> Option<PathBuf> {
+    canonicalize_cached(head.join(tail)).ok()?
 }
 
 #[derive(PartialEq, Eq, Hash, PartialOrd, Ord, Debug)]
@@ -41,41 +40,33 @@ impl FileType {
     }
 }
 
+static INCLUDE_REGEX: LazyLock<Regex> =
+    LazyLock::new(|| Regex::new(r##"^\s*#include\s*(["<])([^">]*)[">]"##).unwrap());
+
 /// Given a C-like source, try to resolve includes.
 ///
 /// Includes are generally of the form `#include <name>` or `#include "name"`
-pub async fn extract_includes(
-    path: &PathBuf,
-    include_dirs: &[PathBuf],
-) -> Result<Vec<PathBuf>, Error> {
-    let f = File::open(path).await.map_err(|source| Error::IOError {
+pub fn extract_includes(path: &PathBuf, include_dirs: &[PathBuf]) -> Result<Vec<PathBuf>, Error> {
+    let f = File::open(path).map_err(|source| Error::FileIOError {
         source,
         path: path.clone(),
         message: "open",
     })?;
 
     let reader = BufReader::new(f);
-
-    let inc_re = Regex::new(r##"^\s*#include\s*(["<])([^">]*)[">]"##).unwrap();
-
     let mut result = Vec::new();
     let parent_dir = PathBuf::from(path.parent().unwrap());
 
-    let mut lines = reader.lines();
+    let lines = reader.lines();
 
-    loop {
-        let line = lines.next_line().await.map_err(|source| Error::IOError {
+    for line in lines {
+        let line = line.map_err(|source| Error::FileIOError {
             source,
             path: path.clone(),
             message: "line read",
         })?;
 
-        let line = match line {
-            Some(value) => value,
-            None => break,
-        };
-
-        if let Some(captures) = inc_re.captures(&line) {
+        if let Some(captures) = INCLUDE_REGEX.captures(&line) {
             let inc_type = captures.get(1).unwrap().as_str();
             let relative_path = PathBuf::from(captures.get(2).unwrap().as_str());
 
@@ -115,7 +106,7 @@ pub struct SourceWithIncludes {
 }
 
 /// Given a list of paths, figure out their dependencies
-pub async fn all_sources_and_includes<I, E>(
+pub fn all_sources_and_includes<I, E>(
     paths: I,
     includes: &[PathBuf],
 ) -> Result<Vec<SourceWithIncludes>, Error>
@@ -124,14 +115,15 @@ where
     E: Debug,
 {
     let includes = Arc::new(Vec::from(includes));
-
-    let mut join_set = JoinSet::new();
+    let mut handles = Vec::new();
 
     for entry in paths {
         let path = match entry {
-            Ok(value) => value.canonicalize().map_err(|e| Error::Internal {
-                message: format!("{:?}", e),
-            })?,
+            Ok(value) => canonicalize_cached(value)
+                .map_err(|e| Error::Internal {
+                    message: format!("{:?}", e),
+                })?
+                .ok_or(Error::FileNotFound)?,
             Err(e) => {
                 return Err(Error::Internal {
                     message: format!("{:?}", e),
@@ -144,27 +136,27 @@ where
             continue;
         }
 
-        // prepare data to mve into sub-task
+        // prepare data to move into sub-task
         let includes = includes.clone();
 
-        join_set.spawn(async move {
+        handles.push(thread::spawn(move || {
             trace!("PROCESS: {:?}", path);
-            let includes = match extract_includes(&path, &includes).await {
+            let includes = match extract_includes(&path, &includes) {
                 Ok(value) => value,
                 Err(e) => {
-                    error!("Error extracing includes: {:?}", e);
+                    error!("Error extracting includes: {:?}", e);
                     return Err(e);
                 }
             };
 
             Ok(SourceWithIncludes { path, includes })
-        });
+        }));
     }
 
     let mut results = Vec::new();
-    while let Some(h) = join_set.join_next().await {
-        let r = h.map_err(Error::JoinError)?;
-        results.push(r?)
+    for handle in handles {
+        let res = handle.join().map_err(|_| Error::JoinError)?;
+        results.push(res?);
     }
 
     Ok(results)

src/dependencies/cparse.rs

@@ -1,21 +1,19 @@
 use std::path::PathBuf;
 
-use tokio::task::JoinError;
-
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
-    #[error("I/O error at path {}: {}", path.to_string_lossy(), message)]
+    #[error("I/O error: {}", message)]
     IOError {
         #[source]
         source: std::io::Error,
-        path: PathBuf,
         message: &'static str,
     },
 
-    #[error("I/O error: {}", message)]
-    AsyncIOError {
+    #[error("I/O error at path {}: {}", path.to_string_lossy(), message)]
+    FileIOError {
         #[source]
-        source: tokio::io::Error,
+        source: std::io::Error,
+        path: PathBuf,
         message: &'static str,
     },
 
@@ -25,8 +23,16 @@ pub enum Error {
     #[error("Failed to parse JSON")]
     JsonParseError(serde_json::Error),
 
+    // std::thread panics do not return printable values by default, and while
+    // it might be possible to upcast to a `dyn Debug`, it is hardly worth it
+    // here.
     #[error("Subtask join error")]
-    JoinError(JoinError),
+    JoinError,
+
+    // Unfortunately, in most cases where this error can occur the path is no
+    // longer available to avoid unnecessary cloning in the hot path.
+    #[error("Required file not found")]
+    FileNotFound,
 
     #[error("Internal error")]
     Internal { message: String },