( )⚙ D12528 rust-dirstatemap: add simpler method `get_or_insert_node` for the common case

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12528

rust-dirstatemap: add simpler method `get_or_insert_node` for the common case
Needs ReviewPublic

Authored by Alphare on Tue, Apr 12, 12:05 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

All but one case use the exact same input for most arguments, this simplifies
code and reduces footgun potential.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Tue, Apr 12, 12:05 PM
Alphare retitled this revision from rust-dirstatemap: add a simpler method `get_or_insert_node` for the common case to rust-dirstatemap: add simpler method `get_or_insert_node` for the common case.Tue, Apr 12, 12:10 PM

Revision Contents
Changeset List

Diff 33091

rust/hg-core/src/dirstate_tree/dirstate_map.rs

if map.on_disk.is_empty() { if map.on_disk.is_empty() {
return Ok((map, None)); return Ok((map, None));
} }
let parents = parse_dirstate_entries( let parents = parse_dirstate_entries(
map.on_disk, map.on_disk,
|path, entry, copy_source| { |path, entry, copy_source| {
let tracked = entry.state().is_tracked(); let tracked = entry.state().is_tracked();
let node = Self::get_or_insert_node( let node = Self::get_or_insert_node_inner(
map.on_disk, map.on_disk,
&mut map.unreachable_bytes, &mut map.unreachable_bytes,
&mut map.root, &mut map.root,
path, path,
WithBasename::to_cow_borrowed, WithBasename::to_cow_borrowed,
|ancestor| { |ancestor| {
if tracked { if tracked {
ancestor.tracked_descendants_count += 1 ancestor.tracked_descendants_count += 1
return Ok(Some(child)); return Ok(Some(child));
} }
} else { } else {
return Ok(None); return Ok(None);
} }
} }
} }
/// Get a mutable reference to the node at `path`, creating it if it does
/// not exist.
///
/// `each_ancestor` is a callback that is called for each ancestor node
/// when descending the tree. It is used to keep the different counters
/// of the `DirstateMap` up-to-date.
fn get_or_insert_node<'tree, 'path>( fn get_or_insert_node<'tree, 'path>(
&'tree mut self,
path: &'path HgPath,
each_ancestor: impl FnMut(&mut Node),
) -> Result<&'tree mut Node<'on_disk>, DirstateV2ParseError> {
Self::get_or_insert_node_inner(
self.on_disk,
&mut self.unreachable_bytes,
&mut self.root,
path,
WithBasename::to_cow_owned,
each_ancestor,
)
}
/// Lower-level version of `get_or_insert_node_inner`, which is used when
/// parsing disk data to remove allocations for new nodes.
fn get_or_insert_node_inner<'tree, 'path>(
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
unreachable_bytes: &mut u32, unreachable_bytes: &mut u32,
root: &'tree mut ChildNodes<'on_disk>, root: &'tree mut ChildNodes<'on_disk>,
path: &'path HgPath, path: &'path HgPath,
to_cow: impl Fn( to_cow: impl Fn(
WithBasename<&'path HgPath>, WithBasename<&'path HgPath>,
) -> WithBasename<Cow<'on_disk, HgPath>>, ) -> WithBasename<Cow<'on_disk, HgPath>>,
mut each_ancestor: impl FnMut(&mut Node), mut each_ancestor: impl FnMut(&mut Node),
p2_info: bool, p2_info: bool,
has_meaningful_mtime: bool, has_meaningful_mtime: bool,
parent_file_data_opt: Option<ParentFileData>, parent_file_data_opt: Option<ParentFileData>,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
let (had_entry, was_tracked) = match old_entry_opt { let (had_entry, was_tracked) = match old_entry_opt {
Some(old_entry) => (true, old_entry.tracked()), Some(old_entry) => (true, old_entry.tracked()),
None => (false, false), None => (false, false),
}; };
let node = Self::get_or_insert_node( let node = self.get_or_insert_node(filename, |ancestor| {
self.on_disk,
&mut self.unreachable_bytes,
&mut self.root,
filename,
WithBasename::to_cow_owned,
|ancestor| {
if !had_entry { if !had_entry {
ancestor.descendants_with_entry_count += 1; ancestor.descendants_with_entry_count += 1;
} }
if was_tracked { if was_tracked {
if !wc_tracked { if !wc_tracked {
ancestor.tracked_descendants_count = ancestor ancestor.tracked_descendants_count = ancestor
.tracked_descendants_count .tracked_descendants_count
.checked_sub(1) .checked_sub(1)
.expect("tracked count to be >= 0"); .expect("tracked count to be >= 0");
} }
} else { } else {
if wc_tracked { if wc_tracked {
ancestor.tracked_descendants_count += 1; ancestor.tracked_descendants_count += 1;
} }
} }
}, })?;
)?;
let v2_data = if let Some(parent_file_data) = parent_file_data_opt { let v2_data = if let Some(parent_file_data) = parent_file_data_opt {
DirstateV2Data { DirstateV2Data {
wc_tracked, wc_tracked,
p1_tracked, p1_tracked,
p2_info, p2_info,
mode_size: parent_file_data.mode_size, mode_size: parent_file_data.mode_size,
mtime: if has_meaningful_mtime { mtime: if has_meaningful_mtime {
parent_file_data.mtime parent_file_data.mtime
} else { } else {
None None
}, },
..Default::default() ..Default::default()
} }
} else { } else {
DirstateV2Data { DirstateV2Data {
wc_tracked, wc_tracked,
p1_tracked, p1_tracked,
p2_info, p2_info,
..Default::default() ..Default::default()
} }
}; };
node.data = NodeData::Entry(DirstateEntry::from_v2_data(v2_data));
if !had_entry { if !had_entry {
self.nodes_with_entry_count += 1; self.nodes_with_entry_count += 1;
} }
node.data = NodeData::Entry(DirstateEntry::from_v2_data(v2_data));
Ok(()) Ok(())
} }
fn set_tracked( fn set_tracked(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
old_entry_opt: Option<DirstateEntry>, old_entry_opt: Option<DirstateEntry>,
) -> Result<bool, DirstateV2ParseError> { ) -> Result<bool, DirstateV2ParseError> {
let was_tracked = old_entry_opt.map_or(false, |e| e.tracked()); let was_tracked = old_entry_opt.map_or(false, |e| e.tracked());
let had_entry = old_entry_opt.is_some(); let had_entry = old_entry_opt.is_some();
let tracked_count_increment = if was_tracked { 0 } else { 1 }; let tracked_count_increment = if was_tracked { 0 } else { 1 };
let mut new = false; let mut new = false;
let node = Self::get_or_insert_node( let node = self.get_or_insert_node(filename, |ancestor| {
self.on_disk,
&mut self.unreachable_bytes,
&mut self.root,
filename,
WithBasename::to_cow_owned,
|ancestor| {
if !had_entry { if !had_entry {
ancestor.descendants_with_entry_count += 1; ancestor.descendants_with_entry_count += 1;
} }
ancestor.tracked_descendants_count += tracked_count_increment; ancestor.tracked_descendants_count += tracked_count_increment;
}, })?;
)?; if let Some(old_entry) = old_entry_opt {
let new_entry = if let Some(old_entry) = old_entry_opt {
let mut e = old_entry.clone(); let mut e = old_entry.clone();
if e.tracked() { if e.tracked() {
// XXX // XXX
// This is probably overkill for more case, but we need this to // This is probably overkill for more case, but we need this to
// fully replace the `normallookup` call with `set_tracked` // fully replace the `normallookup` call with `set_tracked`
// one. Consider smoothing this in the future. // one. Consider smoothing this in the future.
e.set_possibly_dirty(); e.set_possibly_dirty();
} else { } else {
new = true; new = true;
e.set_tracked(); e.set_tracked();
} }
e node.data = NodeData::Entry(e)
} else { } else {
node.data = NodeData::Entry(DirstateEntry::new_tracked());
self.nodes_with_entry_count += 1; self.nodes_with_entry_count += 1;
new = true; new = true;
DirstateEntry::new_tracked()
}; };
node.data = NodeData::Entry(new_entry);
Ok(new) Ok(new)
} }
/// It is the responsibility of the caller to know that there was an entry /// It is the responsibility of the caller to know that there was an entry
/// there before. Does not handle the removal of copy source /// there before. Does not handle the removal of copy source
fn set_untracked( fn set_untracked(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
old_entry: DirstateEntry, old_entry: DirstateEntry,
) -> Result<(), DirstateV2ParseError> { ) -> Result<(), DirstateV2ParseError> {
let node = Self::get_or_insert_node( let node = self.get_or_insert_node(filename, |ancestor| {
self.on_disk,
&mut self.unreachable_bytes,
&mut self.root,
filename,
WithBasename::to_cow_owned,
|ancestor| {
ancestor.tracked_descendants_count = ancestor ancestor.tracked_descendants_count = ancestor
.tracked_descendants_count .tracked_descendants_count
.checked_sub(1) .checked_sub(1)
.expect("tracked_descendants_count should be >= 0"); .expect("tracked_descendants_count should be >= 0");
}, })?;
)?;
let mut new_entry = old_entry.clone(); let mut new_entry = old_entry.clone();
new_entry.set_untracked(); new_entry.set_untracked();
node.data = NodeData::Entry(new_entry); node.data = NodeData::Entry(new_entry);
Ok(()) Ok(())
} }
fn set_clean( fn set_clean(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
old_entry: DirstateEntry, old_entry: DirstateEntry,
mode: u32, mode: u32,
size: u32, size: u32,
mtime: TruncatedTimestamp, mtime: TruncatedTimestamp,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
let node = Self::get_or_insert_node( let node = self.get_or_insert_node(filename, |ancestor| {
self.on_disk,
&mut self.unreachable_bytes,
&mut self.root,
filename,
WithBasename::to_cow_owned,
|ancestor| {
if !old_entry.tracked() { if !old_entry.tracked() {
ancestor.tracked_descendants_count += 1; ancestor.tracked_descendants_count += 1;
} }
}, })?;
)?;
let mut new_entry = old_entry.clone(); let mut new_entry = old_entry.clone();
new_entry.set_clean(mode, size, mtime); new_entry.set_clean(mode, size, mtime);
node.data = NodeData::Entry(new_entry); node.data = NodeData::Entry(new_entry);
Ok(()) Ok(())
} }
fn set_possibly_dirty( fn set_possibly_dirty(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
let node = Self::get_or_insert_node( let node = self.get_or_insert_node(filename, |_ancestor| {})?;
self.on_disk,
&mut self.unreachable_bytes,
&mut self.root,
filename,
WithBasename::to_cow_owned,
|_ancestor| {},
)?;
let entry = node.data.as_entry_mut().expect("entry should exist"); let entry = node.data.as_entry_mut().expect("entry should exist");
entry.set_possibly_dirty(); entry.set_possibly_dirty();
node.data = NodeData::Entry(*entry); node.data = NodeData::Entry(*entry);
Ok(()) Ok(())
} }
/// Clears the cached mtime for the (potential) folder at `path`. /// Clears the cached mtime for the (potential) folder at `path`.
pub(super) fn clear_cached_mtime( pub(super) fn clear_cached_mtime(
} }
pub fn copy_map_insert( pub fn copy_map_insert(
&mut self, &mut self,
key: &HgPath, key: &HgPath,
value: &HgPath, value: &HgPath,
) -> Result<Option<HgPathBuf>, DirstateV2ParseError> { ) -> Result<Option<HgPathBuf>, DirstateV2ParseError> {
self.with_dmap_mut(|map| { self.with_dmap_mut(|map| {
let node = DirstateMap::get_or_insert_node( let node = map.get_or_insert_node(&key, |_ancestor| {})?;
map.on_disk, let had_copy_source = node.copy_source.is_none();
&mut map.unreachable_bytes, let old = node
&mut map.root,
&key,
WithBasename::to_cow_owned,
|_ancestor| {},
)?;
if node.copy_source.is_none() {
map.nodes_with_copy_source_count += 1
}
Ok(node
.copy_source .copy_source
.replace(value.to_owned().into()) .replace(value.to_owned().into())
.map(Cow::into_owned)) .map(Cow::into_owned);
if had_copy_source {
map.nodes_with_copy_source_count += 1
}
Ok(old)
}) })
} }
pub fn len(&self) -> usize { pub fn len(&self) -> usize {
let map = self.get_map(); let map = self.get_map();
map.nodes_with_entry_count as usize map.nodes_with_entry_count as usize
} }
for res in self.iter() { for res in self.iter() {
let (path, entry) = res?; let (path, entry) = res?;
if entry.p2_info() { if entry.p2_info() {
files_with_p2_info.push(path.to_owned()) files_with_p2_info.push(path.to_owned())
} }
} }
self.with_dmap_mut(|map| { self.with_dmap_mut(|map| {
for path in files_with_p2_info.iter() { for path in files_with_p2_info.iter() {
let node = DirstateMap::get_or_insert_node( let node = map.get_or_insert_node(path, |_| {})?;
map.on_disk,
&mut map.unreachable_bytes,
&mut map.root,
path,
WithBasename::to_cow_owned,
|_| {},
)?;
let entry = let entry =
node.data.as_entry_mut().expect("entry should exist"); node.data.as_entry_mut().expect("entry should exist");
entry.drop_merge_data(); entry.drop_merge_data();
if let Some(source) = node.copy_source.take().as_deref() { if let Some(source) = node.copy_source.take().as_deref() {
copies.push((path.to_owned(), source.to_owned())); copies.push((path.to_owned(), source.to_owned()));
} }
} }
Ok(copies) Ok(copies)