Batch Git note edits and add a Cargo benchmark example

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<TreeEntry>,
+    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<Edit, Error> {
-        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<Edit, Error> {
-        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<ObjectId>,
-        objects: &(impl Find + Write),
-    ) -> Result<Edit, Error> {
+        objects: &impl Find,
+    ) -> Result<Option<ObjectId>, 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<ObjectId, Error> {
+        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
     }

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::<Result<Vec<_>, _>>()?;
+    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();

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"