From 24a2d7aaddb91ce281ea1979785085471caaaad2 Mon Sep 17 00:00:00 2001 From: Codex GPT-6 Date: Thu, 17 Sep 2026 16:33:39 +0100 Subject: [PATCH 1/2] feat(gix-note): batch staged note edits into a single tree write Creating many notes with repeated `State::replace()` calls serializes all materialized mappings after every edit. Add `State::edit()` to update retained state and `State::write()` to serialize once. Lookups observe pending edits, while immediate replacement and removal continue to flush. Failed writes recover the last saved root. Regression coverage checks zero writes during staging, cached reads, persisted replacements and removals, non-note preservation, no-op flushes, and recovery after failed writes. Validation with `GIX_TEST_IGNORE_ARCHIVES=1`: - `cargo test -p gix-note --all-features --locked --offline` (17 tests) - `cargo test -p gix --no-default-features --features sha1,notes --test gix --locked --offline repository::note::` (6 tests) --- gix-note/src/lib.rs | 85 +++++++++++++++++++++++++----------- gix-note/tests/note.rs | 97 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 26 deletions(-) diff --git a/gix-note/src/lib.rs b/gix-note/src/lib.rs index 4cf654451b2..27edabf6256 100644 --- a/gix-note/src/lib.rs +++ b/gix-note/src/lib.rs @@ -32,6 +32,8 @@ pub struct Edit { /// Use [`State::get()`], [`State::replace()`], and [`State::remove()`] to keep parsed /// tree entries and opened fanout subtrees in memory. A state starts at one root /// tree and advances to each tree produced by [`State::replace()`] or [`State::remove()`]. +/// For bulk changes, use [`State::edit()`] repeatedly and [`State::write()`] once to +/// avoid serializing all materialized notes after every edit. /// /// As in Git, the exact fanout can depend on which lazy subtrees were materialized. /// If an operation fails after changing materialized data, the state discards that @@ -41,6 +43,7 @@ pub struct State { root_tree_id: ObjectId, root: InternalNode, non_notes: Vec, + dirty: bool, } impl State { @@ -64,14 +67,16 @@ impl State { root_tree_id, root, non_notes, + dirty: false, }) } - /// Return the current root tree represented by this state. + /// Return the last written root tree represented by this state. /// /// This initially matches the ID passed to [`State::new()`]. After a successful /// [`Self::replace()`] or [`Self::remove()`], it matches the [`Edit::tree`] returned by that - /// operation. Lookups, failed operations, and removal of a missing note leave it unchanged. + /// operation. [`Self::edit()`] stages changes without advancing it; [`Self::write()`] + /// advances it to include those changes. Lookups and failed operations leave it unchanged. pub fn root_tree_id(&self) -> ObjectId { self.root_tree_id } @@ -93,7 +98,7 @@ impl State { /// Associate `note_blob_id` with `annotated_object_id`, replacing any existing note. /// /// Use `objects` to read and write the notes tree. Return the new root tree and - /// any previous note. + /// any previous note. This also writes changes staged by [`Self::edit()`]. /// /// The notes tree is rewritten with the same progressive fanout heuristic as /// Git while retaining entries that are not notes. Untouched fanout subtrees @@ -106,41 +111,54 @@ impl State { note_blob_id: ObjectId, objects: &(impl Find + Write), ) -> Result { - validate_replace_kinds( - self.root_tree_id.kind(), - annotated_object_id.kind(), - note_blob_id.kind(), - )?; - self.edit(annotated_object_id, Some(note_blob_id), objects) + let previous = self.edit(annotated_object_id, Some(note_blob_id), objects)?; + Ok(Edit { + tree: self.write(objects)?, + previous, + }) } /// Remove the note associated with `annotated_object_id`. /// /// Use `objects` to read and write the notes tree. Return the new root tree and - /// removed note. + /// removed note. This also writes changes staged by [`Self::edit()`]. /// - /// If there is no such note, the root is returned unchanged. + /// If there is no such note and no staged changes, the root is returned unchanged. pub fn remove(&mut self, annotated_object_id: ObjectId, objects: &(impl Find + Write)) -> Result { - validate_annotated_object_kind(self.root_tree_id.kind(), &annotated_object_id)?; - self.edit(annotated_object_id, None, objects) + let previous = self.edit(annotated_object_id, None, objects)?; + Ok(Edit { + tree: self.write(objects)?, + previous, + }) } - fn edit( + /// Stage a note replacement or removal without writing any objects, returning the previous note ID. + /// + /// `Some(note_blob_id)` associates that blob with `annotated_object_id`; `None` removes the mapping. + /// As with [`Self::replace()`], object hash kinds are validated but the note's object kind is not. + /// Lookups and subsequent edits see staged changes immediately. Call [`Self::write()`] to serialize + /// a batch of edits once; [`Self::root_tree_id()`] continues to return the last written tree until then. + /// If an operation fails after changing materialized data, all edits since the last successful write + /// are discarded along with that data. + pub fn edit( &mut self, annotated_object_id: ObjectId, note_blob_id: Option, - objects: &(impl Find + Write), - ) -> Result { + objects: &impl Find, + ) -> Result, Error> { + if let Some(note_blob_id) = note_blob_id { + validate_replace_kinds( + self.root_tree_id.kind(), + annotated_object_id.kind(), + note_blob_id.kind(), + )?; + } else { + validate_annotated_object_kind(self.root_tree_id.kind(), &annotated_object_id)?; + } self.reset_on_error(|state| { let previous_note_blob_id = state .root .remove(&annotated_object_id, 0, objects, &mut state.non_notes)?; - if note_blob_id.is_none() && previous_note_blob_id.is_none() { - return Ok(Edit { - tree: state.root_tree_id, - previous: previous_note_blob_id, - }); - } if let Some(note_blob_id) = note_blob_id { state.root.insert( Node::Note(Note { @@ -152,13 +170,27 @@ impl State { &mut state.non_notes, )?; } + state.dirty |= note_blob_id.is_some() || previous_note_blob_id.is_some(); + Ok(previous_note_blob_id) + }) + } + + /// Write all staged edits to `objects` and return the resulting root tree ID. + /// + /// Materialized notes remain cached for subsequent lookups and edits. The tree uses the same + /// progressive fanout and non-note preservation as [`Self::replace()`]. If there are no staged + /// changes, return the current root without writing any objects. On failure, discard staged edits + /// and recover from the last successfully written root. + pub fn write(&mut self, objects: &(impl Find + Write)) -> Result { + if !self.dirty { + return Ok(self.root_tree_id); + } + self.reset_on_error(|state| { state.root_tree_id = state .root .write(&mut state.non_notes, state.root_tree_id.kind(), objects)?; - Ok(Edit { - tree: state.root_tree_id, - previous: previous_note_blob_id, - }) + state.dirty = false; + Ok(state.root_tree_id) }) } @@ -174,6 +206,7 @@ impl State { tree_id: root_tree_id, }))); self.non_notes.clear(); + self.dirty = false; } result } diff --git a/gix-note/tests/note.rs b/gix-note/tests/note.rs index e1e1dc301f8..87b1a49e2ab 100644 --- a/gix-note/tests/note.rs +++ b/gix-note/tests/note.rs @@ -269,6 +269,103 @@ fn state_reuses_materialized_trees_across_operations() -> gix_testtools::Result Ok(()) } +#[test] +fn staged_edits_reuse_state_without_writing_trees() -> gix_testtools::Result { + let kind = gix_testtools::object_hash(); + let objects = CountingObjectDb::new(kind); + let note_blob_id = objects.write_buf(gix_object::Kind::Blob, b"note")?; + let replacement_blob_id = objects.write_buf(gix_object::Kind::Blob, b"replacement")?; + let root_tree_id = objects.write(&Tree { + entries: vec![Entry { + mode: EntryKind::Blob.into(), + filename: "metadata".into(), + oid: note_blob_id, + }], + })?; + let mut state = gix_note::State::new(root_tree_id, &objects)?; + let annotated_object_ids = (0..512_u32) + .map(|index| gix_object::compute_hash(kind, gix_object::Kind::Blob, &index.to_le_bytes())) + .collect::, _>>()?; + objects.writes.set(0); + for &annotated_object_id in &annotated_object_ids { + assert_eq!( + state.edit(annotated_object_id, Some(note_blob_id), &objects)?, + None, + "each staged mapping is new" + ); + assert_eq!( + state.get(&annotated_object_id, &objects)?, + Some(note_blob_id), + "lookups see staged mappings" + ); + } + assert_eq!( + state.edit(annotated_object_ids[0], Some(replacement_blob_id), &objects)?, + Some(note_blob_id), + "a staged replacement returns the previous note" + ); + assert_eq!( + state.edit(annotated_object_ids[1], None, &objects)?, + Some(note_blob_id), + "a staged removal returns the previous note" + ); + assert_eq!(state.root_tree_id(), root_tree_id, "edits retain the last written root"); + assert_eq!(objects.writes.get(), 0, "staging a batch writes no trees"); + assert_eq!(objects.reads.get(), 1, "the whole batch reuses the initialized root"); + + let written_tree_id = state.write(&objects)?; + let writes = objects.writes.get(); + assert!(writes > 1, "writing the batch creates fanout trees"); + assert_eq!( + state.write(&objects)?, + written_tree_id, + "writing without edits is a no-op" + ); + assert_eq!(objects.writes.get(), writes, "a clean state writes no objects"); + let mut persisted = gix_note::State::new(written_tree_id, &objects)?; + for (index, annotated_object_id) in annotated_object_ids.iter().enumerate() { + assert_eq!( + persisted.get(annotated_object_id, &objects)?, + match index { + 0 => Some(replacement_blob_id), + 1 => None, + _ => Some(note_blob_id), + }, + "the written tree contains the final batch" + ); + } + assert_entry_at_path( + &objects.inner, + written_tree_id, + &["metadata"], + EntryKind::Blob, + note_blob_id, + )?; + + state.edit(annotated_object_ids[0], Some(note_blob_id), &objects)?; + objects.fail_next_write.set(true); + state.write(&objects).expect_err("the injected tree write fails"); + assert_eq!( + state.root_tree_id(), + written_tree_id, + "a failed flush retains the saved root" + ); + assert_eq!( + state.get(&annotated_object_ids[0], &objects)?, + Some(replacement_blob_id), + "a failed flush discards pending changes" + ); + state.edit(annotated_object_ids[0], Some(note_blob_id), &objects)?; + let edit = state.remove(annotated_object_ids[1], &objects)?; + assert_eq!(edit.previous, None, "the removed note was already absent"); + assert_eq!( + one_shot::get(edit.tree, &annotated_object_ids[0], &objects)?, + Some(note_blob_id), + "an immediate operation also flushes previously staged edits" + ); + Ok(()) +} + #[test] fn state_recovers_after_failed_operations() -> gix_testtools::Result { let kind = gix_testtools::object_hash(); From 6b536ed7674861204a6cff0cc110176bfd24af9f Mon Sep 17 00:00:00 2001 From: Codex GPT-6 Date: Thu, 17 Sep 2026 16:41:21 +0100 Subject: [PATCH 2/2] Add the notes benchmark as a `gix` Cargo example Register `gix-notes-bench` as a normal `gix` example gated by `notes` so Cargo example builds and Clippy cover it. It uses the staged notes API to create one note per commit-graph entry, packs the final objects, verifies them through a fresh object database, and only then creates an unused notes ref. Enable `gix-pack`'s `generate` and `streaming-input` features in a dev-dependency using the existing version and path. This supplies the features the standalone package previously got from `gix-pack` defaults without enabling them for normal library consumers. Validation: - `cargo check -p gix --examples --locked --offline` - `cargo clippy -p gix --no-default-features --features sha1,notes --example gix-notes-bench --locked --offline -- -D warnings` - `cargo run -p gix --no-default-features --features sha1,notes --example gix-notes-bench -- --help` - `cargo fmt --all -- --check` The normal library dependency graph with `sha1,notes` enables only `object-cache-dynamic,sha1` on `gix-pack`. The removed-Clippy-lint warning comes from existing workspace configuration. --- gix/Cargo.toml | 9 ++ gix/examples/gix-notes-bench.rs | 221 ++++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 gix/examples/gix-notes-bench.rs diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 31aa32ac313..73534a643e1 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -22,6 +22,11 @@ name = "clone" path = "examples/clone.rs" required-features = ["blocking-network-client"] +[[example]] +name = "gix-notes-bench" +path = "examples/gix-notes-bench.rs" +required-features = ["notes"] + [features] default = ["max-performance-safe", "comfort", "basic", "extras", "auto-chain-error", "sha1"] @@ -424,6 +429,10 @@ gix = { path = ".", default-features = false, features = [ "merge", "tree-error", "sha1", "sha256" ] } gix-hash = { version = "^0.26.2", path = "../gix-hash" } +gix-pack = { version = "^0.74.2", path = "../gix-pack", default-features = false, features = [ + "generate", + "streaming-input", +] } pretty_assertions = "1.4.0" gix-testtools = { path = "../tests/tools", features = ["sha1", "sha256"] } is_ci = "1.1.1" diff --git a/gix/examples/gix-notes-bench.rs b/gix/examples/gix-notes-bench.rs new file mode 100644 index 00000000000..e23d992ae90 --- /dev/null +++ b/gix/examples/gix-notes-bench.rs @@ -0,0 +1,221 @@ +//! Benchmark creating, packing, and reading one Git note per commit in an existing commit-graph. +//! +//! Run from the workspace root, selecting a repository with a commit-graph and an unused notes ref: +//! +//! ```sh +//! cargo run --release -p gix --no-default-features --features sha1,notes \ +//! --example gix-notes-bench -- /path/to/repository all refs/notes/gix-notes-bench +//! ``` +//! +//! Replace `all` with a positive limit for a smaller run. Each note contains the annotated commit's +//! hexadecimal ID followed by a newline. One retained notes state stages all edits before serializing +//! the final trees and a single notes commit into memory. Packing includes compression, index creation, +//! integrity verification, and syncing both files. Reading verifies every note using a fresh ODB and +//! notes state with a warm OS file cache. The destination ref is published only after verification. + +use std::{ + fs::File, + io::Cursor, + path::PathBuf, + sync::atomic::AtomicBool, + time::{Duration, Instant}, +}; + +use gix::{ + note::plumbing::State, + objs::{FindExt, Kind, Write}, +}; +use gix_pack::data::output::{self, bytes::FromEntriesIter}; + +type Result = std::result::Result>; +type Memory = gix::odb::memory::Proxy; +const NOTES_REF: &str = "refs/notes/gix-notes-bench"; + +fn main() -> Result<()> { + let mut args = std::env::args_os().skip(1); + let repository = args.next().map_or_else(|| ".".into(), PathBuf::from); + if repository.as_os_str() == "--help" { + println!("Usage: gix-notes-bench [repository=.] [limit=all] [ref={NOTES_REF}]"); + println!("Annotate every commit in the existing commit-graph; refuse an existing notes ref."); + return Ok(()); + } + let limit = args + .next() + .filter(|arg| arg != "all") + .map(|arg| arg.to_string_lossy().parse::()) + .transpose()? + .unwrap_or(usize::MAX); + let notes_ref = args + .next() + .map(|arg| arg.into_string().map_err(|_| "the notes ref must be UTF-8")) + .transpose()? + .unwrap_or_else(|| NOTES_REF.into()); + if args.next().is_some() || limit == 0 || !notes_ref.starts_with("refs/notes/") { + return Err("expected a repository, a positive limit, and a ref under refs/notes/".into()); + } + + let setup_start = Instant::now(); + let repo = gix::open(&repository)?; + if repo.try_find_reference(notes_ref.as_str())?.is_some() { + return Err(format!("{notes_ref} already exists; choose a new notes ref").into()); + } + let hash = repo.object_hash(); + let graph = repo.commit_graph()?; + let mut commit_ids: Vec<_> = graph.iter_ids().map(ToOwned::to_owned).collect(); + // Graph files are OID-sorted. Sorting by the suffix spreads edits over the notes fanout. + commit_ids.sort_unstable_by(|a, b| a.as_bytes()[8..].cmp(&b.as_bytes()[8..])); + commit_ids.truncate(limit); + if commit_ids.is_empty() { + return Err("the commit-graph is empty".into()); + } + let count = commit_ids.len(); + println!("repository: {}", repo.git_dir().display()); + println!("commits: {count} (graph contains {})", graph.num_commits()); + println!("payload: commit ID plus newline ({} bytes/note)", hash.len_in_hex() + 1); + println!("setup_seconds: {:.6}", setup_start.elapsed().as_secs_f64()); + drop(graph); + + let write_start = Instant::now(); + let mut objects = Memory::new(gix::objs::find::Never, hash); + let empty_tree_id = objects.write_buf(Kind::Tree, &[])?; + let mut state = State::new(empty_tree_id, &objects).map_err(gix::Exn::into_error)?; + let mut payload = vec![b'\n'; hash.len_in_hex() + 1]; + let mut last_report = Instant::now(); + for (index, &commit_id) in commit_ids.iter().enumerate() { + let _ = commit_id.hex_to_buf(&mut payload[..hash.len_in_hex()]); + let note_blob_id = objects.write_buf(Kind::Blob, &payload)?; + let previous = state + .edit(commit_id, Some(note_blob_id), &objects) + .map_err(gix::Exn::into_error)?; + assert!(previous.is_none(), "each graph commit must be annotated exactly once"); + if (index + 1).is_multiple_of(10_000) && last_report.elapsed() >= Duration::from_secs(5) { + eprintln!( + "created {}/{count} notes in {:.1}s", + index + 1, + write_start.elapsed().as_secs_f64() + ); + last_report = Instant::now(); + } + } + // Serialize the retained notes tree once, into the same in-memory object database. + let root_tree_id = state.write(&objects).map_err(gix::Exn::into_error)?; + drop(state); + let signature = gix::actor::Signature { + name: "gix-notes-bench".into(), + email: "gix-notes-bench@example.com".into(), + time: gix::date::Time::now_utc(), + }; + let notes_commit_id = objects.write(&gix::objs::Commit { + tree: root_tree_id, + parents: Default::default(), + author: signature.clone(), + committer: signature, + encoding: None, + message: format!("Attach benchmark notes to {count} commit-graph commits\n").into(), + extra_headers: Vec::new(), + })?; + let create_time = write_start.elapsed(); + report("create_in_memory", create_time, count); + + let pack_start = Instant::now(); + let mut storage = objects.take_object_memory().expect("object memory is enabled"); + drop(objects); + // The bootstrap empty tree is the only superseded object; all other objects are final. + storage.remove(&empty_tree_id); + let object_count = u32::try_from(storage.len())?; + let blob_count = storage.values().filter(|(kind, _)| *kind == Kind::Blob).count(); + assert_eq!(blob_count, count, "each commit has a distinct note blob"); + println!("packed_objects: {object_count}"); + println!( + "object_bytes: {}", + storage.values().map(|(_, data)| data.len()).sum::() + ); + let entries = storage.drain().map(|(object_id, (kind, data))| { + output::Entry::from_data( + &output::Count::from_data(object_id, None), + &gix::objs::Data { + kind, + data: &data, + object_hash: hash, + }, + gix::zlib::Compression::DEFAULT, + ) + .map(|entry| vec![entry]) + }); + let mut encoder = FromEntriesIter::new( + entries, + Vec::new(), + object_count, + gix_pack::data::Version::default(), + hash, + ); + for chunk in encoder.by_ref() { + chunk?; + } + let pack = encoder.into_write(); + drop(storage); + println!("pack_bytes: {}", pack.len()); + let pack_dir = repo.objects.store().path().join("pack"); + let outcome = gix_pack::Bundle::write_to_directory( + &mut Cursor::new(pack), + Some(&pack_dir), + &mut gix::progress::Discard, + &AtomicBool::new(false), + None::, + hash, + gix_pack::bundle::write::Options { + thread_limit: Some(1), + ..Default::default() + }, + )?; + let pack_path = outcome.data_path.as_ref().ok_or("no pack was written")?; + let index_path = outcome.index_path.as_ref().ok_or("no pack index was written")?; + File::open(pack_path)?.sync_all()?; + File::open(index_path)?.sync_all()?; + let pack_time = pack_start.elapsed(); + report("pack_and_index", pack_time, count); + report("write_total", create_time + pack_time, count); + println!("pack: {}", pack_path.display()); + println!("notes_commit: {notes_commit_id}"); + + // Read through a fresh on-disk ODB, with no in-memory objects or parsed notes state. + // The operating system's file cache is intentionally warm from writing the pack. + let read_start = Instant::now(); + let disk = gix::odb::at(repo.objects.store().path(), hash)?; + let mut buffer = Vec::new(); + let persisted_tree_id = disk.find_commit(¬es_commit_id, &mut buffer)?.tree(); + assert_eq!( + persisted_tree_id, root_tree_id, + "the packed commit references the final notes tree" + ); + let mut state = State::new(persisted_tree_id, &disk).map_err(gix::Exn::into_error)?; + for commit_id in &commit_ids { + let note_blob_id = state + .get(commit_id, &disk) + .map_err(gix::Exn::into_error)? + .ok_or_else(|| format!("missing note for {commit_id}"))?; + let actual = disk.find_blob(¬e_blob_id, &mut buffer)?; + let _ = commit_id.hex_to_buf(&mut payload[..hash.len_in_hex()]); + assert_eq!(actual.data, payload, "note payload for {commit_id}"); + } + report("read_and_verify", read_start.elapsed(), count); + + // Publish only after the complete pack round-trip succeeds. Never replace an existing ref. + repo.reference( + notes_ref.as_str(), + notes_commit_id, + gix::refs::transaction::PreviousValue::MustNotExist, + "gix-notes benchmark", + )?; + if let Some(keep_path) = outcome.keep_path { + std::fs::remove_file(keep_path)?; + } + println!("verified_notes: {count}"); + println!("notes_ref: {notes_ref}"); + Ok(()) +} + +fn report(phase: &str, elapsed: Duration, count: usize) { + println!("{phase}_seconds: {:.6}", elapsed.as_secs_f64()); + println!("{phase}_notes_per_second: {:.0}", count as f64 / elapsed.as_secs_f64()); +}