Thorough merge after GC

including accurate eq impl for UniMap

Co-authored-by: Ralf Jung <[email protected]>

Author: Vanille-N

src/tools/miri/bench-cargo-miri/zip-equal/Cargo.lock

@@ -0,0 +1,7 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "zip-equal"
+version = "0.1.0"

src/tools/miri/bench-cargo-miri/zip-equal/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "zip-equal"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

src/tools/miri/bench-cargo-miri/zip-equal/src/main.rs

@@ -0,0 +1,22 @@
+//! This is a pathological pattern in which opportunities to merge
+//! adjacent identical items in the RangeMap are not properly detected
+//! because `RangeMap::iter_mut` is never called on overlapping ranges
+//! and thus never merges previously split ranges. This does not produce any
+//! additional cost for access operations, but it makes the job of the Tree Borrows
+//! GC procedure much more costly.
+//! See https://github.com/rust-lang/miri/issues/2863
+
+const LENGTH: usize = (1 << 14) - 1;
+const LONG: &[u8] = &[b'x'; LENGTH];
+
+fn main() {
+    assert!(eq(LONG, LONG))
+}
+
+fn eq(s1: &[u8], s2: &[u8]) -> bool {
+    if s1.len() != s2.len() {
+        return false;
+    }
+
+    s1.iter().zip(s2).all(|(c1, c2)| *c1 == *c2)
+}

src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs

@@ -488,7 +488,13 @@ impl<'tcx> Tree {
 /// Integration with the BorTag garbage collector
 impl Tree {
     pub fn remove_unreachable_tags(&mut self, live_tags: &FxHashSet<BorTag>) {
-        assert!(self.keep_only_needed(self.root, live_tags)); // root can't be removed
+        let root_is_needed = self.keep_only_needed(self.root, live_tags); // root can't be removed
+        assert!(root_is_needed);
+        // Right after the GC runs is a good moment to check if we can
+        // merge some adjacent ranges that were made equal by the removal of some
+        // tags (this does not necessarily mean that they have identical internal representations,
+        // see the `PartialEq` impl for `UniValMap`)
+        self.rperms.merge_adjacent_thorough();
     }
 
     /// Traverses the entire tree looking for useless tags.

src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs

@@ -36,13 +36,42 @@ pub struct UniKeyMap<K> {
 }
 
 /// From UniIndex to V
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, Eq)]
 pub struct UniValMap<V> {
     /// The mapping data. Thanks to Vec we get both fast accesses, and
     /// a memory-optimal representation if there are few deletions.
     data: Vec<Option<V>>,
 }
 
+impl<V: PartialEq> UniValMap<V> {
+    /// Exact equality of two maps.
+    /// Less accurate but faster than `equivalent`, mostly because
+    /// of the fast path when the lengths are different.
+    pub fn identical(&self, other: &Self) -> bool {
+        self.data == other.data
+    }
+
+    /// Equality up to trailing `None`s of two maps, i.e.
+    /// do they represent the same mapping ?
+    pub fn equivalent(&self, other: &Self) -> bool {
+        let min_len = self.data.len().min(other.data.len());
+        self.data[min_len..].iter().all(Option::is_none)
+            && other.data[min_len..].iter().all(Option::is_none)
+            && (self.data[..min_len] == other.data[..min_len])
+    }
+}
+
+impl<V: PartialEq> PartialEq for UniValMap<V> {
+    /// 2023-05: We found that using `equivalent` rather than `identical`
+    /// in the equality testing of the `RangeMap` is neutral for most
+    /// benchmarks, while being quite beneficial for `zip-equal`
+    /// and to a lesser extent for `unicode`, `slice-get-unchecked` and
+    /// `backtraces` as well.
+    fn eq(&self, other: &Self) -> bool {
+        self.equivalent(other)
+    }
+}
+
 impl<V> Default for UniValMap<V> {
     fn default() -> Self {
         Self { data: Vec::default() }

src/tools/miri/src/range_map.rs

@@ -227,6 +227,24 @@ impl<T> RangeMap<T> {
         };
         slice.iter_mut().map(|elem| (elem.range.clone(), &mut elem.data))
     }
+
+    /// Remove all adjacent duplicates
+    pub fn merge_adjacent_thorough(&mut self)
+    where
+        T: PartialEq,
+    {
+        let clean = Vec::with_capacity(self.v.len());
+        for elem in std::mem::replace(&mut self.v, clean) {
+            if let Some(prev) = self.v.last_mut() {
+                if prev.data == elem.data {
+                    assert_eq!(prev.range.end, elem.range.start);
+                    prev.range.end = elem.range.end;
+                    continue;
+                }
+            }
+            self.v.push(elem);
+        }
+    }
 }
 
 #[cfg(test)]