Skip to content

Commit 1013ab9

Browse files
authored
perf(inode): O(1) lookup_child via name->ino HashMap (#166)
## Summary Every FUSE dentry probe (and the kernel does a lot of them on `ls -l` of a model dir, on `os.path.exists` checks, on tarball extracts, etc.) goes through `InodeTable::lookup_child(parent, name)`, which was an O(N) linear scan over `parent.children: Vec<DirChild>`. For directories with hundreds or thousands of entries (safetensors shards split across N files, large datasets), this scaled poorly. This PR adds a `child_index: HashMap<Arc<str>, u64>` alongside the existing `children` Vec. `lookup_child` is now O(1); the Vec stays for ordered iteration (readdir) and ino-based walks. ### Sync The index is updated at every children-mutation site: - `insert` (push), `move_child` (push on new parent, remove on old) - `evict_if_safe`, `evict_batch_if_safe` (retain by ino set) - `detach_from_parent` (called by `remove`) - `unlink_one` (remove by name) The `Arc<str>` name is shared between `DirChild` and `child_index` keys, so the index adds one `Arc::clone` (refcount bump) per directory entry, no extra heap alloc. ### Tests - New `test_child_index_stays_consistent_across_mutations` asserts the Vec ↔ HashMap invariant after each mutation type (insert, remove, unlink, re-insert). - 328 lib + 338 with `--features nfs`, clippy clean.
1 parent c2732f8 commit 1013ab9

1 file changed

Lines changed: 112 additions & 32 deletions

File tree

src/virtual_fs/inode.rs

Lines changed: 112 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ pub struct InodeEntry {
134134
/// create thousands of unique names under freshly-mkdir'd directories.
135135
pub children_from_remote: bool,
136136
pub children: Vec<DirChild>,
137+
/// Name → ino lookup for `lookup_child`. Kept in sync with `children`
138+
/// via the `add_child` / `remove_child_*` helpers so a directory with
139+
/// thousands of entries doesn't pay O(N) on every dentry probe.
140+
pub child_index: HashMap<Arc<str>, u64>,
137141
/// Old remote paths that should be deleted on next flush (set by rename of dirty files).
138142
pub pending_deletes: Vec<String>,
139143
/// When this inode's metadata was last validated against the remote (via HEAD).
@@ -149,6 +153,37 @@ impl InodeEntry {
149153
self.dirty_generation > 0
150154
}
151155

156+
/// Append a child to this directory and register it in the lookup index.
157+
/// `name` is moved into both structures (shared `Arc`).
158+
pub fn add_child(&mut self, ino: u64, name: Arc<str>) {
159+
self.child_index.insert(name.clone(), ino);
160+
self.children.push(DirChild { ino, name });
161+
}
162+
163+
/// Remove a child by ino. Returns the removed name (for callers that
164+
/// need to fix up external state).
165+
pub fn remove_child_by_ino(&mut self, ino: u64) -> Option<Arc<str>> {
166+
let pos = self.children.iter().position(|c| c.ino == ino)?;
167+
let removed = self.children.remove(pos);
168+
self.child_index.remove(&removed.name);
169+
Some(removed.name)
170+
}
171+
172+
/// Remove a child by name. Returns the removed ino.
173+
pub fn remove_child_by_name(&mut self, name: &str) -> Option<u64> {
174+
let ino = self.child_index.remove(name)?;
175+
if let Some(pos) = self.children.iter().position(|c| c.ino == ino) {
176+
self.children.remove(pos);
177+
}
178+
Some(ino)
179+
}
180+
181+
/// Drop excess capacity from both child stores after a bulk removal.
182+
pub fn shrink_children(&mut self) {
183+
self.children.shrink_to_fit();
184+
self.child_index.shrink_to_fit();
185+
}
186+
152187
/// Returns true if the children listing has been populated. Only
153188
/// meaningful for directories.
154189
pub fn children_loaded(&self) -> bool {
@@ -254,6 +289,7 @@ impl InodeTable {
254289
children_loaded_at: None,
255290
children_from_remote: false,
256291
children: Vec::new(),
292+
child_index: HashMap::new(),
257293
pending_deletes: Vec::new(),
258294
last_revalidated: None,
259295
eviction: EvictionState::default(),
@@ -456,7 +492,7 @@ impl InodeTable {
456492
self.path_to_inode.remove(&*entry.full_path);
457493

458494
if let Some(parent) = self.inodes.get_mut(&entry.parent) {
459-
parent.children.retain(|c| c.ino != ino);
495+
parent.remove_child_by_ino(ino);
460496
// Force readdir to re-fetch so the inode can be re-materialized
461497
// on next access. Without this the kernel would ask for a child
462498
// that no longer exists in our in-memory tree.
@@ -488,18 +524,21 @@ impl InodeTable {
488524
}
489525

490526
let mut evicted = Vec::new();
491-
for set in by_parent.values() {
527+
for (parent_ino, set) in &by_parent {
492528
for &ino in set {
493529
if let Some(entry) = self.inodes.remove(&ino) {
494530
self.path_to_inode.remove(&*entry.full_path);
531+
if let Some(parent) = self.inodes.get_mut(parent_ino) {
532+
parent.child_index.remove(&entry.name);
533+
}
495534
evicted.push(ino);
496535
}
497536
}
498537
}
499538
for (parent_ino, evicted_set) in &by_parent {
500539
if let Some(parent) = self.inodes.get_mut(parent_ino) {
501540
parent.children.retain(|c| !evicted_set.contains(&c.ino));
502-
parent.children.shrink_to_fit();
541+
parent.shrink_children();
503542
// Force readdir to re-fetch the listing so evicted inodes can
504543
// be re-materialized on next access.
505544
parent.children_loaded_at = None;
@@ -512,18 +551,11 @@ impl InodeTable {
512551
self.path_to_inode.get(path).and_then(|ino| self.inodes.get(ino))
513552
}
514553

515-
/// Find a child of `parent` by name.
516-
/// Skips stale DirChild entries whose inode has been removed.
554+
/// Find a child of `parent` by name. O(1) via `child_index`.
517555
pub fn lookup_child(&self, parent: u64, name: &str) -> Option<&InodeEntry> {
518556
let parent_entry = self.inodes.get(&parent)?;
519-
for child in &parent_entry.children {
520-
if &*child.name == name
521-
&& let Some(entry) = self.inodes.get(&child.ino)
522-
{
523-
return Some(entry);
524-
}
525-
}
526-
None
557+
let ino = *parent_entry.child_index.get(name)?;
558+
self.inodes.get(&ino)
527559
}
528560

529561
/// Insert a new inode, returning its inode number.
@@ -599,6 +631,7 @@ impl InodeTable {
599631
children_loaded_at: None,
600632
children_from_remote: false,
601633
children: Vec::new(),
634+
child_index: HashMap::new(),
602635
pending_deletes: Vec::new(),
603636
last_revalidated: Some(Instant::now()),
604637
eviction: EvictionState {
@@ -612,10 +645,7 @@ impl InodeTable {
612645

613646
// Add to parent's children
614647
if let Some(parent_entry) = self.inodes.get_mut(&parent) {
615-
parent_entry.children.push(DirChild {
616-
ino: inode,
617-
name: name_arc,
618-
});
648+
parent_entry.add_child(inode, name_arc);
619649
// POSIX: new subdirectory's ".." links to parent
620650
if kind == InodeKind::Directory {
621651
parent_entry.nlink += 1;
@@ -801,7 +831,7 @@ impl InodeTable {
801831
invalidate_children_loaded: bool,
802832
) {
803833
if let Some(parent) = self.inodes.get_mut(&parent_ino) {
804-
parent.children.retain(|c| c.ino != child_ino);
834+
parent.remove_child_by_ino(child_ino);
805835
if invalidate_children_loaded {
806836
parent.children_loaded_at = None;
807837
}
@@ -826,6 +856,9 @@ impl InodeTable {
826856
if let Some(child) = self.inodes.remove(&child_ino) {
827857
self.path_to_inode.remove(&*child.full_path);
828858
stack.extend(child.children.iter().map(|c| c.ino));
859+
// The descendants' parents are also being removed in this
860+
// sweep (entire subtree), so their child_index will be
861+
// discarded — no per-entry cleanup needed.
829862
}
830863
}
831864

@@ -839,16 +872,13 @@ impl InodeTable {
839872
// Find child ino and build full_path in a single parent lookup
840873
let (child_ino, full_path) = {
841874
let parent_entry = self.inodes.get(&parent)?;
842-
let ino = parent_entry.children.iter().find(|c| &*c.name == name).map(|c| c.ino)?;
875+
let ino = *parent_entry.child_index.get(name)?;
843876
let path = child_path(&parent_entry.full_path, name);
844877
(ino, path)
845878
};
846879

847-
// Remove the DirChild from parent
848-
if let Some(parent_entry) = self.inodes.get_mut(&parent)
849-
&& let Some(pos) = parent_entry.children.iter().position(|c| &*c.name == name)
850-
{
851-
parent_entry.children.remove(pos);
880+
if let Some(parent_entry) = self.inodes.get_mut(&parent) {
881+
parent_entry.remove_child_by_name(name);
852882
}
853883

854884
self.path_to_inode.remove(full_path.as_str());
@@ -872,21 +902,16 @@ impl InodeTable {
872902
let is_dir = self.inodes.get(&ino).is_some_and(|e| e.kind == InodeKind::Directory);
873903

874904
// Detach from old parent
875-
if let Some(old_p) = self.inodes.get_mut(&old_parent)
876-
&& let Some(pos) = old_p.children.iter().position(|c| c.ino == ino && &*c.name == old_name)
877-
{
878-
old_p.children.remove(pos);
905+
if let Some(old_p) = self.inodes.get_mut(&old_parent) {
906+
old_p.remove_child_by_name(old_name);
879907
}
880908

881909
// Allocate the new name once, share with the InodeEntry below.
882910
let new_name_arc: Arc<str> = Arc::from(new_name);
883911

884912
// Attach to new parent
885913
if let Some(new_p) = self.inodes.get_mut(&new_parent) {
886-
new_p.children.push(DirChild {
887-
ino,
888-
name: new_name_arc.clone(),
889-
});
914+
new_p.add_child(ino, new_name_arc.clone());
890915
}
891916

892917
// Adjust parent nlink for cross-parent directory moves
@@ -2409,4 +2434,59 @@ mod tests {
24092434
table.drop_open_handles(busy);
24102435
assert!(!table.has_open_handles(busy));
24112436
}
2437+
2438+
// ── child_index invariants ─────────────────────────────────────
2439+
2440+
/// Assert that every parent's `child_index` exactly matches its
2441+
/// `children` Vec (same names mapped to same inos, no extras).
2442+
fn assert_child_index_consistent(table: &InodeTable) {
2443+
for (ino, entry) in &table.inodes {
2444+
if entry.kind != InodeKind::Directory {
2445+
continue;
2446+
}
2447+
assert_eq!(
2448+
entry.children.len(),
2449+
entry.child_index.len(),
2450+
"directory ino={ino} children/child_index length mismatch"
2451+
);
2452+
for c in &entry.children {
2453+
assert_eq!(
2454+
entry.child_index.get(&c.name).copied(),
2455+
Some(c.ino),
2456+
"directory ino={ino} child_index missing name={}",
2457+
c.name
2458+
);
2459+
}
2460+
}
2461+
}
2462+
2463+
#[test]
2464+
fn test_child_index_stays_consistent_across_mutations() {
2465+
let mut table = InodeTable::new(false);
2466+
2467+
let a = mk_file(&mut table, "a.txt");
2468+
let b = mk_file(&mut table, "b.txt");
2469+
mk_file(&mut table, "c.txt");
2470+
assert_child_index_consistent(&table);
2471+
2472+
// O(1) lookup via index.
2473+
assert_eq!(table.lookup_child(ROOT_INODE, "a.txt").map(|e| e.inode), Some(a));
2474+
assert_eq!(table.lookup_child(ROOT_INODE, "b.txt").map(|e| e.inode), Some(b));
2475+
assert!(table.lookup_child(ROOT_INODE, "missing.txt").is_none());
2476+
2477+
// remove() path.
2478+
table.remove(b);
2479+
assert_child_index_consistent(&table);
2480+
assert!(table.lookup_child(ROOT_INODE, "b.txt").is_none());
2481+
2482+
// unlink_one() path.
2483+
let (_, _) = table.unlink_one(ROOT_INODE, "a.txt").unwrap();
2484+
assert_child_index_consistent(&table);
2485+
assert!(table.lookup_child(ROOT_INODE, "a.txt").is_none());
2486+
2487+
// Re-insert under same name should rebuild the index entry.
2488+
let a2 = mk_file(&mut table, "a.txt");
2489+
assert_child_index_consistent(&table);
2490+
assert_eq!(table.lookup_child(ROOT_INODE, "a.txt").map(|e| e.inode), Some(a2));
2491+
}
24122492
}

0 commit comments

Comments
 (0)