Add caching of debug_diff_dirs computation

Summary:
# Context:

We are working to refactor the EdenFs CLI client library into a supported XFN consumable library to avoid code duplication and sub-optimal usage patterns.

# This Diff:

Watchman had some caching of frequently ran Sapling operations. This will be handly for clients that are more long-running (e.g. buck2 and meerkat in daemon mode). Here we are adding caching to debug_diff_dirs results.

Reviewed By: lXXXw

Differential Revision: D73067767

fbshipit-source-id: fe3aa4df15797c8e3e521315241e7e5255620846

Author: jdelliot

eden/fs/cli_rs/sapling-client/src/debug_diff_dirs.rs

@@ -8,11 +8,14 @@
 use std::path::Path;
 use std::path::PathBuf;
 use std::process::Stdio;
+use std::sync::LazyLock;
+use std::sync::Mutex;
 
 use async_process_traits::Child;
 use async_process_traits::Command;
 use async_process_traits::CommandSpawner;
 use async_process_traits::TokioCommandSpawner;
+use lru_cache::LruCache;
 use tokio::io::AsyncBufReadExt;
 use tokio::io::BufReader;
 
@@ -23,12 +26,29 @@ use crate::utils::get_sapling_executable_path;
 use crate::utils::get_sapling_options;
 use crate::utils::process_one_status_line;
 
+// NOTE: We might wish to cache Results here, but we would want to add a way to evict
+// Err entries from the cache based on some policy - e.g. a TTL in seconds.
+// For now, we just cache Ok entries.
+const DIFF_DIRS_LRU_CACHE_SIZE: usize = 32;
+static DIFF_DIRS_LRU_CACHE: LazyLock<Mutex<LruCache<GetDiffDirsParams, SaplingGetDiffDirsResult>>> =
+    LazyLock::new(|| Mutex::new(LruCache::new(DIFF_DIRS_LRU_CACHE_SIZE)));
+
 #[derive(Clone, Debug, PartialEq)]
 pub enum SaplingGetDiffDirsResult {
     Normal(Vec<(SaplingStatus, String)>),
     TooManyChanges,
 }
 
