rust-index: use interior mutability in head revs and caches
For upcoming changes in `hg-cpython` switching to the `hg-core` index in
ancestors iterators, we will need to avoid excessive mutability, restricting
the use of mutable references on `hg::index::Index` to methods that actually
logically mutate it, whereas the maintenance of caches such as `head_revs`
clearly does not. We illustrate that immediately by switching to immutable
borrows in the corresponding methods of `hg-cpython::MixedIndex`
--- a/rust/hg-core/src/revlog/index.rs Thu Oct 26 15:26:19 2023 +0200
+++ b/rust/hg-core/src/revlog/index.rs Fri Oct 27 21:48:45 2023 +0200
@@ -261,12 +261,13 @@
offsets: RwLock<Option<Vec<usize>>>,
uses_generaldelta: bool,
is_inline: bool,
- /// Cache of the head revisions in this index, kept in sync. Should
+ /// Cache of (head_revisions, filtered_revisions)
+ ///
+ /// The head revisions in this index, kept in sync. Should
/// be accessed via the [`Self::head_revs`] method.
- head_revs: Vec<Revision>,
- /// Cache of the last filtered revisions in this index, used to make sure
+ /// The last filtered revisions in this index, used to make sure
/// we haven't changed filters when returning the cached `head_revs`.
- filtered_revs: HashSet<Revision>,
+ head_revs: RwLock<(Vec<Revision>, HashSet<Revision>)>,
}
impl Debug for Index {
@@ -366,8 +367,7 @@
offsets: RwLock::new(Some(offsets)),
uses_generaldelta,
is_inline: true,
- head_revs: vec![],
- filtered_revs: HashSet::new(),
+ head_revs: RwLock::new((vec![], HashSet::new())),
})
} else {
Err(HgError::corrupted("unexpected inline revlog length"))
@@ -378,8 +378,7 @@
offsets: RwLock::new(None),
uses_generaldelta,
is_inline: false,
- head_revs: vec![],
- filtered_revs: HashSet::new(),
+ head_revs: RwLock::new((vec![], HashSet::new())),
})
}
}
@@ -532,17 +531,27 @@
}
/// Return the head revisions of this index
- pub fn head_revs(&mut self) -> Result<Vec<Revision>, GraphError> {
+ pub fn head_revs(&self) -> Result<Vec<Revision>, GraphError> {
self.head_revs_filtered(&HashSet::new())
}
/// Return the head revisions of this index
pub fn head_revs_filtered(
- &mut self,
+ &self,
filtered_revs: &HashSet<Revision>,
) -> Result<Vec<Revision>, GraphError> {
- if !self.head_revs.is_empty() && filtered_revs == &self.filtered_revs {
- return Ok(self.head_revs.to_owned());
+ {
+ let guard = self
+ .head_revs
+ .read()
+ .expect("RwLock on Index.head_revs should not be poisoned");
+ let self_head_revs = &guard.0;
+ let self_filtered_revs = &guard.1;
+ if !self_head_revs.is_empty()
+ && filtered_revs == self_filtered_revs
+ {
+ return Ok(self_head_revs.to_owned());
+ }
}
let mut revs: HashSet<Revision, RandomState> =
if filtered_revs.is_empty() {
@@ -570,8 +579,11 @@
let mut as_vec: Vec<Revision> =
revs.into_iter().map(Into::into).collect();
as_vec.sort_unstable();
- self.head_revs = as_vec.to_owned();
- self.filtered_revs = filtered_revs.to_owned();
+ *self
+ .head_revs
+ .write()
+ .expect("RwLock on Index.head_revs should not be poisoned") =
+ (as_vec.to_owned(), filtered_revs.to_owned());
Ok(as_vec)
}
@@ -651,6 +663,14 @@
Ok(())
}
+ fn clear_head_revs(&self) {
+ self.head_revs
+ .write()
+ .expect("RwLock on Index.head_revs should not be poisoined")
+ .0
+ .clear()
+ }
+
/// TODO move this to the trait probably, along with other things
pub fn append(
&mut self,
@@ -662,7 +682,7 @@
offsets.push(new_offset)
}
self.bytes.added.extend(revision_data.into_v1().as_bytes());
- self.head_revs.clear();
+ self.clear_head_revs();
Ok(())
}
@@ -676,16 +696,19 @@
if let Some(offsets) = &mut *self.get_offsets_mut() {
offsets.truncate(rev.0 as usize)
}
- self.head_revs.clear();
+ self.clear_head_revs();
Ok(())
}
- pub fn clear_caches(&mut self) {
+ pub fn clear_caches(&self) {
// We need to get the 'inline' value from Python at init and use this
// instead of offsets to determine whether we're inline since we might
// clear caches. This implies re-populating the offsets on-demand.
- self.offsets = RwLock::new(None);
- self.head_revs.clear();
+ *self
+ .offsets
+ .write()
+ .expect("RwLock on Index.offsets should not be poisoed") = None;
+ self.clear_head_revs();
}
/// Unchecked version of `is_snapshot`.
--- a/rust/hg-cpython/src/revlog.rs Thu Oct 26 15:26:19 2023 +0200
+++ b/rust/hg-cpython/src/revlog.rs Fri Oct 27 21:48:45 2023 +0200
@@ -225,7 +225,7 @@
self.nt(py).borrow_mut().take();
self.docket(py).borrow_mut().take();
self.nodemap_mmap(py).borrow_mut().take();
- self.index(py).borrow_mut().clear_caches();
+ self.index(py).borrow().clear_caches();
self.call_cindex(py, "clearcaches", args, kw)
}
@@ -849,7 +849,7 @@
}
fn inner_headrevs(&self, py: Python) -> PyResult<PyObject> {
- let index = &mut *self.index(py).borrow_mut();
+ let index = &*self.index(py).borrow();
let as_vec: Vec<PyObject> = index
.head_revs()
.map_err(|e| graph_error(py, e))?
@@ -881,7 +881,7 @@
py: Python,
py_revs: &PyTuple,
) -> PyResult<PyObject> {
- let index = &mut *self.index(py).borrow_mut();
+ let index = &*self.index(py).borrow();
let revs: Vec<_> = rev_pyiter_collect(py, py_revs.as_object(), index)?;
let as_vec: Vec<_> = index
.ancestors(&revs)
@@ -897,7 +897,7 @@
py: Python,
py_revs: &PyTuple,
) -> PyResult<PyObject> {
- let index = &mut *self.index(py).borrow_mut();
+ let index = &*self.index(py).borrow();
let revs: Vec<_> = rev_pyiter_collect(py, py_revs.as_object(), index)?;
let as_vec: Vec<_> = index
.common_ancestor_heads(&revs)
@@ -980,7 +980,7 @@
target_density: f64,
min_gap_size: usize,
) -> PyResult<PyObject> {
- let index = &mut *self.index(py).borrow_mut();
+ let index = &*self.index(py).borrow();
let revs: Vec<_> = rev_pyiter_collect(py, &revs, index)?;
let as_nested_vec =
index.slice_chunk_to_density(&revs, target_density, min_gap_size);