Conversation
|
Reviews in this chain: |
| _name: &OsStr, | ||
| _size: u32, | ||
| name: &OsStr, | ||
| size: u32, |
There was a problem hiding this comment.
32 bits for size seems a bit conservative... 4GiB is not that uncommon for file sizes nowadays, though probably would be a problem due to other reasons in the sandbox agent
There was a problem hiding this comment.
I don't think it's possible to have single syscall with bigger size. I think FUSE is typed for u32 🤔 At least it is in this codebase
| SyscallArgs::Lookup { | ||
| parent, | ||
| name: name.to_string(), | ||
| parent_path: parent_path.map(String::from), |
There was a problem hiding this comment.
We could probably get away without these extra allocations - after all, these strings have to exist somewhere already and we just want to "print" them.
The SyscallArgs variants currently allocate String for every trace event, but the data is already available as borrowed &str:
pathcomes fromself.path_for_inode(ino)→Option<&str>borrowed from theinode_to_nodeHashMapnamecomes fromname.to_string_lossy()→Cow<'_, str>(zero-copy for valid UTF-8)
Since trace() serializes and sends immediately, the borrows just need to live through that call. Adding a lifetime to the types eliminates all heap allocations:
#[derive(Debug, Serialize)]
pub enum SyscallArgs<'a> {
Lookup {
parent: u64,
name: &'a str,
#[serde(skip_serializing_if = "Option::is_none")]
parent_path: Option<&'a str>,
},
Getattr {
ino: u64,
fh: Option<u64>,
#[serde(skip_serializing_if = "Option::is_none")]
path: Option<&'a str>,
},
// ... same for all variants
}
#[derive(Debug, Serialize)]
pub struct SyscallEvent<'a> {
pub syscall: &'static str,
pub timestamp_ns: u128,
#[serde(flatten)]
pub args: SyscallArgs<'a>,
}
Then the convenience methods just pass the borrows through with no .to_string():
pub fn lookup(&self, parent: u64, name: &str, parent_path: Option<&str>) {
self.trace("lookup", SyscallArgs::Lookup { parent, name, parent_path });
}
Zero heap allocations per trace event. The only allocation is the output JSON buffer in serde_json::to_vec, which is unavoidable.
There was a problem hiding this comment.
Thanks, i've been living under a rock and never actually seen rust to this day, so a bit of learning like that helps ;)
657659b to
0af1c43
Compare
No description provided.