Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid extra name lookup #1044

Merged
merged 1 commit into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions lading/src/bin/logrotate_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,11 @@ impl Filesystem for LogrotateFS {
) {
let tick = self.get_current_tick();

// NOTE this call to State::read is almost surely allocating. It'd be
// pretty slick if we could get the buffer directly from the OS to pass
// down for writing but we can't. I suppose we could send up the raw
// blocks and then chain them together as needed but absent a compelling
// reason to do that the simplicity of this API is nice.
if let Some(data) = self
.state
.read(ino as usize, offset as usize, size as usize, tick)
Expand Down Expand Up @@ -234,16 +239,12 @@ impl Filesystem for LogrotateFS {

// reaming children
if let Some(child_inodes) = self.state.readdir(ino as usize) {
for child_ino in child_inodes {
if let Some(name) = self.state.get_name(*child_ino) {
let file_type = self
.state
.get_file_type(*child_ino)
.expect("inode must have file type");
entries.push((*child_ino as u64, file_type, name.to_string()));
} else {
error!("child inode has no name");
}
for (child_name, child_ino) in child_inodes {
let file_type = self
.state
.get_file_type(*child_ino)
.expect("inode must have file type");
entries.push((*child_ino as u64, file_type, child_name.clone()));
}
} else {
reply.error(ENOENT);
Expand Down
45 changes: 13 additions & 32 deletions lading/src/generator/file_gen/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

//use lading_payload::block;

use std::collections::{HashMap, HashSet};
use std::collections::HashMap;

use bytes::Bytes;
use lading_payload::block;
Expand Down Expand Up @@ -89,7 +89,7 @@ impl File {
/// instances or `File` instances. Root directory will not have a `parent`.
#[derive(Debug)]
pub struct Directory {
children: HashSet<Inode>,
children: HashMap<String, Inode>,
parent: Option<Inode>,
}

Expand Down Expand Up @@ -172,10 +172,10 @@ impl State {
let mut nodes = HashMap::new();

let mut root_dir = Directory {
children: HashSet::new(),
children: HashMap::new(),
parent: None,
};
root_dir.children.insert(logs_inode);
root_dir.children.insert("logs".to_string(), logs_inode);
nodes.insert(
root_inode,
Node::Directory {
Expand All @@ -185,10 +185,12 @@ impl State {
);

let mut logs_dir = Directory {
children: HashSet::new(),
children: HashMap::new(),
parent: Some(root_inode),
};
logs_dir.children.insert(foo_log_inode);
logs_dir
.children
.insert("foo.log".to_string(), foo_log_inode);
nodes.insert(
logs_inode,
Node::Directory {
Expand Down Expand Up @@ -252,20 +254,10 @@ impl State {
self.advance_time(now);

if let Some(Node::Directory { dir, .. }) = self.nodes.get(&parent_inode) {
for child_inode in &dir.children {
let child_node = &self
.nodes
.get(child_inode)
.expect("catastrophic programming error");
let child_name = match child_node {
Node::Directory { name, .. } | Node::File { name, .. } => name,
};
if child_name == name {
return Some(*child_inode);
}
}
dir.children.get(name).copied()
} else {
None
}
None
}

/// Look up the attributes for an `Inode`.
Expand Down Expand Up @@ -342,25 +334,14 @@ impl State {
///
/// Function does not advance time in the model.
#[tracing::instrument(skip(self))]
pub fn readdir(&self, inode: Inode) -> Option<&HashSet<Inode>> {
pub fn readdir(&self, inode: Inode) -> Option<&HashMap<String, Inode>> {
if let Some(Node::Directory { dir, .. }) = self.nodes.get(&inode) {
Some(&dir.children)
} else {
None
}
}

/// Get the name of an inode if it exists
#[tracing::instrument(skip(self))]
pub fn get_name(&self, inode: Inode) -> Option<&str> {
self.nodes
.get(&inode)
.map(|node| match node {
Node::Directory { name, .. } | Node::File { name, .. } => name,
})
.map(String::as_str)
}

/// Get the fuser file type of an inode if it exists
#[tracing::instrument(skip(self))]
pub fn get_file_type(&self, inode: Inode) -> Option<fuser::FileType> {
Expand Down Expand Up @@ -396,7 +377,7 @@ impl State {
let subdirectory_count = dir
.children
.iter()
.filter(|child_inode| {
.filter(|(_, child_inode)| {
matches!(self.nodes.get(child_inode), Some(Node::Directory { .. }))
})
.count();
Expand Down
Loading