Merge branch 'perf-and-safety'

Author: Byron

gix-filter/src/worktree/encode_to_git.rs

@@ -26,7 +26,6 @@ pub(crate) mod function {
     use encoding_rs::DecoderResult;
 
     use super::{Error, RoundTripCheck};
-    use crate::clear_and_set_capacity;
 
     /// Decode `src` according to `src_encoding` to `UTF-8` for storage in git and place it in `buf`.
     /// Note that the encoding is always applied, there is no conditional even if `src_encoding` already is `UTF-8`.
@@ -40,13 +39,8 @@ pub(crate) mod function {
         let buf_len = decoder
             .max_utf8_buffer_length_without_replacement(src.len())
             .ok_or(Error::Overflow { input_len: src.len() })?;
-        clear_and_set_capacity(buf, buf_len);
-        // SAFETY: `clear_and_set_capacity` assure that we have the given `buf_len` allocated, so setting its length is only making available
-        //          what is allocated. Later we will truncate to the amount of actually written bytes.
-        #[allow(unsafe_code)]
-        unsafe {
-            buf.set_len(buf_len);
-        }
+        buf.clear();
+        buf.resize(buf_len, 0);
         let (res, read, written) = decoder.decode_to_utf8_without_replacement(src, buf, true);
         match res {
             DecoderResult::InputEmpty => {
@@ -55,11 +49,7 @@ pub(crate) mod function {
                     "encoding_rs estimates the maximum amount of bytes written correctly"
                 );
                 assert_eq!(read, src.len(), "input buffer should be fully consumed");
-                // SAFETY: we trust that `encoding_rs` reports this number correctly, and truncate everything else.
-                #[allow(unsafe_code)]
-                unsafe {
-                    buf.set_len(written);
-                }
+                buf.truncate(written);
             }
             DecoderResult::OutputFull => {
                 unreachable!("we assure that the output buffer is big enough as per the encoder's estimate")

gix-filter/src/worktree/encode_to_worktree.rs

@@ -17,7 +17,6 @@ pub(crate) mod function {
     use encoding_rs::EncoderResult;
 
     use super::Error;
-    use crate::clear_and_set_capacity;
 
     /// Encode `src_utf8`, which is assumed to be UTF-8 encoded, according to `worktree_encoding` for placement in the working directory,
     /// and write it to `buf`, possibly resizing it.
@@ -33,13 +32,8 @@ pub(crate) mod function {
             .ok_or(Error::Overflow {
                 input_len: src_utf8.len(),
             })?;
-        clear_and_set_capacity(buf, buf_len);
-        // SAFETY: `clear_and_set_capacity` assure that we have the given `buf_len` allocated, so setting its length is only making available
-        //          what is allocated. Later we will truncate to the amount of actually written bytes.
-        #[allow(unsafe_code)]
-        unsafe {
-            buf.set_len(buf_len);
-        }
+        buf.clear();
+        buf.resize(buf_len, 0);
         let src = std::str::from_utf8(src_utf8)?;
         let (res, read, written) = encoder.encode_from_utf8_without_replacement(src, buf, true);
         match res {
@@ -49,11 +43,7 @@ pub(crate) mod function {
                     "encoding_rs estimates the maximum amount of bytes written correctly"
                 );
                 assert_eq!(read, src_utf8.len(), "input buffer should be fully consumed");
-                // SAFETY: we trust that `encoding_rs` reports this number correctly, and truncate everything else.
-                #[allow(unsafe_code)]
-                unsafe {
-                    buf.set_len(written);
-                }
+                buf.truncate(written);
             }
             EncoderResult::OutputFull => {
                 unreachable!("we assure that the output buffer is big enough as per the encoder's estimate")

gix-odb/src/store_impls/dynamic/find.rs

@@ -75,6 +75,7 @@ pub(crate) mod error {
     }
 }
 pub use error::Error;
+use gix_features::zlib;
 
 use crate::{store::types::PackId, Find};
 
@@ -86,6 +87,7 @@ where
         &'b self,
         mut id: &'b gix_hash::oid,
         buffer: &'a mut Vec<u8>,
+        inflate: &mut zlib::Inflate,
         pack_cache: &mut impl DecodeEntry,
         snapshot: &mut load_index::Snapshot,
         recursion: Option<error::DeltaBaseRecursion<'_>>,
@@ -147,6 +149,7 @@ where
                         let res = match pack.decode_entry(
                             entry,
                             buffer,
+                            inflate,
                             |id, _out| {
                                 index_file.pack_offset_by_id(id).map(|pack_offset| {
                                     gix_pack::data::decode::entry::ResolvedBase::InPack(pack.entry(pack_offset))
@@ -182,6 +185,7 @@ where
                                     .try_find_cached_inner(
                                         &base_id,
                                         &mut buf,
+                                        inflate,
                                         pack_cache,
                                         snapshot,
                                         recursion
@@ -231,6 +235,7 @@ where
                                 pack.decode_entry(
                                     entry,
                                     buffer,
+                                    inflate,
                                     |id, out| {
                                         index_file
                                             .pack_offset_by_id(id)
@@ -347,7 +352,8 @@ where
     ) -> Result<Option<(gix_object::Data<'a>, Option<gix_pack::data::entry::Location>)>, Self::Error> {
         let id = id.as_ref();
         let mut snapshot = self.snapshot.borrow_mut();
-        self.try_find_cached_inner(id, buffer, pack_cache, &mut snapshot, None)
+        let mut inflate = self.inflate.borrow_mut();
+        self.try_find_cached_inner(id, buffer, &mut inflate, pack_cache, &mut snapshot, None)
     }
 
     fn location_by_oid(
@@ -364,6 +370,7 @@ where
 
         let id = id.as_ref();
         let mut snapshot = self.snapshot.borrow_mut();
+        let mut inflate = self.inflate.borrow_mut();
         'outer: loop {
             {
                 let marker = snapshot.marker;
@@ -404,13 +411,14 @@ where
                         buf.resize(entry.decompressed_size.try_into().expect("representable size"), 0);
                         assert_eq!(pack.id, pack_id.to_intrinsic_pack_id(), "both ids must always match");
 
-                        let res = pack.decompress_entry(&entry, buf).ok().map(|entry_size_past_header| {
-                            gix_pack::data::entry::Location {
+                        let res = pack
+                            .decompress_entry(&entry, &mut inflate, buf)
+                            .ok()
+                            .map(|entry_size_past_header| gix_pack::data::entry::Location {
                                 pack_id: pack.id,
                                 pack_offset,
                                 entry_size: entry.header_size() + entry_size_past_header,
-                            }
-                        });
+                            });
 
                         if idx != 0 {
                             snapshot.indices.swap(0, idx);

gix-odb/src/store_impls/dynamic/handle.rs

@@ -257,6 +257,7 @@ impl super::Store {
             refresh: RefreshMode::default(),
             ignore_replacements: false,
             token: Some(token),
+            inflate: RefCell::new(Default::default()),
             snapshot: RefCell::new(self.collect_snapshot()),
             max_recursion_depth: Self::INITIAL_MAX_RECURSION_DEPTH,
             packed_object_count: Default::default(),
@@ -273,6 +274,7 @@ impl super::Store {
             refresh: Default::default(),
             ignore_replacements: false,
             token: Some(token),
+            inflate: RefCell::new(Default::default()),
             snapshot: RefCell::new(self.collect_snapshot()),
             max_recursion_depth: Self::INITIAL_MAX_RECURSION_DEPTH,
             packed_object_count: Default::default(),
@@ -391,6 +393,7 @@ where
                 }
                 .into()
             },
+            inflate: RefCell::new(Default::default()),
             snapshot: RefCell::new(self.store.collect_snapshot()),
             max_recursion_depth: self.max_recursion_depth,
             packed_object_count: Default::default(),

gix-odb/src/store_impls/dynamic/header.rs

@@ -1,3 +1,4 @@
+use gix_features::zlib;
 use std::ops::Deref;
 
 use gix_hash::oid;
@@ -15,6 +16,7 @@ where
     fn try_header_inner<'b>(
         &'b self,
         mut id: &'b gix_hash::oid,
+        inflate: &mut zlib::Inflate,
         snapshot: &mut load_index::Snapshot,
         recursion: Option<DeltaBaseRecursion<'_>>,
     ) -> Result<Option<Header>, Error> {
@@ -71,7 +73,7 @@ where
                             },
                         };
                         let entry = pack.entry(pack_offset);
-                        let res = match pack.decode_header(entry, |id| {
+                        let res = match pack.decode_header(entry, inflate, |id| {
                             index_file.pack_offset_by_id(id).map(|pack_offset| {
                                 gix_pack::data::decode::header::ResolvedBase::InPack(pack.entry(pack_offset))
                             })
@@ -85,6 +87,7 @@ where
                                 let hdr = self
                                     .try_header_inner(
                                         &base_id,
+                                        inflate,
                                         snapshot,
                                         recursion
                                             .map(DeltaBaseRecursion::inc_depth)
@@ -127,7 +130,7 @@ where
                                     .as_ref()
                                     .expect("pack to still be available like just now");
                                 let entry = pack.entry(pack_offset);
-                                pack.decode_header(entry, |id| {
+                                pack.decode_header(entry, inflate, |id| {
                                     index_file
                                         .pack_offset_by_id(id)
                                         .map(|pack_offset| {
@@ -184,6 +187,7 @@ where
     fn try_header(&self, id: impl AsRef<oid>) -> Result<Option<Header>, Self::Error> {
         let id = id.as_ref();
         let mut snapshot = self.snapshot.borrow_mut();
-        self.try_header_inner(id, &mut snapshot, None)
+        let mut inflate = self.inflate.borrow_mut();
+        self.try_header_inner(id, &mut inflate, &mut snapshot, None)
     }
 }

gix-odb/src/store_impls/dynamic/mod.rs

@@ -1,4 +1,5 @@
 //! The standard object store which should fit all needs.
+use gix_features::zlib;
 use std::{cell::RefCell, ops::Deref};
 
 use crate::Store;
@@ -24,6 +25,7 @@ where
 
     pub(crate) token: Option<handle::Mode>,
     snapshot: RefCell<load_index::Snapshot>,
+    inflate: RefCell<zlib::Inflate>,
     packed_object_count: RefCell<Option<u64>>,
 }
 

gix-pack/src/bundle/find.rs

@@ -1,25 +1,30 @@
+use gix_features::zlib;
+
 impl crate::Bundle {
-    /// Find an object with the given [`ObjectId`][gix_hash::ObjectId] and place its data into `out`.
+    /// Find an object with the given [`ObjectId`](gix_hash::ObjectId) and place its data into `out`.
+    /// `inflate` is used to decompress objects, and will be reset before first use, but not after the last use.
     ///
-    /// [`cache`][crate::cache::DecodeEntry] is used to accelerate the lookup.
+    /// [`cache`](crate::cache::DecodeEntry) is used to accelerate the lookup.
     ///
     /// **Note** that ref deltas are automatically resolved within this pack only, which makes this implementation unusable
     /// for thin packs, which by now are expected to be resolved already.
     pub fn find<'a>(
         &self,
         id: impl AsRef<gix_hash::oid>,
         out: &'a mut Vec<u8>,
+        inflate: &mut zlib::Inflate,
         cache: &mut impl crate::cache::DecodeEntry,
     ) -> Result<Option<(gix_object::Data<'a>, crate::data::entry::Location)>, crate::data::decode::Error> {
         let idx = match self.index.lookup(id) {
             Some(idx) => idx,
             None => return Ok(None),
         };
-        self.get_object_by_index(idx, out, cache).map(Some)
+        self.get_object_by_index(idx, out, inflate, cache).map(Some)
     }
 
     /// Special-use function to get an object given an index previously returned from
-    /// `internal_find_pack_index`.
+    /// [index::File::](crate::index::File::lookup()).
+    /// `inflate` is used to decompress objects, and will be reset before first use, but not after the last use.
     ///
     /// # Panics
     ///
@@ -28,6 +33,7 @@ impl crate::Bundle {
         &self,
         idx: u32,
         out: &'a mut Vec<u8>,
+        inflate: &mut zlib::Inflate,
         cache: &mut impl crate::cache::DecodeEntry,
     ) -> Result<(gix_object::Data<'a>, crate::data::entry::Location), crate::data::decode::Error> {
         let ofs = self.index.pack_offset_at_index(idx);
@@ -37,6 +43,7 @@ impl crate::Bundle {
             .decode_entry(
                 pack_entry,
                 out,
+                inflate,
                 |id, _out| {
                     self.index.lookup(id).map(|idx| {
                         crate::data::decode::entry::ResolvedBase::InPack(

gix-pack/src/cache/delta/traverse/resolve.rs

@@ -122,7 +122,7 @@ where
             let (result_size, consumed) = data::delta::decode_header_size(&delta_bytes[consumed..]);
             header_ofs += consumed;
 
-            set_len(fully_resolved_delta_bytes, result_size as usize);
+            fully_resolved_delta_bytes.resize(result_size as usize, 0);
             data::delta::apply(&base_bytes, fully_resolved_delta_bytes, &delta_bytes[header_ofs..]);
 
             // FIXME: this actually invalidates the "pack_offset()" computation, which is not obvious to consumers
@@ -394,28 +394,13 @@ where
     })
 }
 
-fn set_len(v: &mut Vec<u8>, new_len: usize) {
-    if new_len > v.len() {
-        v.reserve_exact(new_len.saturating_sub(v.capacity()) + (v.capacity() - v.len()));
-        // SAFETY:
-        // 1. we have reserved enough capacity to fit `new_len`
-        // 2. the caller is trusted to write into `v` to completely fill `new_len`.
-        #[allow(unsafe_code, clippy::uninit_vec)]
-        unsafe {
-            v.set_len(new_len);
-        }
-    } else {
-        v.truncate(new_len)
-    }
-}
-
 fn decompress_all_at_once_with(
     inflate: &mut zlib::Inflate,
     b: &[u8],
     decompressed_len: usize,
     out: &mut Vec<u8>,
 ) -> Result<(), Error> {
-    set_len(out, decompressed_len);
+    out.resize(decompressed_len, 0);
     inflate.reset();
     inflate.once(b, out).map_err(|err| Error::ZlibInflate {
         source: err,