fix: static linked list delta cache with memory cap (#851)

Previously, the 64 slot big LRU cache for pack deltas didn't
use any memory limit which could lead to memory exhaustion in the
face of untypical, large objects.

Now we add a generous default limit to do *better* in such situations.
It's worth noting though that that even without any cache, the working
set of buffers to do delta resolution takes considerable memory, despite
trying to keep it minimal.

Note that for bigger objects, the cache is now not used at all, which probably
leads to terrible performance as not even the base object can be cached.

Author: Byron

Diff for: Makefile

@@ -151,6 +151,7 @@ unit-tests: ## run all unit tests
 	cd gix-ref/tests && cargo test --all-features
 	cd gix-odb && cargo test && cargo test --all-features
 	cd gix-object && cargo test && cargo test --features verbose-object-parsing-errors
+	cd gix-pack && cargo test --all-features
 	cd gix-pack/tests && cargo test --features internal-testing-to-avoid-being-run-by-cargo-test-all \
 				&& cargo test --features "internal-testing-gix-features-parallel"
 	cd gix-index/tests && cargo test --features internal-testing-to-avoid-being-run-by-cargo-test-all \

Diff for: gix-pack/src/cache/lru.rs

@@ -107,41 +107,74 @@ mod _static {
     /// Values of 64 seem to improve performance.
     pub struct StaticLinkedList<const SIZE: usize> {
         inner: uluru::LRUCache<Entry, SIZE>,
-        free_list: Vec<Vec<u8>>,
+        last_evicted: Vec<u8>,
         debug: gix_features::cache::Debug,
+        /// the amount of bytes we are currently holding, taking into account the capacities of all Vecs we keep.
+        mem_used: usize,
+        /// The total amount of memory we should be able to hold with all entries combined.
+        mem_limit: usize,
     }
 
-    impl<const SIZE: usize> Default for StaticLinkedList<SIZE> {
-        fn default() -> Self {
+    impl<const SIZE: usize> StaticLinkedList<SIZE> {
+        /// Create a new list with a memory limit of `mem_limit` in bytes. If 0, there is no memory limit.
+        pub fn new(mem_limit: usize) -> Self {
             StaticLinkedList {
                 inner: Default::default(),
-                free_list: Vec::new(),
+                last_evicted: Vec::new(),
                 debug: gix_features::cache::Debug::new(format!("StaticLinkedList<{SIZE}>")),
+                mem_used: 0,
+                mem_limit: if mem_limit == 0 { usize::MAX } else { mem_limit },
             }
         }
     }
 
+    impl<const SIZE: usize> Default for StaticLinkedList<SIZE> {
+        fn default() -> Self {
+            Self::new(96 * 1024 * 1024)
+        }
+    }
+
     impl<const SIZE: usize> DecodeEntry for StaticLinkedList<SIZE> {
         fn put(&mut self, pack_id: u32, offset: u64, data: &[u8], kind: gix_object::Kind, compressed_size: usize) {
+            // We cannot possibly hold this much.
+            if data.len() > self.mem_limit {
+                return;
+            }
+            // If we could hold it but are are at limit, all we can do is make space.
+            let mem_free = self.mem_limit - self.mem_used;
+            if data.len() > mem_free {
+                // prefer freeing free-lists instead of clearing our cache
+                let free_list_cap = self.last_evicted.len();
+                self.last_evicted = Vec::new();
+                // still not enough? clear everything
+                if data.len() > mem_free + free_list_cap {
+                    self.inner.clear();
+                    self.mem_used = 0;
+                } else {
+                    self.mem_used -= free_list_cap;
+                }
+            }
             self.debug.put();
+            let (prev_cap, cur_cap);
             if let Some(previous) = self.inner.insert(Entry {
                 offset,
                 pack_id,
-                data: self
-                    .free_list
-                    .pop()
-                    .map(|mut v| {
-                        v.clear();
-                        v.resize(data.len(), 0);
-                        v.copy_from_slice(data);
-                        v
-                    })
-                    .unwrap_or_else(|| Vec::from(data)),
+                data: {
+                    let mut v = std::mem::take(&mut self.last_evicted);
+                    prev_cap = v.capacity();
+                    v.clear();
+                    v.resize(data.len(), 0);
+                    v.copy_from_slice(data);
+                    cur_cap = v.capacity();
+                    v
+                },
                 kind,
                 compressed_size,
             }) {
-                self.free_list.push(previous.data)
+                // No need to adjust capacity as we already counted it.
+                self.last_evicted = previous.data;
             }
+            self.mem_used = self.mem_used + cur_cap - prev_cap;
         }
 
         fn get(&mut self, pack_id: u32, offset: u64, out: &mut Vec<u8>) -> Option<(gix_object::Kind, usize)> {
@@ -162,6 +195,56 @@ mod _static {
             res
         }
     }
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+
+        #[test]
+        fn no_limit() {
+            let c = StaticLinkedList::<10>::new(0);
+            assert_eq!(
+                c.mem_limit,
+                usize::MAX,
+                "zero is automatically turned into a large limit that is equivalent to unlimited"
+            );
+        }
+
+        #[test]
+        fn journey() {
+            let mut c = StaticLinkedList::<10>::new(100);
+            assert_eq!(c.mem_limit, 100);
+            assert_eq!(c.mem_used, 0);
+
+            // enough memory for normal operation
+            let mut last_mem_used = 0;
+            for _ in 0..10 {
+                c.put(0, 0, &[0], gix_object::Kind::Blob, 1);
+                assert!(c.mem_used > last_mem_used);
+                last_mem_used = c.mem_used;
+            }
+            assert_eq!(c.mem_used, 80, "there is a minimal vec size");
+            assert_eq!(c.inner.len(), 10);
+            assert_eq!(c.last_evicted.len(), 0);
+
+            c.put(0, 0, &(0..20).collect::<Vec<_>>(), gix_object::Kind::Blob, 1);
+            assert_eq!(c.inner.len(), 10);
+            assert_eq!(c.mem_used, 80 + 20);
+            assert_eq!(c.last_evicted.len(), 1);
+
+            c.put(0, 0, &(0..50).collect::<Vec<_>>(), gix_object::Kind::Blob, 1);
+            assert_eq!(c.inner.len(), 1, "cache clearance wasn't necessary");
+            assert_eq!(c.last_evicted.len(), 0, "the free list was cleared");
+            assert_eq!(c.mem_used, 50);
+
+            c.put(0, 0, &(0..101).collect::<Vec<_>>(), gix_object::Kind::Blob, 1);
+            assert_eq!(
+                c.inner.len(),
+                1,
+                "objects that won't ever fit within the memory limit are ignored"
+            );
+        }
+    }
 }
 
 #[cfg(feature = "pack-cache-lru-static")]

Diff for: gix-pack/src/cache/object.rs

@@ -1,5 +1,3 @@
-//! # Note
-//!
 //! This module is a bit 'misplaced' if spelled out like 'gix_pack::cache::object::*' but is best placed here for code re-use and
 //! general usefulness.
 use crate::cache;