Skip to content

Commit

Permalink
change!: Use gix-object::Find trait
Browse files Browse the repository at this point in the history
  • Loading branch information
@Byron
Byron committed Nov 4, 2023
1 parent 2637f8f commit d6e7201
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 167 deletions.
22 changes: 9 additions & 13 deletions gix-pack/src/bundle/write/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ pub use types::{Options, Outcome};

use crate::bundle::write::types::SharedTempFile;

type ThinPackLookupFn = Box<dyn for<'a> FnMut(gix_hash::ObjectId, &'a mut Vec<u8>) -> Option<gix_object::Data<'a>>>;
type ThinPackLookupFnSend =
Box<dyn for<'a> FnMut(gix_hash::ObjectId, &'a mut Vec<u8>) -> Option<gix_object::Data<'a>> + Send + 'static>;

/// The progress ids used in [`write_to_directory()`][crate::Bundle::write_to_directory()].
///
/// Use this information to selectively extract the progress of interest in case the parent application has custom visualization.
Expand Down Expand Up @@ -54,7 +50,7 @@ impl crate::Bundle {
///
/// * `progress` provides detailed progress information which can be discarded with [`gix_features::progress::Discard`].
/// * `should_interrupt` is checked regularly and when true, the whole operation will stop.
/// * `thin_pack_base_object_lookup_fn` If set, we expect to see a thin-pack with objects that reference their base object by object id which is
/// * `thin_pack_base_object_lookup` If set, we expect to see a thin-pack with objects that reference their base object by object id which is
/// expected to exist in the object database the bundle is contained within.
/// `options` further configure how the task is performed.
///
Expand All @@ -68,7 +64,7 @@ impl crate::Bundle {
directory: Option<&Path>,
progress: &mut dyn DynNestedProgress,
should_interrupt: &AtomicBool,
thin_pack_base_object_lookup_fn: Option<ThinPackLookupFn>,
thin_pack_base_object_lookup: Option<impl gix_object::Find>,
options: Options,
) -> Result<Outcome, Error> {
let _span = gix_features::trace::coarse!("gix_pack::Bundle::write_to_directory()");
Expand All @@ -90,8 +86,8 @@ impl crate::Bundle {
let (pack_entries_iter, pack_version): (
Box<dyn Iterator<Item = Result<data::input::Entry, data::input::Error>>>,
_,
) = match thin_pack_base_object_lookup_fn {
Some(thin_pack_lookup_fn) => {
) = match thin_pack_base_object_lookup {
Some(thin_pack_lookup) => {
let pack = interrupt::Read {
inner: pack,
should_interrupt,
Expand All @@ -104,7 +100,7 @@ impl crate::Bundle {
data::input::EntryDataMode::KeepAndCrc32,
object_hash,
)?,
thin_pack_lookup_fn,
thin_pack_lookup,
);
let pack_version = pack_entries_iter.inner.version();
let pack_entries_iter = data::input::EntriesToBytesIter::new(
Expand Down Expand Up @@ -178,7 +174,7 @@ impl crate::Bundle {
directory: Option<impl AsRef<Path>>,
progress: &mut dyn DynNestedProgress,
should_interrupt: &'static AtomicBool,
thin_pack_base_object_lookup_fn: Option<ThinPackLookupFnSend>,
thin_pack_base_object_lookup: Option<impl gix_object::Find + Send + 'static>,
options: Options,
) -> Result<Outcome, Error> {
let _span = gix_features::trace::coarse!("gix_pack::Bundle::write_to_directory_eagerly()");
Expand All @@ -198,8 +194,8 @@ impl crate::Bundle {
let (pack_entries_iter, pack_version): (
Box<dyn Iterator<Item = Result<data::input::Entry, data::input::Error>> + Send + 'static>,
_,
) = match thin_pack_base_object_lookup_fn {
Some(thin_pack_lookup_fn) => {
) = match thin_pack_base_object_lookup {
Some(thin_pack_lookup) => {
let pack = interrupt::Read {
inner: pack,
should_interrupt,
Expand All @@ -212,7 +208,7 @@ impl crate::Bundle {
data::input::EntryDataMode::KeepAndCrc32,
object_hash,
)?,
thin_pack_lookup_fn,
thin_pack_lookup,
);
let pack_kind = pack_entries_iter.inner.version();
(Box::new(pack_entries_iter), pack_kind)
Expand Down
16 changes: 8 additions & 8 deletions gix-pack/src/data/input/lookup_ref_delta_objects.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ use gix_hash::ObjectId;
use crate::data::{entry::Header, input};

/// An iterator to resolve thin packs on the fly.
pub struct LookupRefDeltaObjectsIter<I, LFn> {
pub struct LookupRefDeltaObjectsIter<I, Find> {
/// The inner iterator whose entries we will resolve.
pub inner: I,
lookup: LFn,
lookup: Find,
/// The cached delta to provide next time we are called, it's the delta to go with the base we just resolved in its place.
next_delta: Option<input::Entry>,
/// Fuse to stop iteration after first missing object.
Expand All @@ -21,14 +21,14 @@ pub struct LookupRefDeltaObjectsIter<I, LFn> {
buf: Vec<u8>,
}

impl<I, LFn> LookupRefDeltaObjectsIter<I, LFn>
impl<I, Find> LookupRefDeltaObjectsIter<I, Find>
where
I: Iterator<Item = Result<input::Entry, input::Error>>,
LFn: for<'a> FnMut(ObjectId, &'a mut Vec<u8>) -> Option<gix_object::Data<'a>>,
Find: gix_object::Find,
{
/// Create a new instance wrapping `iter` and using `lookup` as function to retrieve objects that will serve as bases
/// for ref deltas seen while traversing `iter`.
pub fn new(iter: I, lookup: LFn) -> Self {
pub fn new(iter: I, lookup: Find) -> Self {
LookupRefDeltaObjectsIter {
inner: iter,
lookup,
Expand Down Expand Up @@ -75,10 +75,10 @@ where
}
}

impl<I, LFn> Iterator for LookupRefDeltaObjectsIter<I, LFn>
impl<I, Find> Iterator for LookupRefDeltaObjectsIter<I, Find>
where
I: Iterator<Item = Result<input::Entry, input::Error>>,
LFn: for<'a> FnMut(ObjectId, &'a mut Vec<u8>) -> Option<gix_object::Data<'a>>,
Find: gix_object::Find,
{
type Item = Result<input::Entry, input::Error>;

Expand All @@ -94,7 +94,7 @@ where
Header::RefDelta { base_id } => {
match self.inserted_entry_length_at_offset.iter().rfind(|e| e.oid == base_id) {
None => {
let base_entry = match (self.lookup)(base_id, &mut self.buf) {
let base_entry = match self.lookup.try_find(&base_id, &mut self.buf).ok()? {
Some(obj) => {
let current_pack_offset = entry.pack_offset;
let mut entry = match input::Entry::from_data_obj(&obj, 0) {
Expand Down
126 changes: 91 additions & 35 deletions gix-pack/src/data/output/count/objects/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,11 @@ pub fn objects_unthreaded(
}

mod expand {
use std::cell::RefCell;
use std::sync::atomic::{AtomicBool, Ordering};

use gix_hash::{oid, ObjectId};
use gix_object::{CommitRefIter, TagRefIter};
use gix_object::{CommitRefIter, Data, TagRefIter};

use super::{
tree,
Expand Down Expand Up @@ -206,24 +207,15 @@ mod expand {

let objects_ref = if parent_commit_ids.is_empty() {
traverse_delegate.clear();
let objects = ExpandedCountingObjects::new(db, out, objects);
gix_traverse::tree::breadthfirst(
current_tree_iter,
&mut tree_traversal_state,
|oid, buf| {
stats.decoded_objects += 1;
match db.find(oid, buf).ok() {
Some((obj, location)) => {
objects.fetch_add(1, Ordering::Relaxed);
stats.expanded_objects += 1;
out.push(output::Count::from_data(oid, location));
obj.try_into_tree_iter()
}
None => None,
}
},
&objects,
&mut traverse_delegate,
)
.map_err(Error::TreeTraverse)?;
out = objects.dissolve(stats);
&traverse_delegate.non_trees
} else {
for commit_id in &parent_commit_ids {
Expand Down Expand Up @@ -252,17 +244,16 @@ mod expand {
};

changes_delegate.clear();
let objects = CountingObjects::new(db);
gix_diff::tree::Changes::from(Some(parent_tree))
.needed_to_obtain(
current_tree_iter.clone(),
&mut tree_diff_state,
|oid, buf| {
stats.decoded_objects += 1;
db.find_tree_iter(oid, buf).map(|t| t.0)
},
&objects,
&mut changes_delegate,
)
.map_err(Error::TreeChanges)?;
stats.decoded_objects += objects.into_count();
}
&changes_delegate.objects
};
Expand All @@ -283,24 +274,17 @@ mod expand {
match obj.0.kind {
Tree => {
traverse_delegate.clear();
gix_traverse::tree::breadthfirst(
gix_object::TreeRefIter::from_bytes(obj.0.data),
&mut tree_traversal_state,
|oid, buf| {
stats.decoded_objects += 1;
match db.find(oid, buf).ok() {
Some((obj, location)) => {
objects.fetch_add(1, Ordering::Relaxed);
stats.expanded_objects += 1;
out.push(output::Count::from_data(oid, location));
obj.try_into_tree_iter()
}
None => None,
}
},
&mut traverse_delegate,
)
.map_err(Error::TreeTraverse)?;
{
let objects = ExpandedCountingObjects::new(db, out, objects);
gix_traverse::tree::breadthfirst(
gix_object::TreeRefIter::from_bytes(obj.0.data),
&mut tree_traversal_state,
&objects,
&mut traverse_delegate,
)
.map_err(Error::TreeTraverse)?;
out = objects.dissolve(stats);
}
for id in &traverse_delegate.non_trees {
out.push(id_to_count(db, buf1, id, objects, stats, allow_pack_lookups));
}
Expand Down Expand Up @@ -374,4 +358,76 @@ mod expand {
},
}
}

struct CountingObjects<'a> {
decoded_objects: std::cell::RefCell<usize>,
objects: &'a dyn crate::Find,
}

impl<'a> CountingObjects<'a> {
fn new(objects: &'a dyn crate::Find) -> Self {
Self {
decoded_objects: Default::default(),
objects,
}
}

fn into_count(self) -> usize {
self.decoded_objects.into_inner()
}
}

impl gix_object::Find for CountingObjects<'_> {
fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec<u8>) -> Result<Option<Data<'a>>, gix_object::find::Error> {
let res = Ok(self.objects.try_find(id, buffer)?.map(|t| t.0));
*self.decoded_objects.borrow_mut() += 1;
res
}
}

struct ExpandedCountingObjects<'a> {
decoded_objects: std::cell::RefCell<usize>,
expanded_objects: std::cell::RefCell<usize>,
out: std::cell::RefCell<Vec<output::Count>>,
objects_count: &'a gix_features::progress::AtomicStep,
objects: &'a dyn crate::Find,
}

impl<'a> ExpandedCountingObjects<'a> {
fn new(
objects: &'a dyn crate::Find,
out: Vec<output::Count>,
objects_count: &'a gix_features::progress::AtomicStep,
) -> Self {
Self {
decoded_objects: Default::default(),
expanded_objects: Default::default(),
out: RefCell::new(out),
objects_count,
objects,
}
}

fn dissolve(self, stats: &mut Outcome) -> Vec<output::Count> {
stats.decoded_objects += self.decoded_objects.into_inner();
stats.expanded_objects += self.expanded_objects.into_inner();
self.out.into_inner()
}
}

impl gix_object::Find for ExpandedCountingObjects<'_> {
fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec<u8>) -> Result<Option<Data<'a>>, gix_object::find::Error> {
let maybe_obj = self.objects.try_find(id, buffer)?;
*self.decoded_objects.borrow_mut() += 1;
match maybe_obj {
None => Ok(None),
Some((obj, location)) => {
self.objects_count.fetch_add(1, Ordering::Relaxed);
*self.expanded_objects.borrow_mut() += 1;
self.out.borrow_mut().push(output::Count::from_data(id, location));
Ok(Some(obj))
}
}
}
}
}
2 changes: 1 addition & 1 deletion gix-pack/src/data/output/count/objects/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub enum Error {
#[error(transparent)]
CommitDecode(gix_object::decode::Error),
#[error(transparent)]
FindExisting(#[from] crate::find::existing::Error),
FindExisting(#[from] gix_object::find::existing::Error),
#[error(transparent)]
InputIteration(Box<dyn std::error::Error + Send + Sync + 'static>),
#[error(transparent)]
Expand Down
6 changes: 3 additions & 3 deletions gix-pack/src/data/output/entry/iter_from_counts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ pub(crate) mod function {
stats.objects_copied_from_pack += 1;
entry
}
None => match db.try_find(&count.id, buf).map_err(Error::FindExisting)? {
None => match db.try_find(&count.id, buf).map_err(Error::Find)? {
Some((obj, _location)) => {
stats.decoded_and_recompressed_objects += 1;
output::Entry::from_data(count, &obj)
Expand All @@ -216,7 +216,7 @@ pub(crate) mod function {
},
}
}
None => match db.try_find(&count.id, buf).map_err(Error::FindExisting)? {
None => match db.try_find(&count.id, buf).map_err(Error::Find)? {
Some((obj, _location)) => {
stats.decoded_and_recompressed_objects += 1;
output::Entry::from_data(count, &obj)
Expand Down Expand Up @@ -397,7 +397,7 @@ mod types {
#[allow(missing_docs)]
pub enum Error {
#[error(transparent)]
FindExisting(crate::find::Error),
Find(gix_object::find::Error),
#[error(transparent)]
NewEntry(#[from] entry::Error),
}
Expand Down
54 changes: 0 additions & 54 deletions gix-pack/src/find.rs
View file
Original file line number Diff line number Diff line change
@@ -1,57 +1,3 @@
/// The error returned by methods of the [Find](crate::Find) trait.
pub type Error = Box<dyn std::error::Error + Send + Sync + 'static>;

///
pub mod existing {
use gix_hash::ObjectId;

/// The error returned by the [`find(…)`](crate::FindExt::find()) trait methods.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error(transparent)]
Find(crate::find::Error),
#[error("An object with id {} could not be found", .oid)]
NotFound { oid: ObjectId },
}
}

///
pub mod existing_object {
use gix_hash::ObjectId;

/// The error returned by the various [`find_*`](crate::FindExt::find_commit()) trait methods.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error(transparent)]
Find(crate::find::Error),
#[error(transparent)]
Decode(gix_object::decode::Error),
#[error("An object with id {} could not be found", .oid)]
NotFound { oid: ObjectId },
#[error("Expected object of kind {} something else", .expected)]
ObjectKind { expected: gix_object::Kind },
}
}

///
pub mod existing_iter {
use gix_hash::ObjectId;

/// The error returned by the various [`find_*`](crate::FindExt::find_commit()) trait methods.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error(transparent)]
Find(crate::find::Error),
#[error("An object with id {} could not be found", .oid)]
NotFound { oid: ObjectId },
#[error("Expected object of kind {} something else", .expected)]
ObjectKind { expected: gix_object::Kind },
}
}

/// An Entry in a pack providing access to its data.
///
/// Its commonly retrieved by reading from a pack index file followed by a read from a pack data file.
Expand Down
Loading

0 comments on commit d6e7201

Please sign in to comment.