git-ents.gitmain
⌘K
foforge
commit 6cb99f7
perf: parallelize toolchain import's tree walk across cores

build_tree wrote one loose object per file, sequentially, single-threaded; for a large import (rustup’s lib/rustlib/src/rust alone is tens of thousands of files) that per-object filesystem syscall overhead dominated wall time, not hashing or compression (already at zlib’s fastest level). Each directory’s entries are now written in parallel via rayon, one gix::odb::Handle clone per entry (Handle holds RefCell`s and so is `Send but not Sync — it can’t be shared by reference across threads, but cloning it per entry before the parallel fan-out is the intended way to use it from more than one thread). A full rustup --embed import dropped from timing out past 5 minutes to ~40s wall time on 4 cores, producing the exact same tree id as before.

feat: parallelize `build_tree’s per-entry writes with rayon Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

Cargo.lock @@ -771,6 +771,25 @@ "crossbeam-utils", ] +[[package]] +name = "crossbeam-deque" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9dd111b7b7f7d55b72c0a6ae361660ee5853c9af73f70c3c2ef6858b950e2e51" +dependencies = [ + "crossbeam-epoch", + "crossbeam-utils", +] + +[[package]] +name = "crossbeam-epoch" +version = "0.9.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "crossbeam-utils" version = "0.8.21" @@ -993,6 +1012,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d0881ea181b1df73ff77ffaaf9c7544ecc11e82fba9b5f27b262a3c73a332555" +[[package]] +name = "either" +version = "1.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91622ff5e7162018101f2fea40d6ebf4a78bbe5a49736a2020649edf9693679e" + [[package]] name = "encode_unicode" version = "1.0.0" @@ -1518,6 +1543,7 @@ "facet-git-tree", "git-store", "gix", + "rayon", "semver", "spdx", "target-lexicon", @@ -3369,6 +3395,26 @@ "getrandom 0.3.4", ] +[[package]] +name = "rayon" +version = "1.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb39b166781f92d482534ef4b4b1b2568f42613b53e5b6c160e24cfbfa30926d" +dependencies = [ + "either", + "rayon-core", +] + +[[package]] +name = "rayon-core" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22e18b0f0062d30d4230b2e85ff77fdfe4326feb054b9783a3460d8435c8ab91" +dependencies = [ + "crossbeam-deque", + "crossbeam-utils", +] + [[package]] name = "redox_syscall" version = "0.5.18"
Cargo.toml @@ -65,6 +65,7 @@ iddqd = "0.4" maud = { version = "0.27", features = ["axum"] } pulldown-cmark = { version = "0.13", default-features = false, features = ["html"] } +rayon = "1" rstest = "0.26" semver = "1" spdx = "0.13"
crates/git-toolchain/Cargo.toml @@ -10,6 +10,7 @@ facet-git-tree = { workspace = true } git-store = { workspace = true } gix = { workspace = true } +rayon = { workspace = true } semver = { workspace = true } spdx = { workspace = true } target-lexicon = { workspace = true }
crates/git-toolchain/src/lib.rs @@ -35,6 +35,7 @@ use gix::bstr::ByteSlice as _; use gix::objs::tree::{Entry as TreeEntry, EntryKind, EntryMode}; use gix::objs::{FindExt as _, Tree, Write as _}; +use rayon::prelude::*; /// The ref namespace holding toolchains, one ref per toolchain: /// `refs/meta/toolchains/<name>`. A toolchain's identity is its tip commit's @@ -366,31 +367,53 @@ } /// Build `dir`'s tree bottom-up: a directory's own entries are all resolved -/// (recursing into subdirectories, writing files and symlinks as blobs) -/// before its own tree object is written, so every child is already an -/// object id by the time its parent's entry list is sorted and written. +/// in parallel (recursing into subdirectories, writing files and symlinks as +/// blobs) before its own tree object is written, so every child is already +/// an object id by the time its parent's entry list is sorted and written. +/// +/// A large import (a rustup sysroot's `lib/rustlib/src/rust` alone is tens +/// of thousands of files) is dominated by per-object filesystem syscall +/// overhead, not by hashing or compression (gix's loose-object writer already +/// runs zlib at its fastest level); fanning the write out across every core +/// is the lever that actually matters here. `gix::odb::Handle` holds +/// `RefCell`s and so is `Send` but not `Sync` — it cannot be shared by +/// reference across threads — but it is cheaply `Clone` (an `Arc`-backed +/// handle to the same store), which is the intended way to use one per +/// thread; each entry gets its own clone before the parallel fan-out so no +/// two threads ever touch the same `Handle`. fn build_tree(odb: &gix::odb::Handle, dir: &Path) -> Result<Tree, Error> { - let mut entries = Vec::new(); let read_dir = fs::read_dir(dir).map_err(|error| Error::Io(dir.to_owned(), error))?; - for item in read_dir { - let item = item.map_err(|error| Error::Io(dir.to_owned(), error))?; - let path = item.path(); - let name = item - .file_name() - .into_string() - .map_err(|_name| Error::NotUtf8(path.clone()))?; - let file_type = item - .file_type() - .map_err(|error| Error::Io(path.clone(), error))?; - let Some((oid, mode)) = write_entry(odb, &path, file_type)? else { - continue; - }; - entries.push(TreeEntry { - mode, - filename: name.into(), - oid, - }); - } + let items = read_dir + .collect::<Result<Vec<_>, _>>() + .map_err(|error| Error::Io(dir.to_owned(), error))?; + + let mut entries: Vec<TreeEntry> = items + .into_iter() + .map(|item| (item, odb.clone())) + .collect::<Vec<_>>() + .into_par_iter() + .map(|(item, odb)| -> Result<Option<TreeEntry>, Error> { + let path = item.path(); + let name = item + .file_name() + .into_string() + .map_err(|_name| Error::NotUtf8(path.clone()))?; + let file_type = item + .file_type() + .map_err(|error| Error::Io(path.clone(), error))?; + let Some((oid, mode)) = write_entry(&odb, &path, file_type)? else { + return Ok(None); + }; + Ok(Some(TreeEntry { + mode, + filename: name.into(), + oid, + })) + }) + .collect::<Result<Vec<Option<TreeEntry>>, Error>>()? + .into_iter() + .flatten() + .collect(); entries.sort(); Ok(Tree { entries }) }