Merge pull request #1012 from NobodyXu/optimization/try-into-de-momo

Optimize `gix`: de-momo `impl TryInto` by hand

Author: Byron

gix/src/clone/mod.rs

@@ -68,14 +68,30 @@ impl PrepareFetch {
         url: Url,
         path: impl AsRef<std::path::Path>,
         kind: crate::create::Kind,
-        mut create_opts: crate::create::Options,
+        create_opts: crate::create::Options,
         open_opts: crate::open::Options,
     ) -> Result<Self, Error>
     where
         Url: TryInto<gix_url::Url, Error = E>,
         gix_url::parse::Error: From<E>,
     {
-        let mut url = url.try_into().map_err(gix_url::parse::Error::from)?;
+        Self::new_inner(
+            url.try_into().map_err(gix_url::parse::Error::from)?,
+            path.as_ref(),
+            kind,
+            create_opts,
+            open_opts,
+        )
+    }
+
+    #[allow(clippy::result_large_err)]
+    fn new_inner(
+        mut url: gix_url::Url,
+        path: &std::path::Path,
+        kind: crate::create::Kind,
+        mut create_opts: crate::create::Options,
+        open_opts: crate::open::Options,
+    ) -> Result<Self, Error> {
         create_opts.destination_must_be_empty = true;
         let mut repo = crate::ThreadSafeRepository::init_opts(path, kind, create_opts, open_opts)?.to_thread_local();
         url.canonicalize(repo.options.current_dir_or_empty())

gix/src/remote/build.rs

@@ -10,7 +10,10 @@ impl Remote<'_> {
         Url: TryInto<gix_url::Url, Error = E>,
         gix_url::parse::Error: From<E>,
     {
-        self.push_url_inner(url, true)
+        self.push_url_inner(
+            url.try_into().map_err(|err| remote::init::Error::Url(err.into()))?,
+            true,
+        )
     }
 
     /// Set the `url` to be used when pushing data to a remote, without applying rewrite rules in case these could be faulty,
@@ -20,7 +23,10 @@ impl Remote<'_> {
         Url: TryInto<gix_url::Url, Error = E>,
         gix_url::parse::Error: From<E>,
     {
-        self.push_url_inner(url, false)
+        self.push_url_inner(
+            url.try_into().map_err(|err| remote::init::Error::Url(err.into()))?,
+            false,
+        )
     }
 
     /// Configure how tags should be handled when fetching from the remote.
@@ -29,14 +35,11 @@ impl Remote<'_> {
         self
     }
 
-    fn push_url_inner<Url, E>(mut self, push_url: Url, should_rewrite_urls: bool) -> Result<Self, remote::init::Error>
-    where
-        Url: TryInto<gix_url::Url, Error = E>,
-        gix_url::parse::Error: From<E>,
-    {
-        let push_url = push_url
-            .try_into()
-            .map_err(|err| remote::init::Error::Url(err.into()))?;
+    fn push_url_inner(
+        mut self,
+        push_url: gix_url::Url,
+        should_rewrite_urls: bool,
+    ) -> Result<Self, remote::init::Error> {
         self.push_url = push_url.into();
 
         let (_, push_url_alias) = should_rewrite_urls

gix/src/remote/init.rs

@@ -67,7 +67,18 @@ impl<'repo> Remote<'repo> {
         Url: TryInto<gix_url::Url, Error = E>,
         gix_url::parse::Error: From<E>,
     {
-        let url = url.try_into().map_err(|err| Error::Url(err.into()))?;
+        Self::from_fetch_url_inner(
+            url.try_into().map_err(|err| Error::Url(err.into()))?,
+            should_rewrite_urls,
+            repo,
+        )
+    }
+
+    fn from_fetch_url_inner(
+        url: gix_url::Url,
+        should_rewrite_urls: bool,
+        repo: &'repo Repository,
+    ) -> Result<Self, Error> {
         let (url_alias, _) = should_rewrite_urls
             .then(|| rewrite_urls(&repo.config, Some(&url), None))
             .unwrap_or(Ok((None, None)))?;

gix/src/repository/object.rs

@@ -8,6 +8,7 @@ use gix_ref::{
     transaction::{LogChange, PreviousValue, RefLog},
     FullName,
 };
+use smallvec::SmallVec;
 
 use crate::{commit, ext::ObjectIdExt, object, tag, Id, Object, Reference, Tree};
 
@@ -115,13 +116,17 @@ impl crate::Repository {
         let mut buf = self.shared_empty_buf();
         object.write_to(buf.deref_mut()).expect("write to memory works");
 
-        let oid = gix_object::compute_hash(self.object_hash(), object.kind(), &buf);
+        self.write_object_inner(&buf, object.kind())
+    }
+
+    fn write_object_inner(&self, buf: &[u8], kind: gix_object::Kind) -> Result<Id<'_>, object::write::Error> {
+        let oid = gix_object::compute_hash(self.object_hash(), kind, buf);
         if self.objects.contains(&oid) {
             return Ok(oid.attach(self));
         }
 
         self.objects
-            .write_buf(object.kind(), &buf)
+            .write_buf(kind, buf)
             .map(|oid| oid.attach(self))
             .map_err(Into::into)
     }
@@ -130,6 +135,7 @@ impl crate::Repository {
     ///
     /// We avoid writing duplicate objects to slow disks that will eventually have to be garbage collected by
     /// pre-hashing the data, and checking if the object is already present.
+    #[momo]
     pub fn write_blob(&self, bytes: impl AsRef<[u8]>) -> Result<Id<'_>, object::write::Error> {
         let bytes = bytes.as_ref();
         let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, bytes);
@@ -154,13 +160,18 @@ impl crate::Repository {
     ) -> Result<Id<'_>, object::write::Error> {
         let mut buf = self.shared_empty_buf();
         std::io::copy(&mut bytes, buf.deref_mut()).expect("write to memory works");
-        let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, &buf);
+
+        self.write_blob_stream_inner(&buf)
+    }
+
+    fn write_blob_stream_inner(&self, buf: &[u8]) -> Result<Id<'_>, object::write::Error> {
+        let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, buf);
         if self.objects.contains(&oid) {
             return Ok(oid.attach(self));
         }
 
         self.objects
