This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10829

dirstate-v2: Drop parent directory cache when removing a dirstate node
ClosedPublic

Authored by SimonSapin on Jun 1 2021, 12:51 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG9d58e54b5966: dirstate-v2: Drop parent directory cache when removing a dirstate node
Summary

The premise of the directory cache is that the dirstate contains child nodes
for every entry that read_dir would return. When removing nodes, that may
not be the case anymore so the cache should be invalidated.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Jun 1 2021, 12:51 PM
SimonSapin added a commit: rHG9d58e54b5966: dirstate-v2: Drop parent directory cache when removing a dirstate node.Jun 1 2021, 3:36 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG9d58e54b5966: dirstate-v2: Drop parent directory cache when removing a dirstate node (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.
baymax updated this revision to Diff 28387.Jun 1 2021, 5:05 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Revision Contents
Changeset List

Diff 28362

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

filename: &HgPath, filename: &HgPath,
old_state: EntryState, old_state: EntryState,
) -> Result<bool, DirstateError> { ) -> Result<bool, DirstateError> {
struct Dropped { struct Dropped {
was_tracked: bool, was_tracked: bool,
had_entry: bool, had_entry: bool,
had_copy_source: bool, had_copy_source: bool,
} }
/// If this returns `Ok(Some((dropped, removed)))`, then
///
/// * `dropped` is about the leaf node that was at `filename`
/// * `removed` is whether this particular level of recursion just
/// removed a node in `nodes`.
fn recur<'on_disk>( fn recur<'on_disk>(
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
nodes: &mut ChildNodes<'on_disk>, nodes: &mut ChildNodes<'on_disk>,
path: &HgPath, path: &HgPath,
) -> Result<Option<Dropped>, DirstateV2ParseError> { ) -> Result<Option<(Dropped, bool)>, DirstateV2ParseError> {
let (first_path_component, rest_of_path) = let (first_path_component, rest_of_path) =
path.split_first_component(); path.split_first_component();
let node = if let Some(node) = let node = if let Some(node) =
nodes.make_mut(on_disk)?.get_mut(first_path_component) nodes.make_mut(on_disk)?.get_mut(first_path_component)
{ {
node node
} else { } else {
return Ok(None); return Ok(None);
}; };
let dropped; let dropped;
if let Some(rest) = rest_of_path { if let Some(rest) = rest_of_path {
if let Some(d) = recur(on_disk, &mut node.children, rest)? { if let Some((d, removed)) =
recur(on_disk, &mut node.children, rest)?
{
dropped = d; dropped = d;
if dropped.was_tracked { if dropped.was_tracked {
node.tracked_descendants_count -= 1; node.tracked_descendants_count -= 1;
} }
// Directory caches must be invalidated when removing a
// child node
if removed {
if let NodeData::CachedDirectory { .. } = &node.data {
node.data = NodeData::None
}
}
} else { } else {
return Ok(None); return Ok(None);
} }
} else { } else {
let had_entry = node.data.has_entry(); let had_entry = node.data.has_entry();
if had_entry { if had_entry {
node.data = NodeData::None node.data = NodeData::None
} }
dropped = Dropped { dropped = Dropped {
was_tracked: node was_tracked: node
.data .data
.as_entry() .as_entry()
.map_or(false, |entry| entry.state.is_tracked()), .map_or(false, |entry| entry.state.is_tracked()),
had_entry, had_entry,
had_copy_source: node.copy_source.take().is_some(), had_copy_source: node.copy_source.take().is_some(),
}; };
} }
// After recursion, for both leaf (rest_of_path is None) nodes and // After recursion, for both leaf (rest_of_path is None) nodes and
// parent nodes, remove a node if it just became empty. // parent nodes, remove a node if it just became empty.
if !node.data.has_entry() let remove = !node.data.has_entry()
&& node.copy_source.is_none() && node.copy_source.is_none()
&& node.children.is_empty() && node.children.is_empty();
{ if remove {
nodes.make_mut(on_disk)?.remove(first_path_component); nodes.make_mut(on_disk)?.remove(first_path_component);
} }
Ok(Some(dropped)) Ok(Some((dropped, remove)))
} }
if let Some(dropped) = recur(self.on_disk, &mut self.root, filename)? { if let Some((dropped, _removed)) =
recur(self.on_disk, &mut self.root, filename)?
{
if dropped.had_entry { if dropped.had_entry {
self.nodes_with_entry_count -= 1 self.nodes_with_entry_count -= 1
} }
if dropped.had_copy_source { if dropped.had_copy_source {
self.nodes_with_copy_source_count -= 1 self.nodes_with_copy_source_count -= 1
} }
Ok(dropped.had_entry) Ok(dropped.had_entry)
} else { } else {

tests/test-status.t

$ hg status $ hg status
Creating a new file changes the directory’s mtime, invalidating the cache Creating a new file changes the directory’s mtime, invalidating the cache
$ touch subdir/unknown $ touch subdir/unknown
$ hg status $ hg status
? subdir/unknown ? subdir/unknown
$ rm subdir/unknown
$ hg status
Removing a node from the dirstate resets the cache for its parent directory
$ hg forget subdir/a
$ hg debugdirstate --dirs --no-dates | grep '^d'
d 0 0 set subdir
$ hg ci -qm '#1'
$ hg debugdirstate --dirs --no-dates | grep '^d'
d 0 0 unset subdir
$ hg status
? subdir/a
#endif #endif