# HG changeset patch # User Georges Racinet on incendie.racinet.fr # Date 1698436125 -7200 # Node ID 59183a19954ef56050c7320d8563bc955cfaa09d # Parent ca81cd96000ad174c47b8f20da9794126cb4c00f 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` diff -r ca81cd96000a -r 59183a19954e rust/hg-core/src/revlog/index.rs --- 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>>, 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, - /// 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, + head_revs: RwLock<(Vec, HashSet)>, } 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, GraphError> { + pub fn head_revs(&self) -> Result, 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, ) -> Result, 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 = if filtered_revs.is_empty() { @@ -570,8 +579,11 @@ let mut as_vec: Vec = 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`. diff -r ca81cd96000a -r 59183a19954e rust/hg-cpython/src/revlog.rs --- 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 { - let index = &mut *self.index(py).borrow_mut(); + let index = &*self.index(py).borrow(); let as_vec: Vec = index .head_revs() .map_err(|e| graph_error(py, e))? @@ -881,7 +881,7 @@ py: Python, py_revs: &PyTuple, ) -> PyResult { - 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 { - 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 { - 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);