-            .write_buf(gix_object::Kind::Blob, &buf)
+            .write_buf(gix_object::Kind::Blob, buf)
             .map_err(Into::into)
             .map(|oid| oid.attach(self))
     }
@@ -208,21 +219,39 @@ impl crate::Repository {
         Name: TryInto<FullName, Error = E>,
         commit::Error: From<E>,
     {
+        self.commit_as_inner(
+            committer.into(),
+            author.into(),
+            reference.try_into()?,
+            message.as_ref(),
+            tree.into(),
+            parents.into_iter().map(Into::into).collect(),
+        )
+    }
+
+    fn commit_as_inner(
+        &self,
+        committer: gix_actor::SignatureRef<'_>,
+        author: gix_actor::SignatureRef<'_>,
+        reference: FullName,
+        message: &str,
+        tree: ObjectId,
+        parents: SmallVec<[gix_hash::ObjectId; 1]>,
+    ) -> Result<Id<'_>, commit::Error> {
         use gix_ref::{
             transaction::{Change, RefEdit},
             Target,
         };
 
         // TODO: possibly use CommitRef to save a few allocations (but will have to allocate for object ids anyway.
         //       This can be made vastly more efficient though if we wanted to, so we lie in the API
-        let reference = reference.try_into()?;
         let commit = gix_object::Commit {
-            message: message.as_ref().into(),
-            tree: tree.into(),
-            author: author.into().to_owned(),
-            committer: committer.into().to_owned(),
+            message: message.into(),
+            tree,
+            author: author.into(),
+            committer: committer.into(),
             encoding: None,
-            parents: parents.into_iter().map(Into::into).collect(),
+            parents,
             extra_headers: Default::default(),
         };
 

gix/src/repository/reference.rs

@@ -87,14 +87,27 @@ impl crate::Repository {
         Name: TryInto<FullName, Error = E>,
         gix_validate::reference::name::Error: From<E>,
     {
-        let name = name.try_into().map_err(gix_validate::reference::name::Error::from)?;
-        let id = target.into();
+        self.reference_inner(
+            name.try_into().map_err(gix_validate::reference::name::Error::from)?,
+            target.into(),
+            constraint,
+            log_message.into(),
+        )
+    }
+
+    fn reference_inner(
+        &self,
+        name: FullName,
+        id: ObjectId,
+        constraint: PreviousValue,
+        log_message: BString,
+    ) -> Result<Reference<'_>, reference::edit::Error> {
         let mut edits = self.edit_reference(RefEdit {
             change: Change::Update {
                 log: LogChange {
                     mode: RefLog::AndReference,
                     force_create_reflog: false,
-                    message: log_message.into(),
+                    message: log_message,
                 },
                 expected: constraint,
                 new: Target::Peeled(id),