+#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+struct GetDiffDirsParams {
+    first: String,
+    second: Option<String>,
+    limit_results: usize,
+    root: Option<PathBuf>,
+    included_roots: Option<Vec<PathBuf>>,
+    excluded_roots: Option<Vec<PathBuf>>,
+}
+
 pub async fn get_diff_dirs_with_includes(
     first: &str,
     second: Option<&str>,
@@ -58,67 +78,84 @@ pub async fn get_diff_dirs(
     included_roots: &Option<Vec<PathBuf>>,
     excluded_roots: &Option<Vec<PathBuf>>,
 ) -> Result<SaplingGetDiffDirsResult> {
-    get_diff_dirs_impl(
-        &TokioCommandSpawner,
-        first,
-        second,
+    let params = GetDiffDirsParams {
+        first: first.to_string(),
+        second: second.map(|s| s.to_string()),
         limit_results,
-        root,
-        included_roots,
-        excluded_roots,
-    )
-    .await
+        root: root.clone(),
+        included_roots: included_roots.clone(),
+        excluded_roots: excluded_roots.clone(),
+    };
+
+    get_diff_dirs_impl(&TokioCommandSpawner, params).await
 }
 
 async fn get_diff_dirs_impl<Spawner>(
     spawner: &Spawner,
-    first: &str,
-    second: Option<&str>,
-    limit_results: usize,
-    root: &Option<PathBuf>,
-    included_roots: &Option<Vec<PathBuf>>,
-    excluded_roots: &Option<Vec<PathBuf>>,
+    params: GetDiffDirsParams,
 ) -> Result<SaplingGetDiffDirsResult>
 where
     Spawner: CommandSpawner,
 {
-    let mut args = vec!["debugdiffdirs", "--rev", first];
-    if let Some(second) = second {
-        args.push("--rev");
-        args.push(second);
+    {
+        let mut lru_cache = DIFF_DIRS_LRU_CACHE.lock().unwrap();
+        let entry = lru_cache.get_mut(&params).cloned();
+        if let Some(entry) = entry {
+            return Ok(entry);
+        }
     }
 
-    let root_path_arg: String;
-    if let Some(root) = root {
-        root_path_arg = format!("path:{}", root.display());
-        args.push(&root_path_arg);
-    };
+    let result = {
+        let mut args = vec!["debugdiffdirs", "--rev", &params.first];
+        let second: String;
+        if let Some(second_) = &params.second {
+            second = second_.to_string();
+            args.push("--rev");
+            args.push(&second);
+        }
 
-    let mut command = Spawner::Command::new(get_sapling_executable_path());
-    command
-        .envs(get_sapling_options())
-        .args(args)
-        .stdout(Stdio::piped());
-    let mut child = spawner.spawn(&mut command)?;
-    let stdout = child.stdout().take().ok_or_else(|| {
-        SaplingError::Other("Failed to read stdout when invoking 'sl debugdiffdirs'.".to_string())
-    })?;
-    let reader = BufReader::new(stdout);
-
-    let mut status = vec![];
-    let mut lines = reader.lines();
-    while let Some(line) = lines.next_line().await? {
-        if let Some(status_line) = process_one_status_line(&line)? {
-            if is_path_included(&status_line.1, included_roots, excluded_roots) {
-                if status.len() >= limit_results {
-                    return Ok(SaplingGetDiffDirsResult::TooManyChanges);
+        let root_path_arg: String;
+        if let Some(root) = &params.root {
+            root_path_arg = format!("path:{}", root.display());
+            args.push(&root_path_arg);
+        };
+
+        let mut command = Spawner::Command::new(get_sapling_executable_path());
+        command
+            .envs(get_sapling_options())
+            .args(args)
+            .stdout(Stdio::piped());
+        let mut child = spawner.spawn(&mut command)?;
+        let stdout = child.stdout().take().ok_or_else(|| {
+            SaplingError::Other(
+                "Failed to read stdout when invoking 'sl debugdiffdirs'.".to_string(),
+            )
+        })?;
+        let reader = BufReader::new(stdout);
+
+        let mut status = vec![];
+        let mut lines = reader.lines();
+        while let Some(line) = lines.next_line().await? {
+            if let Some(status_line) = process_one_status_line(&line)? {
+                if is_path_included(
+                    &status_line.1,
+                    &params.included_roots,
+                    &params.excluded_roots,
+                ) {
+                    if status.len() >= params.limit_results {
+                        return Ok(SaplingGetDiffDirsResult::TooManyChanges);
+                    }
+                    status.push(status_line);
                 }
-                status.push(status_line);
             }
         }
-    }
 
-    Ok(SaplingGetDiffDirsResult::Normal(status))
+        SaplingGetDiffDirsResult::Normal(status)
+    };
+
+    let mut lru_cache = DIFF_DIRS_LRU_CACHE.lock().unwrap();
+    lru_cache.insert(params, result.clone());
+    Ok(result)
 }
 
 fn is_path_included(
@@ -167,16 +204,15 @@ A fbcode/buck2/app/buck2_audit_server/src/perf
             get_sapling_executable_path(),
             Some((0, Some(output.as_bytes().to_vec()))),
         );
-        let result = get_diff_dirs_impl(
-            &spawner,
-            FIRST_COMMIT_ID,
-            Some(SECOND_COMMIT_ID),
-            1000,
-            &None,
-            &None,
-            &None,
-        )
-        .await?;
+        let params = GetDiffDirsParams {
+            first: FIRST_COMMIT_ID.to_string(),
+            second: Some(SECOND_COMMIT_ID.to_string()),
+            limit_results: 1000,
+            root: None,
+            included_roots: None,
+            excluded_roots: None,
+        };
+        let result = get_diff_dirs_impl(&spawner, params).await?;
         let expected = SaplingGetDiffDirsResult::Normal(vec![
             (
                 SaplingStatus::Modified,