Skip to content

Commit

Permalink
dumpfile: minor performance improvements
Browse files Browse the repository at this point in the history
Printing out the dumpfile seems to take ~50% of the total time of
running the `create-dumpfile` command, which is surprising.  Let's pick
off some low-hanging fruit by doing fewer allocations:

 - preallocate the string buffer we use for each line

 - use the push/pop API of PathBuf instead of creating a new PathBuf
   every time with .join().  This also solves the question of how we can
   consume the new PathBuf into the hardlinks table: we now definitely
   can't, so making a copy is now obviously correct.
  • Loading branch information
allisonkarlitskaya committed Oct 15, 2024
1 parent 9c1b0dc commit 2074f3a
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions src/dumpfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ struct DumpfileWriter<'a, W: Write> {
}

fn writeln_fmt(writer: &mut impl Write, f: impl Fn(&mut String) -> fmt::Result) -> Result<()> {
let mut tmp = String::new();
let mut tmp = String::with_capacity(256);
f(&mut tmp)?;
Ok(writeln!(writer, "{}", tmp)?)
}
Expand All @@ -181,7 +181,7 @@ impl<'a, W: Write> DumpfileWriter<'a, W> {
Self { hardlinks: HashMap::new(), writer }
}

fn write_dir(&mut self, path: &Path, dir: &Directory) -> Result<()> {
fn write_dir(&mut self, mut path: &mut PathBuf, dir: &Directory) -> Result<()> {
// nlink is 2 + number of subdirectories
// this is also true for the root dir since '..' is another self-ref
let nlink = dir.entries.iter().fold(2, |count, ent| count + {
Expand All @@ -194,21 +194,23 @@ impl<'a, W: Write> DumpfileWriter<'a, W> {
writeln_fmt(self.writer, |fmt| write_directory(fmt, path, &dir.stat, nlink))?;

for DirEnt { name, inode } in dir.entries.iter() {
let subpath = path.join(name);
path.push(name);

match inode {
Inode::Directory(ref dir) => {
self.write_dir(&subpath, dir)?;
self.write_dir(&mut path, dir)?;
},
Inode::Leaf(ref leaf) => {
self.write_leaf(subpath, leaf)?;
self.write_leaf(&path, leaf)?;
}
}

path.pop();
}
Ok(())
}

fn write_leaf(&mut self, path: PathBuf, leaf: &Rc<Leaf>) -> Result<()> {
fn write_leaf(&mut self, path: &Path, leaf: &Rc<Leaf>) -> Result<()> {
let nlink = Rc::strong_count(leaf);

if nlink > 1 {
Expand All @@ -217,8 +219,8 @@ impl<'a, W: Write> DumpfileWriter<'a, W> {
if let Some(target) = self.hardlinks.get(&ptr) {
return writeln_fmt(self.writer, |fmt| write_hardlink(fmt, &path, target));
}
// TODO: can we figure out a way to _move_ this into the hash
// table and keep a reference to it so we can do the print below?

// @path gets modified all the time, so take a copy
self.hardlinks.insert(ptr, OsString::from(&path));
}

Expand All @@ -227,5 +229,5 @@ impl<'a, W: Write> DumpfileWriter<'a, W> {
}

pub fn write_dumpfile<W: Write>(writer: &mut W, fs: &FileSystem) -> Result<()> {
DumpfileWriter::new(writer).write_dir(Path::new("/"), &fs.root)
DumpfileWriter::new(writer).write_dir(&mut PathBuf::from("/"), &fs.root)
}

0 comments on commit 2074f3a

Please sign in to comment.