Avoid some allocations, hashes that aren't needed

This commit removes allocations in the FUSE component that don't
need to exist and adjust the interior hashmap to use FxHashMap,
avoiding a secure hash in favor of a fast one. Also, directories
now keep their children in a BTreeSet so that readdir is always
in the same order when called.

Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>

examples/lading-logrotatefs.yaml

@@ -8,7 +8,7 @@ generator:
         total_rotations: 4
         max_depth: 0
         variant: "ascii"
-        bytes_per_second: 10MB
+        bytes_per_second: 1.3MB
         maximum_prebuild_cache_size_bytes: 1GB
         mount_point: /tmp/logrotate
 

lading/src/generator/file_gen/logrotate_fs.rs

@@ -342,52 +342,56 @@ impl Filesystem for LogrotateFS {
         state.advance_time(tick);
 
         let root_inode = state.root_inode();
+        let mut entry_offset = 0;
 
-        // TODO building up a vec of entries here to handle offset really does
-        // suggest that the model needs to be exposed in such a way that this
-        // needn't be done.
-        //
-        // Ah, actually, the right buffer to push into is reply.add. There's no
-        // need for `entries` at all.
-        let mut entries = Vec::new();
+        // Entry 0: "."
+        if entry_offset >= offset
+            && reply.add(ino, entry_offset + 1, fuser::FileType::Directory, ".")
+        {
+            reply.ok();
+            return;
+        }
+        entry_offset += 1;
 
-        // '.' and '..'
-        entries.push((ino, fuser::FileType::Directory, ".".to_string()));
+        // Entry 1: ".." when applicable
         if ino != root_inode as u64 {
-            let parent_ino = state
-                .get_parent_inode(ino as usize)
-                .expect("inode must have parent");
-            entries.push((
-                parent_ino as u64,
-                fuser::FileType::Directory,
-                "..".to_string(),
-            ));
+            if entry_offset >= offset {
+                let parent_ino = state
+                    .get_parent_inode(ino as usize)
+                    .expect("inode must have parent");
+                if reply.add(
+                    parent_ino as u64,
+                    entry_offset + 1,
+                    fuser::FileType::Directory,
+                    "..",
+                ) {
+                    reply.ok();
+                    return;
+                }
+            }
+            entry_offset += 1;
         }
 
-        // remaining children
+        // Child entries, returned in inode order by `State::readdir`
         if let Some(child_inodes) = state.readdir(ino as usize) {
-            for child_ino in child_inodes {
-                let file_type = state
-                    .get_file_type(*child_ino)
-                    .expect("inode must have file type");
-                let child_name = state.get_name(*child_ino).expect("inode must have a name");
-                entries.push((*child_ino as u64, file_type, child_name.to_string()));
+            for &child_ino in child_inodes {
+                if entry_offset >= offset {
+                    let file_type = state
+                        .get_file_type(child_ino)
+                        .expect("inode must have file type");
+                    let child_name = state.get_name(child_ino).expect("inode must have a name");
+                    if reply.add(child_ino as u64, entry_offset + 1, file_type, child_name) {
+                        reply.ok();
+                        return;
+                    }
+                }
+                entry_offset += 1;
             }
         } else {
             reply.error(ENOENT);
             return;
         }
 
-        let mut idx = offset as usize;
-        while idx < entries.len() {
-            let (inode, file_type, name) = &entries[idx];
-            let next_offset = (idx + 1) as i64;
-            if reply.add(*inode, next_offset, *file_type, name) {
-                // Buffer is full, exit the loop
-                break;
-            }
-            idx += 1;
-        }
         reply.ok();
     }
 

lading/src/generator/file_gen/logrotate_fs/model.rs

@@ -1,11 +1,11 @@
 //! Model the internal logic of a logrotate filesystem.
 
-use std::collections::{HashMap, HashSet};
-
 use bytes::Bytes;
 use lading_payload::block;
 use metrics::counter;
 use rand::Rng;
+use rustc_hash::FxHashMap;
+use std::collections::BTreeSet;
 
 /// Time representation of the model
 pub(crate) type Tick = u64;
@@ -249,12 +249,21 @@ impl File {
 
 /// Model representation of a `Directory`. Contains children are `Directory`
 /// instances or `File` instances. Root directory will not have a `parent`.
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub(crate) struct Directory {
-    children: HashSet<Inode>,
+    children: BTreeSet<Inode>,
     parent: Option<Inode>,
 }
 
+impl Directory {
+    fn new(parent: Option<Inode>) -> Self {
+        Self {
+            children: BTreeSet::default(),
+            parent,
+        }
+    }
+}
+
 /// A filesystem object, either a `File` or a `Directory`.
 #[derive(Debug)]
 pub(crate) enum Node {
@@ -278,7 +287,7 @@ pub(crate) enum Node {
 /// filesystem. It does not contain any bytes, the caller must maintain this
 /// themselves.
 pub(crate) struct State {
-    nodes: HashMap<Inode, Node>,
+    nodes: FxHashMap<Inode, Node>,
     root_inode: Inode,
     now: Tick,
     block_cache: block::Cache,
@@ -288,6 +297,7 @@ pub(crate) struct State {
     group_names: Vec<Vec<String>>,
     next_inode: Inode,
     next_file_handle: u64,
+    inode_scratch: Vec<Inode>,
 }
 
 impl std::fmt::Debug for State {
@@ -297,6 +307,7 @@ impl std::fmt::Debug for State {
             .field("root_inode", &self.root_inode)
             .field("now", &self.now)
             // intentionally leaving out block_cache
+            // intentionally leaving out inode_scratch
             .field("max_rotations", &self.max_rotations)
             .field("max_bytes_per_file", &self.max_bytes_per_file)
             .field("group_names", &self.group_names)
@@ -349,12 +360,9 @@ impl State {
         R: Rng,
     {
         let root_inode: Inode = 1; // `/`
-        let mut nodes = HashMap::new();
+        let mut nodes = FxHashMap::default();
 
-        let root_dir = Directory {
-            children: HashSet::new(),
-            parent: None,
-        };
+        let root_dir = Directory::default();
         nodes.insert(
             root_inode,
             Node::Directory {
@@ -373,6 +381,7 @@ impl State {
             group_names: Vec::new(),
             next_inode: 2,
             next_file_handle: 0,
+            inode_scratch: Vec::with_capacity(concurrent_logs as usize),
         };
 
         if concurrent_logs == 0 {
@@ -435,10 +444,7 @@ impl State {
                             let new_inode = state.next_inode;
                             state.next_inode += 1;
 
-                            let new_dir = Directory {
-                                children: HashSet::new(),
-                                parent: Some(current_inode),
-                            };
+                            let new_dir = Directory::new(Some(current_inode));
                             state.nodes.insert(
                                 new_inode,
                                 Node::Directory {
@@ -536,8 +542,11 @@ impl State {
     fn advance_time_inner(&mut self, now: Tick) {
         assert!(now >= self.now);
 
-        let mut inodes: Vec<Inode> = self.nodes.keys().copied().collect();
-        for inode in inodes.drain(..) {
+        for inode in self.nodes.keys() {
+            self.inode_scratch.push(*inode);
+        }
+
+        for inode in self.inode_scratch.drain(..) {
             let (rotated_inode, parent_inode, bytes_per_tick, group_id, ordinal) = {
                 // If the node pointed to by inode doesn't exist, that's a
                 // catastrophic programming error. We just copied all inode to node
@@ -806,14 +815,15 @@ impl State {
         }
     }
 
-    /// Read inodes from a directory
+    /// Read inodes from a directory.
     ///
     /// Returns None if the inode is a `File`, else returns the hashset of
-    /// children inodes.
+    /// children inodes. Guaranteed to be in the same order so long as time does
+    /// not advance.
     ///
     /// Function does not advance time in the model.
     #[tracing::instrument(skip(self))]
-    pub(crate) fn readdir(&self, inode: Inode) -> Option<&HashSet<Inode>> {
+    pub(crate) fn readdir(&self, inode: Inode) -> Option<&BTreeSet<Inode>> {
         if let Some(Node::Directory { dir, .. }) = self.nodes.get(&inode) {
             Some(&dir.children)
         } else {