Patchwork D10493: rust: Remove DirstateMap::file_fold_map

login
register
mail settings
Submitter phabricator
Date April 20, 2021, 2:19 p.m.
Message ID <differential-rev-PHID-DREV-6kjkqjym27w3xg2hi6ro-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48811/
State Superseded
Headers show

Comments

phabricator - April 20, 2021, 2:19 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION 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.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10493

AFFECTED FILES
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dispatch.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -30,6 +30,7 @@ 
     dirstate_tree::dispatch::DirstateMapMethods,
     errors::HgError,
     revlog::Node,
+    utils::files::normalize_case,
     utils::hg_path::{HgPath, HgPathBuf},
     DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
     DirstateMapError, DirstateParents, EntryState, StateMapIter,
@@ -329,14 +330,16 @@ 
 
     def filefoldmapasdict(&self) -> PyResult<PyDict> {
         let dict = PyDict::new(py);
-        for (key, value) in
-            self.inner(py).borrow_mut().build_file_fold_map().iter()
-        {
-            dict.set_item(
-                py,
-                PyBytes::new(py, key.as_bytes()).into_object(),
-                PyBytes::new(py, value.as_bytes()).into_object(),
-            )?;
+        for (path, entry) in self.inner(py).borrow_mut().iter() {
+            if entry.state != EntryState::Removed {
+                let key = normalize_case(path);
+                let value = path;
+                dict.set_item(
+                    py,
+                    PyBytes::new(py, key.as_bytes()).into_object(),
+                    PyBytes::new(py, value.as_bytes()).into_object(),
+                )?;
+            }
         }
         Ok(dict)
     }
diff --git a/rust/hg-core/src/dirstate_tree/dispatch.rs b/rust/hg-core/src/dirstate_tree/dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/dispatch.rs
@@ -11,7 +11,6 @@ 
 use crate::DirstateParents;
 use crate::DirstateStatus;
 use crate::EntryState;
-use crate::FastHashMap;
 use crate::HgPathCow;
 use crate::PatternFileWarning;
 use crate::StateMapIter;
@@ -93,8 +92,6 @@ 
         now: Timestamp,
     ) -> Result<Vec<u8>, DirstateError>;
 
-    fn build_file_fold_map(&mut self) -> &FastHashMap<HgPathBuf, HgPathBuf>;
-
     fn set_all_dirs(&mut self) -> Result<(), DirstateMapError>;
 
     fn set_dirs(&mut self) -> Result<(), DirstateMapError>;
@@ -259,10 +256,6 @@ 
         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> {
         self.set_all_dirs()
     }
diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -19,7 +19,6 @@ 
 use crate::DirstateParents;
 use crate::DirstateStatus;
 use crate::EntryState;
-use crate::FastHashMap;
 use crate::HgPathCow;
 use crate::PatternFileWarning;
 use crate::StateMapIter;
@@ -566,10 +565,6 @@ 
         Ok(packed)
     }
 
-    fn build_file_fold_map(&mut self) -> &FastHashMap<HgPathBuf, HgPathBuf> {
-        todo!()
-    }
-
     fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> {
         // Do nothing, this `DirstateMap` does not a separate `all_dirs` that
         // needs to be recomputed
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -12,12 +12,9 @@ 
 use crate::{
     dirstate::{parsers::PARENT_SIZE, EntryState},
     pack_dirstate, parse_dirstate,
-    utils::{
-        files::normalize_case,
-        hg_path::{HgPath, HgPathBuf},
-    },
+    utils::hg_path::{HgPath, HgPathBuf},
     CopyMap, DirsMultiset, DirstateEntry, DirstateError, DirstateMapError,
-    DirstateParents, FastHashMap, StateMap,
+    DirstateParents, StateMap,
 };
 use micro_timer::timed;
 use std::collections::HashSet;
@@ -25,13 +22,10 @@ 
 use std::iter::FromIterator;
 use std::ops::Deref;
 
-pub type FileFoldMap = FastHashMap<HgPathBuf, HgPathBuf>;
-
 #[derive(Default)]
 pub struct DirstateMap {
     state_map: StateMap,
     pub copy_map: CopyMap,
-    file_fold_map: Option<FileFoldMap>,
     pub dirs: Option<DirsMultiset>,
     pub all_dirs: Option<DirsMultiset>,
     non_normal_set: Option<HashSet<HgPathBuf>>,
@@ -68,7 +62,6 @@ 
     pub fn clear(&mut self) {
         self.state_map = StateMap::default();
         self.copy_map.clear();
-        self.file_fold_map = None;
         self.non_normal_set = None;
         self.other_parent_set = None;
         self.set_parents(&DirstateParents {
@@ -134,9 +127,6 @@ 
             }
         }
 
-        if let Some(ref mut file_fold_map) = self.file_fold_map {
-            file_fold_map.remove(&normalize_case(filename));
-        }
         self.state_map.insert(
             filename.to_owned(),
             DirstateEntry {
@@ -171,9 +161,6 @@ 
                 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()
             .0
             .remove(filename);
@@ -381,21 +368,6 @@ 
         self.set_non_normal_other_parent_entries(true);
         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)]