Merge pull request #689 from workingjubilee/formalize-lru

Use a custom container for Cache's cache

Author: ChrisDenton

.github/workflows/main.yml

@@ -256,7 +256,7 @@ jobs:
         with:
           submodules: true
       - name: Install Rust
-        run: rustup update 1.73.0 --no-self-update && rustup default 1.73.0
+        run: rustup update 1.79.0 --no-self-update && rustup default 1.79.0
       - run: cargo build
 
   miri:

Cargo.toml

@@ -14,7 +14,7 @@ autoexamples = true
 autotests = true
 edition = "2021"
 exclude = ["/ci/"]
-rust-version = "1.65.0"
+rust-version = "1.79.0"
 
 [workspace]
 members = ['crates/cpp_smoke_test', 'crates/as-if-std']

src/symbolize/gimli.rs

@@ -52,8 +52,11 @@ cfg_if::cfg_if! {
     }
 }
 
+mod lru;
 mod stash;
 
+use lru::Lru;
+
 const MAPPINGS_CACHE_SIZE: usize = 4;
 
 struct Mapping {
@@ -263,7 +266,7 @@ struct Cache {
     ///
     /// Note that this is basically an LRU cache and we'll be shifting things
     /// around in here as we symbolize addresses.
-    mappings: Vec<(usize, Mapping)>,
+    mappings: Lru<(usize, Mapping), MAPPINGS_CACHE_SIZE>,
 }
 
 struct Library {
@@ -344,7 +347,7 @@ pub unsafe fn clear_symbol_cache() {
 impl Cache {
     fn new() -> Cache {
         Cache {
-            mappings: Vec::with_capacity(MAPPINGS_CACHE_SIZE),
+            mappings: Lru::default(),
             libraries: native_libraries(),
         }
     }
@@ -403,31 +406,18 @@ impl Cache {
     }
 
     fn mapping_for_lib<'a>(&'a mut self, lib: usize) -> Option<(&'a mut Context<'a>, &'a Stash)> {
-        let idx = self.mappings.iter().position(|(idx, _)| *idx == lib);
-
-        // Invariant: after this conditional completes without early returning
-        // from an error, the cache entry for this path is at index 0.
+        let cache_idx = self.mappings.iter().position(|(lib_id, _)| *lib_id == lib);
 
-        if let Some(idx) = idx {
-            // When the mapping is already in the cache, move it to the front.
-            if idx != 0 {
-                let entry = self.mappings.remove(idx);
-                self.mappings.insert(0, entry);
-            }
+        let cache_entry = if let Some(idx) = cache_idx {
+            self.mappings.move_to_front(idx)
         } else {
-            // When the mapping is not in the cache, create a new mapping,
-            // insert it into the front of the cache, and evict the oldest cache
-            // entry if necessary.
-            let mapping = create_mapping(&self.libraries[lib])?;
-
-            if self.mappings.len() == MAPPINGS_CACHE_SIZE {
-                self.mappings.pop();
-            }
-
-            self.mappings.insert(0, (lib, mapping));
-        }
+            // When the mapping is not in the cache, create a new mapping and insert it,
+            // which will also evict the oldest entry.
+            create_mapping(&self.libraries[lib])
+                .and_then(|mapping| self.mappings.push_front((lib, mapping)))
+        };
 
-        let mapping = &mut self.mappings[0].1;
+        let (_, mapping) = cache_entry?;
         let cx: &'a mut Context<'static> = &mut mapping.cx;
         let stash: &'a Stash = &mapping.stash;
         // don't leak the `'static` lifetime, make sure it's scoped to just

src/symbolize/gimli/lru.rs

@@ -0,0 +1,75 @@
+use core::mem::{self, MaybeUninit};
+use core::ptr;
+
+/// least-recently-used cache with static size
+pub(crate) struct Lru<T, const N: usize> {
+    // SAFETY: len <= initialized values
+    len: usize,
+    arr: [MaybeUninit<T>; N],
+}
+
+impl<T, const N: usize> Default for Lru<T, N> {
+    fn default() -> Self {
+        Lru {
+            len: 0,
+            arr: [const { MaybeUninit::uninit() }; N],
+        }
+    }
+}
+
+impl<T, const N: usize> Lru<T, N> {
+    #[inline]
+    pub fn clear(&mut self) {
+        let len = self.len;
+        self.len = 0;
+        // SAFETY: we can't touch these values again due to setting self.len = 0
+        unsafe { ptr::drop_in_place(ptr::addr_of_mut!(self.arr[0..len]) as *mut [T]) }
+    }
+
+    #[inline]
+    pub fn iter(&self) -> impl Iterator<Item = &T> {
+        self.arr[0..self.len]
+            .iter()
+            // SAFETY: we only iterate initialized values due to our len invariant
+            .map(|init| unsafe { init.assume_init_ref() })
+    }
+
+    #[inline]
+    pub fn push_front(&mut self, value: T) -> Option<&mut T> {
+        if N == 0 {
+            return None;
+        } else if self.len == N {
+            self.len = N - 1;
+            // SAFETY: we maintain len invariant and bail on N == 0
+            unsafe { ptr::drop_in_place(self.arr.as_mut_ptr().cast::<T>().add(N - 1)) };
+        };
+        let len_to_init = self.len + 1;
+        let mut last = MaybeUninit::new(value);
+        for elem in self.arr[0..len_to_init].iter_mut() {
+            // OPT(size): using `mem::swap` allows surprising size regressions
+            last = mem::replace(elem, last);
+        }
+        self.len = len_to_init;
+
+        self.arr
+            .first_mut()
+            // SAFETY: we just pushed it
+            .map(|elem| unsafe { elem.assume_init_mut() })
+    }
+
+    #[inline]
+    pub fn move_to_front(&mut self, idx: usize) -> Option<&mut T> {
+        let elem = self.arr[0..self.len].get_mut(idx)?;
+        // SAFETY: we already bailed if the index is bad, so our slicing will be infallible,
+        // so it is permissible to allow the len invariant to decay, as we always restore it
+        let mut last = mem::replace(elem, MaybeUninit::uninit());
+        for elem in self.arr[0..=idx].iter_mut() {
+            // OPT(size): using `mem::swap` allows surprising size regressions
+            last = mem::replace(elem, last);
+        }
+        self.arr
+            .first_mut()
+            // SAFETY: we have restored the len invariant
+            .map(|elem| unsafe { elem.assume_init_mut() })
+    }
+}