consolidate arena/picker via pub trait

Author: dmtrKovalenko

crates/fff-c/src/ffi_types.rs

@@ -74,10 +74,9 @@ pub struct FffFileItem {
 
 impl FffFileItem {
     pub fn from_item(item: &FileItem, picker: &FilePicker) -> Self {
-        let rel_path = picker.relative_path(item);
         FffFileItem {
-            relative_path: cstring_new(&rel_path),
-            file_name: cstring_new(&picker.file_name(item)),
+            relative_path: cstring_new(&item.relative_path(picker)),
+            file_name: cstring_new(&item.file_name(picker)),
             git_status: cstring_new(format_git_status(item.git_status)),
             size: item.size,
             modified: item.modified,
@@ -309,10 +308,9 @@ impl FffGrepMatch {
             None => (false, 0),
         };
 
-        let rel_path = picker.relative_path(file);
         FffGrepMatch {
-            relative_path: cstring_new(&rel_path),
-            file_name: cstring_new(&picker.file_name(file)),
+            relative_path: cstring_new(&file.relative_path(picker)),
+            file_name: cstring_new(&file.file_name(picker)),
             git_status: cstring_new(format_git_status(file.git_status)),
             line_content: cstring_new(&m.line_content),
             match_ranges,

crates/fff-core/src/file_picker.rs

@@ -30,6 +30,7 @@
 //! The background scanner and watcher acquire write locks only when mutating
 //! the file index, so read-heavy search workloads rarely contend.
 
+use crate::FFFStringStorage;
 use crate::background_watcher::BackgroundWatcher;
 use crate::bigram_filter::{BigramFilter, BigramIndexBuilder, BigramOverlay};
 use crate::error::Error;
@@ -226,7 +227,7 @@ impl FileSync {
         // Binary search files by (parent_dir, filename) — same order as the sort
         self.files[..self.base_count].binary_search_by(|f| {
             f.parent_dir_index().cmp(&dir_idx).then_with(|| {
-                let fname = f.file_name_from_arena(arena);
+                let fname = f.file_name(arena);
                 fname.as_str().cmp(filename)
             })
         })
@@ -247,19 +248,31 @@ impl FileSync {
         self.files.insert(position, file);
     }
 
-    /// Remove files matching predicate from both base and overflow.
-    /// Returns number of files removed. Adjusts `base_count` accordingly.
-    fn retain_files<F>(&mut self, mut predicate: F) -> usize
+    fn retain_files_with_arena<F>(&mut self, mut predicate: F) -> usize
     where
-        F: FnMut(&FileItem) -> bool,
+        F: FnMut(&FileItem, ArenaPtr) -> bool,
     {
+        let base_arena = self.arena_base_ptr();
+        let overflow_arena = self.overflow_arena_ptr();
+
+        let base_count = self.base_count;
         let initial_len = self.files.len();
-        // Count how many base files survive.
-        let base_retained = self.files[..self.base_count]
+        let base_retained = self.files[..base_count]
             .iter()
-            .filter(|f| predicate(f))
+            .filter(|f| predicate(f, base_arena))
             .count();
-        self.files.retain(predicate);
+
+        self.files.retain(|f| {
+            predicate(
+                f,
+                if f.is_overflow() {
+                    overflow_arena
+                } else {
+                    base_arena
+                },
+            )
+        });
+
         self.base_count = base_retained;
         initial_len - self.files.len()
     }
@@ -435,50 +448,26 @@ impl std::fmt::Debug for FilePicker {
     }
 }
 
-impl FilePicker {
-    pub fn base_path(&self) -> &Path {
-        &self.base_path
-    }
-
-    #[inline]
-    pub(crate) fn arena_base_ptr(&self) -> ArenaPtr {
-        self.sync_data.arena_base_ptr()
-    }
-
-    /// File name (e.g. `Button.tsx`) from the arena.
-    #[inline]
-    pub fn file_name(&self, file: &FileItem) -> String {
-        file.file_name_from_arena(self.sync_data.arena_for_file(file))
-    }
-
-    /// Relative path (e.g. `src/components/Button.tsx`) from the arena.
+impl FFFStringStorage for &FilePicker {
     #[inline]
-    pub fn relative_path(&self, file: &FileItem) -> String {
-        file.relative_path_from_arena(self.sync_data.arena_for_file(file))
+    fn arena_for(&self, file: &FileItem) -> crate::simd_path::ArenaPtr {
+        self.sync_data.arena_for_file(file)
     }
 
-    /// Directory portion of the path (e.g. `src/components/`).
     #[inline]
-    pub fn dir_str(&self, file: &FileItem) -> String {
-        file.dir_str(self.sync_data.arena_for_file(file))
-    }
-
-    /// Absolute path on disk.
-    #[inline]
-    pub fn absolute_path(&self, file: &FileItem) -> PathBuf {
-        file.absolute_path(self.sync_data.arena_for_file(file), &self.base_path)
+    fn base_arena(&self) -> crate::simd_path::ArenaPtr {
+        self.sync_data.arena_base_ptr()
     }
 
-    /// Relative path of a directory entry.
     #[inline]
-    pub fn dir_relative_path(&self, dir: &DirItem) -> String {
-        dir.relative_path(self.sync_data.arena_base_ptr())
+    fn overflow_arena(&self) -> crate::simd_path::ArenaPtr {
+        self.sync_data.overflow_arena_ptr()
     }
+}
 
-    /// Absolute path of a directory entry.
-    #[inline]
-    pub fn dir_absolute_path(&self, dir: &DirItem) -> PathBuf {
-        dir.absolute_path(self.sync_data.arena_base_ptr(), &self.base_path)
+impl FilePicker {
+    pub fn base_path(&self) -> &Path {
+        &self.base_path
     }
 
     /// Convert an absolute path to a relative path string (relative to base_path).
@@ -1179,17 +1168,16 @@ impl FilePicker {
     // TODO make this O(n)
     pub fn remove_all_files_in_dir(&mut self, dir: impl AsRef<Path>) -> usize {
         let dir_path = dir.as_ref();
-        let dir_rel = self.to_relative_path(dir_path).unwrap_or("").to_string();
-        let dir_prefix = if dir_rel.is_empty() {
+        let relative_dir = self.to_relative_path(dir_path).unwrap_or("").to_string();
+
+        let dir_prefix = if relative_dir.is_empty() {
             String::new()
         } else {
-            format!("{}/", dir_rel)
+            format!("{}{}", relative_dir, std::path::MAIN_SEPARATOR)
         };
-        // Use the safe retain_files method which maintains both indices
-        let arena = self.arena_base_ptr();
-        self.sync_data.retain_files(|file| {
+
+        self.sync_data.retain_files_with_arena(|file, arena| {
             !file.relative_path_starts_with(arena, &dir_prefix)
-                && !file.relative_path_eq(arena, &dir_rel)
         })
     }
 
@@ -1204,6 +1192,11 @@ impl FilePicker {
         }
     }
 
+    #[inline]
+    pub(crate) fn arena_base_ptr(&self) -> ArenaPtr {
+        self.sync_data.arena_base_ptr()
+    }
+
     /// Spawn a background thread to rebuild the bigram index after rescan.
     pub(crate) fn spawn_post_rescan_rebuild(&self, shared_picker: SharedPicker) -> bool {
         if !self.warmup_mmap_cache || self.cancelled.load(Ordering::Relaxed) {
@@ -1934,10 +1927,7 @@ fn walk_filesystem(
         files.par_sort_unstable_by(|a, b| {
             a.parent_dir_index()
                 .cmp(&b.parent_dir_index())
-                .then_with(|| {
-                    a.file_name_from_arena(arena)
-                        .cmp(&b.file_name_from_arena(arena))
-                })
+                .then_with(|| a.file_name(arena).cmp(&b.file_name(arena)))
         });
     });
 

crates/fff-core/src/lib.rs

@@ -85,7 +85,7 @@
 //! );
 //!
 //! assert!(results.total_matched > 0);
-//! assert!(picker.relative_path(results.items.first().unwrap()).ends_with("lib.rs"));
+//! assert!(results.items.first().unwrap().relative_path(picker).ends_with("lib.rs"));
 //!
 //! let _ = std::fs::remove_dir_all(&tmp);
 //! # Ok::<(), Box<dyn std::error::Error>>(())

crates/fff-core/src/score.rs

@@ -687,9 +687,9 @@ mod tests {
         assert_eq!(scores[2].total, 200, "Third should be third highest");
 
         // Verify the files match
-        assert_eq!(items[0].relative_path_from_arena(arena), "file4.rs");
-        assert_eq!(items[1].relative_path_from_arena(arena), "file6.rs");
-        assert_eq!(items[2].relative_path_from_arena(arena), "file2.rs");
+        assert_eq!(items[0].relative_path(arena), "file4.rs");
+        assert_eq!(items[1].relative_path(arena), "file6.rs");
+        assert_eq!(items[2].relative_path(arena), "file2.rs");
     }
 
     #[test]
@@ -783,9 +783,9 @@ mod tests {
         assert_eq!(scores[0].total, 200);
         assert_eq!(scores[1].total, 100);
         assert_eq!(scores[2].total, 50);
-        assert_eq!(items[0].relative_path_from_arena(arena), "file2.rs");
-        assert_eq!(items[1].relative_path_from_arena(arena), "file1.rs");
-        assert_eq!(items[2].relative_path_from_arena(arena), "file3.rs");
+        assert_eq!(items[0].relative_path(arena), "file2.rs");
+        assert_eq!(items[1].relative_path(arena), "file1.rs");
+        assert_eq!(items[2].relative_path(arena), "file3.rs");
     }
 }
 
@@ -867,7 +867,7 @@ mod filename_bonus_tests {
         items
             .iter()
             .zip(scores.iter())
-            .map(|(f, s)| (f.relative_path_from_arena(arena).to_string(), s.clone()))
+            .map(|(f, s)| (f.relative_path(arena).to_string(), s.clone()))
             .collect()
     }
 
@@ -1084,10 +1084,7 @@ mod typo_resistance_tests {
             },
         };
         let (items, _, _) = fuzzy_match_and_score_files(files, &ctx, files.len(), arena, arena);
-        items
-            .iter()
-            .map(|f| f.relative_path_from_arena(arena))
-            .collect()
+        items.iter().map(|f| f.relative_path(arena)).collect()
     }
 
     #[test]

crates/fff-core/src/types.rs

@@ -8,6 +8,36 @@ use crate::query_tracker::QueryMatchEntry;
 use crate::simd_path::{ArenaPtr, PATH_BUF_SIZE};
 use fff_query_parser::{FFFQuery, FuzzyQuery, Location};
 
+/// Different sources of the string storage used by FFF
+/// implements as a deduplicated 16-bytes alined heap
+/// can be stored in RAM or on disk
+pub trait FFFStringStorage {
+    /// Resolve the arena for a [`FileItem`] (handles base vs overflow split).
+    fn arena_for(&self, file: &FileItem) -> ArenaPtr;
+
+    /// The base arena (scan-time paths).
+    fn base_arena(&self) -> ArenaPtr;
+    /// The overflow arena (paths added after the last full scan).
+    fn overflow_arena(&self) -> ArenaPtr;
+}
+
+impl FFFStringStorage for ArenaPtr {
+    #[inline]
+    fn arena_for(&self, _file: &FileItem) -> ArenaPtr {
+        *self
+    }
+
+    #[inline]
+    fn base_arena(&self) -> ArenaPtr {
+        *self
+    }
+
+    #[inline]
+    fn overflow_arena(&self) -> ArenaPtr {
+        *self
+    }
+}
+
 /// Cached file contents — mmap on Unix, heap buffer on Windows.
 ///
 /// On Windows, memory-mapped files hold the file handle open and prevent
@@ -47,30 +77,48 @@ impl FileItemFlags {
     pub const OVERFLOW: u8 = 1 << 2;
 }
 
+pub struct DirFlags;
+
+impl DirFlags {
+    pub const OVERFLOW: u8 = 1 << 0;
+}
+
 /// A directory in the file index. Shares chunk arena with file paths.
 #[derive(Debug, Clone)]
 pub struct DirItem {
+    flags: u8,
     pub(crate) path: crate::simd_path::ChunkedString,
 }
 
 impl DirItem {
+    #[inline(always)]
+    pub fn is_overflow(&self) -> bool {
+        self.flags & DirFlags::OVERFLOW == 0
+    }
+
     pub(crate) fn new(path: crate::simd_path::ChunkedString) -> Self {
-        Self { path }
+        Self { path, flags: 0 }
     }
 
     pub(crate) fn read_relative_path<'a>(&self, arena: ArenaPtr, buf: &'a mut [u8]) -> &'a str {
         self.path.read_to_buf(arena, buf)
     }
 
     /// Relative dir path as owned String (cold path).
-    pub fn relative_path(&self, arena: ArenaPtr) -> String {
+    pub fn relative_path(&self, arena: impl FFFStringStorage) -> String {
         let mut out = String::new();
-        self.path.write_to_string(arena, &mut out);
+        let ptr = if self.is_overflow() {
+            arena.overflow_arena()
+        } else {
+            arena.base_arena()
+        };
+
+        self.path.write_to_string(ptr, &mut out);
         out
     }
 
     /// A path = base_path + "/" + relative. Cold path, allocates.
-    pub fn absolute_path(&self, arena: ArenaPtr, base_path: &Path) -> PathBuf {
+    pub fn absolute_path(&self, arena: impl FFFStringStorage, base_path: &Path) -> PathBuf {
         let rel = self.relative_path(arena);
         if rel.is_empty() {
             base_path.to_path_buf()
@@ -140,9 +188,9 @@ impl FileItem {
     }
 
     /// Returns an absolute path of the file
-    pub fn absolute_path(&self, arena: ArenaPtr, base_path: &Path) -> PathBuf {
+    pub fn absolute_path(&self, arena: impl FFFStringStorage, base_path: &Path) -> PathBuf {
         let mut buf = [0u8; PATH_BUF_SIZE];
-        let rel = self.path.read_to_buf(arena, &mut buf);
+        let rel = self.path.read_to_buf(arena.arena_for(self), &mut buf);
         base_path.join(rel)
     }
 
@@ -158,29 +206,29 @@ impl FileItem {
         self.parent_dir = idx;
     }
 
-    pub(crate) fn dir_str(&self, arena: ArenaPtr) -> String {
+    pub fn dir_str(&self, arena: impl FFFStringStorage) -> String {
         let mut s = String::with_capacity(64);
-        self.path.write_dir_to(arena, &mut s);
+        self.path.write_dir_to(arena.arena_for(self), &mut s);
         s
     }
 
     pub(crate) fn write_dir_str(&self, arena: ArenaPtr, out: &mut String) {
         self.path.write_dir_to(arena, out);
     }
 
-    pub(crate) fn file_name_from_arena(&self, arena: ArenaPtr) -> String {
+    pub fn file_name(&self, arena: impl FFFStringStorage) -> String {
         let mut s = String::with_capacity(32);
-        self.path.write_filename_to(arena, &mut s);
+        self.path.write_filename_to(arena.arena_for(self), &mut s);
         s
     }
 
     pub(crate) fn write_file_name_from_arena(&self, arena: ArenaPtr, out: &mut String) {
         self.path.write_filename_to(arena, out);
     }
 
-    pub(crate) fn relative_path_from_arena(&self, arena: ArenaPtr) -> String {
+    pub fn relative_path(&self, arena: impl FFFStringStorage) -> String {
         let mut s = String::with_capacity(64);
-        self.path.write_to_string(arena, &mut s);
+        self.path.write_to_string(arena.arena_for(self), &mut s);
         s
     }
 

crates/fff-core/tests/bigram_overlay_coherence_test.rs

@@ -1055,7 +1055,7 @@ fn fuzzy_search_paths(picker: &FilePicker, query: &str) -> Vec<String> {
     result
         .items
         .iter()
-        .map(|f| picker.relative_path(f))
+        .map(|f| f.relative_path(picker))
         .collect()
 }