This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10493

rust: Remove DirstateMap::file_fold_map
ClosedPublic

Authored by SimonSapin on Apr 20 2021, 10:18 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG33e5511b571a: rust: Remove DirstateMap::file_fold_map
rHGffcbebd57c31: rust: Remove DirstateMap::file_fold_map
Summary

This was a HashMap constructed on demand and then cached in the DirstateMap
struct to avoid reconstructing at the next access. However the only use is
in Python bindings converting it to a PyDict. That method in turn is wrapped
in a @cachedproperty in Python code.

This was two redudant layers of caching. This changeset removes the Rust-level
one to keep the Python dict cache, and have bindings create a PyDict by
iterating.

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.Apr 20 2021, 10:18 AM
baymax updated this revision to Diff 27143.Apr 20 2021, 1:03 PM

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

Alphare accepted this revision.Apr 22 2021, 5:26 AM
Alphare added a subscriber: Alphare.

A welcome change!

This revision is now accepted and ready to land.Apr 22 2021, 5:26 AM
SimonSapin updated this revision to Diff 27319.May 3 2021, 6:24 AM
SimonSapin added a commit: rHGffcbebd57c31: rust: Remove DirstateMap::file_fold_map.May 6 2021, 12:53 PM
Closed by commit rHGffcbebd57c31: rust: Remove DirstateMap::file_fold_map (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 27653

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

// dirstate_map.rs // dirstate_map.rs
// //
// Copyright 2019 Raphaël Gomès <rgomes@octobus.net> // Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
use crate::dirstate::parsers::clear_ambiguous_mtime; use crate::dirstate::parsers::clear_ambiguous_mtime;
use crate::dirstate::parsers::Timestamp; use crate::dirstate::parsers::Timestamp;
use crate::errors::HgError; use crate::errors::HgError;
use crate::revlog::node::NULL_NODE; use crate::revlog::node::NULL_NODE;
use crate::{ use crate::{
dirstate::{parsers::PARENT_SIZE, EntryState}, dirstate::{parsers::PARENT_SIZE, EntryState},
pack_dirstate, parse_dirstate, pack_dirstate, parse_dirstate,
utils::{ utils::hg_path::{HgPath, HgPathBuf},
files::normalize_case,
hg_path::{HgPath, HgPathBuf},
},
CopyMap, DirsMultiset, DirstateEntry, DirstateError, DirstateMapError, CopyMap, DirsMultiset, DirstateEntry, DirstateError, DirstateMapError,
DirstateParents, FastHashMap, StateMap, DirstateParents, StateMap,
}; };
use micro_timer::timed; use micro_timer::timed;
use std::collections::HashSet; use std::collections::HashSet;
use std::convert::TryInto; use std::convert::TryInto;
use std::iter::FromIterator; use std::iter::FromIterator;
use std::ops::Deref; use std::ops::Deref;
pub type FileFoldMap = FastHashMap<HgPathBuf, HgPathBuf>;
#[derive(Default)] #[derive(Default)]
pub struct DirstateMap { pub struct DirstateMap {
state_map: StateMap, state_map: StateMap,
pub copy_map: CopyMap, pub copy_map: CopyMap,
file_fold_map: Option<FileFoldMap>,
pub dirs: Option<DirsMultiset>, pub dirs: Option<DirsMultiset>,
pub all_dirs: Option<DirsMultiset>, pub all_dirs: Option<DirsMultiset>,
non_normal_set: Option<HashSet<HgPathBuf>>, non_normal_set: Option<HashSet<HgPathBuf>>,
other_parent_set: Option<HashSet<HgPathBuf>>, other_parent_set: Option<HashSet<HgPathBuf>>,
parents: Option<DirstateParents>, parents: Option<DirstateParents>,
dirty_parents: bool, dirty_parents: bool,
} }
impl DirstateMap { impl DirstateMap {
pub fn new() -> Self { pub fn new() -> Self {
Self::default() Self::default()
} }
pub fn clear(&mut self) { pub fn clear(&mut self) {
self.state_map = StateMap::default(); self.state_map = StateMap::default();
self.copy_map.clear(); self.copy_map.clear();
self.file_fold_map = None;
self.non_normal_set = None; self.non_normal_set = None;
self.other_parent_set = None; self.other_parent_set = None;
self.set_parents(&DirstateParents { self.set_parents(&DirstateParents {
p1: NULL_NODE, p1: NULL_NODE,
p2: NULL_NODE, p2: NULL_NODE,
}) })
} }
} }
} }
if old_state == EntryState::Unknown { if old_state == EntryState::Unknown {
if let Some(ref mut all_dirs) = self.all_dirs { if let Some(ref mut all_dirs) = self.all_dirs {
all_dirs.add_path(filename)?; all_dirs.add_path(filename)?;
} }
} }
if let Some(ref mut file_fold_map) = self.file_fold_map {
file_fold_map.remove(&normalize_case(filename));
}
self.state_map.insert( self.state_map.insert(
filename.to_owned(), filename.to_owned(),
DirstateEntry { DirstateEntry {
state: EntryState::Removed, state: EntryState::Removed,
mode: 0, mode: 0,
size, size,
mtime: 0, mtime: 0,
}, },
if let Some(ref mut dirs) = self.dirs { if let Some(ref mut dirs) = self.dirs {
dirs.delete_path(filename)?; dirs.delete_path(filename)?;
} }
} }
if let Some(ref mut all_dirs) = self.all_dirs { if let Some(ref mut all_dirs) = self.all_dirs {
all_dirs.delete_path(filename)?; all_dirs.delete_path(filename)?;
} }
} }
if let Some(ref mut file_fold_map) = self.file_fold_map {
file_fold_map.remove(&normalize_case(filename));
}
self.get_non_normal_other_parent_entries() self.get_non_normal_other_parent_entries()
.0 .0
.remove(filename); .remove(filename);
Ok(exists) Ok(exists)
} }
pub fn clear_ambiguous_times( pub fn clear_ambiguous_times(
let packed = let packed =
pack_dirstate(&mut self.state_map, &self.copy_map, parents, now)?; pack_dirstate(&mut self.state_map, &self.copy_map, parents, now)?;
self.dirty_parents = false; self.dirty_parents = false;
self.set_non_normal_other_parent_entries(true); self.set_non_normal_other_parent_entries(true);
Ok(packed) Ok(packed)
} }
pub fn build_file_fold_map(&mut self) -> &FileFoldMap {
if let Some(ref file_fold_map) = self.file_fold_map {
return file_fold_map;
}
let mut new_file_fold_map = FileFoldMap::default();
for (filename, DirstateEntry { state, .. }) in self.state_map.iter() {
if *state != EntryState::Removed {
new_file_fold_map
.insert(normalize_case(&filename), filename.to_owned());
}
}
self.file_fold_map = Some(new_file_fold_map);
self.file_fold_map.as_ref().unwrap()
}
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
#[test] #[test]
fn test_dirs_multiset() { fn test_dirs_multiset() {

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

use crate::utils::hg_path::{HgPath, HgPathBuf}; use crate::utils::hg_path::{HgPath, HgPathBuf};
use crate::CopyMapIter; use crate::CopyMapIter;
use crate::DirstateEntry; use crate::DirstateEntry;
use crate::DirstateError; use crate::DirstateError;
use crate::DirstateMapError; use crate::DirstateMapError;
use crate::DirstateParents; use crate::DirstateParents;
use crate::DirstateStatus; use crate::DirstateStatus;
use crate::EntryState; use crate::EntryState;
use crate::FastHashMap;
use crate::HgPathCow; use crate::HgPathCow;
use crate::PatternFileWarning; use crate::PatternFileWarning;
use crate::StateMapIter; use crate::StateMapIter;
use crate::StatusError; use crate::StatusError;
use crate::StatusOptions; use crate::StatusOptions;
pub struct DirstateMap { pub struct DirstateMap {
parents: Option<DirstateParents>, parents: Option<DirstateParents>,
&mut packed, &mut packed,
); );
} }
} }
self.dirty_parents = false; self.dirty_parents = false;
Ok(packed) Ok(packed)
} }
fn build_file_fold_map(&mut self) -> &FastHashMap<HgPathBuf, HgPathBuf> {
todo!()
}
fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> { fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> {
// Do nothing, this `DirstateMap` does not a separate `all_dirs` that // Do nothing, this `DirstateMap` does not a separate `all_dirs` that
// needs to be recomputed // needs to be recomputed
Ok(()) Ok(())
} }
fn set_dirs(&mut self) -> Result<(), DirstateMapError> { fn set_dirs(&mut self) -> Result<(), DirstateMapError> {
// Do nothing, this `DirstateMap` does not a separate `dirs` that needs // Do nothing, this `DirstateMap` does not a separate `dirs` that needs

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

use std::path::PathBuf; use std::path::PathBuf;
use crate::dirstate::parsers::Timestamp; use crate::dirstate::parsers::Timestamp;
use crate::matchers::Matcher; use crate::matchers::Matcher;
use crate::utils::hg_path::{HgPath, HgPathBuf}; use crate::utils::hg_path::{HgPath, HgPathBuf};
use crate::CopyMapIter; use crate::CopyMapIter;
use crate::DirstateEntry; use crate::DirstateEntry;
use crate::DirstateError; use crate::DirstateError;
use crate::DirstateMap; use crate::DirstateMap;
use crate::DirstateMapError; use crate::DirstateMapError;
use crate::DirstateParents; use crate::DirstateParents;
use crate::DirstateStatus; use crate::DirstateStatus;
use crate::EntryState; use crate::EntryState;
use crate::FastHashMap;
use crate::HgPathCow; use crate::HgPathCow;
use crate::PatternFileWarning; use crate::PatternFileWarning;
use crate::StateMapIter; use crate::StateMapIter;
use crate::StatusError; use crate::StatusError;
use crate::StatusOptions; use crate::StatusOptions;
pub trait DirstateMapMethods { pub trait DirstateMapMethods {
fn clear(&mut self); fn clear(&mut self);
) -> Result<Option<&'a DirstateParents>, DirstateError>; ) -> Result<Option<&'a DirstateParents>, DirstateError>;
fn pack( fn pack(
&mut self, &mut self,
parents: DirstateParents, parents: DirstateParents,
now: Timestamp, now: Timestamp,
) -> Result<Vec<u8>, DirstateError>; ) -> Result<Vec<u8>, DirstateError>;
fn build_file_fold_map(&mut self) -> &FastHashMap<HgPathBuf, HgPathBuf>;
fn set_all_dirs(&mut self) -> Result<(), DirstateMapError>; fn set_all_dirs(&mut self) -> Result<(), DirstateMapError>;
fn set_dirs(&mut self) -> Result<(), DirstateMapError>; fn set_dirs(&mut self) -> Result<(), DirstateMapError>;
fn status<'a>( fn status<'a>(
&'a self, &'a self,
matcher: &'a (dyn Matcher + Sync), matcher: &'a (dyn Matcher + Sync),
root_dir: PathBuf, root_dir: PathBuf,
fn pack( fn pack(
&mut self, &mut self,
parents: DirstateParents, parents: DirstateParents,
now: Timestamp, now: Timestamp,
) -> Result<Vec<u8>, DirstateError> { ) -> Result<Vec<u8>, DirstateError> {
self.pack(parents, now) self.pack(parents, now)
} }
fn build_file_fold_map(&mut self) -> &FastHashMap<HgPathBuf, HgPathBuf> {
self.build_file_fold_map()
}
fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> { fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> {
self.set_all_dirs() self.set_all_dirs()
} }
fn set_dirs(&mut self) -> Result<(), DirstateMapError> { fn set_dirs(&mut self) -> Result<(), DirstateMapError> {
self.set_dirs() self.set_dirs()
} }

rust/hg-cpython/src/dirstate/dirstate_map.rs

dirstate::{dirs_multiset::Dirs, make_dirstate_tuple}, dirstate::{dirs_multiset::Dirs, make_dirstate_tuple},
parsers::dirstate_parents_to_pytuple, parsers::dirstate_parents_to_pytuple,
}; };
use hg::{ use hg::{
dirstate::parsers::Timestamp, dirstate::parsers::Timestamp,
dirstate_tree::dispatch::DirstateMapMethods, dirstate_tree::dispatch::DirstateMapMethods,
errors::HgError, errors::HgError,
revlog::Node, revlog::Node,
utils::files::normalize_case,
utils::hg_path::{HgPath, HgPathBuf}, utils::hg_path::{HgPath, HgPathBuf},
DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap, DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
DirstateMapError, DirstateParents, EntryState, StateMapIter, DirstateMapError, DirstateParents, EntryState, StateMapIter,
}; };
// TODO // TODO
// This object needs to share references to multiple members of its Rust // This object needs to share references to multiple members of its Rust
// inner struct, namely `copy_map`, `dirs` and `all_dirs`. // inner struct, namely `copy_map`, `dirs` and `all_dirs`.
py, py,
"Dirstate error".to_string(), "Dirstate error".to_string(),
)), )),
} }
} }
def filefoldmapasdict(&self) -> PyResult<PyDict> { def filefoldmapasdict(&self) -> PyResult<PyDict> {
let dict = PyDict::new(py); let dict = PyDict::new(py);
for (key, value) in for (path, entry) in self.inner(py).borrow_mut().iter() {
self.inner(py).borrow_mut().build_file_fold_map().iter() if entry.state != EntryState::Removed {
{ let key = normalize_case(path);
let value = path;
dict.set_item( dict.set_item(
py, py,
PyBytes::new(py, key.as_bytes()).into_object(), PyBytes::new(py, key.as_bytes()).into_object(),
PyBytes::new(py, value.as_bytes()).into_object(), PyBytes::new(py, value.as_bytes()).into_object(),
)?; )?;
} }
}
Ok(dict) Ok(dict)
} }
def __len__(&self) -> PyResult<usize> { def __len__(&self) -> PyResult<usize> {
Ok(self.inner(py).borrow().len()) Ok(self.inner(py).borrow().len())
} }
def __contains__(&self, key: PyObject) -> PyResult<bool> { def __contains__(&self, key: PyObject) -> PyResult<bool> {