[Request] [+-- ] D8774: hg-core: make parse_dirstate return result rather than update inplace

[acezar (Antoine Cezar)]

acezar created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Return a vec rather than updating a hashmap which is faster when then hashmap is
  not needed like in `hg files` which just list tracked files.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/dirstate/parsers.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/dirstate/parsers.rs b/rust/hg-core/src/dirstate/parsers.rs
--- a/rust/hg-core/src/dirstate/parsers.rs
+++ b/rust/hg-core/src/dirstate/parsers.rs
@@ -4,6 +4,7 @@
 // GNU General Public License version 2 or any later version.
 
 use crate::utils::hg_path::HgPath;
+use crate::utils::hg_path::HgPathBuf;
 use crate::{
     dirstate::{CopyMap, EntryState, StateMap},
     DirstateEntry, DirstatePackError, DirstateParents, DirstateParseError,
@@ -19,17 +20,21 @@
 /// Dirstate entries have a static part of 8 + 32 + 32 + 32 + 32 bits.
 const MIN_ENTRY_SIZE: usize = 17;
 
-// TODO parse/pack: is mutate-on-loop better for performance?
+type ParseResult = (
+    DirstateParents,
+    Vec<(HgPathBuf, DirstateEntry)>,
+    Vec<(HgPathBuf, HgPathBuf)>,
+);
 
 #[timed]
 pub fn parse_dirstate(
-    state_map: &mut StateMap,
-    copy_map: &mut CopyMap,
     contents: &[u8],
-) -> Result<DirstateParents, DirstateParseError> {
+) -> Result<ParseResult, DirstateParseError> {
     if contents.len() < PARENT_SIZE * 2 {
         return Err(DirstateParseError::TooLittleData);
     }
+    let mut copies: Vec<(HgPathBuf, HgPathBuf)> = vec![];
+    let mut entries: Vec<(HgPathBuf, DirstateEntry)> = vec![];
 
     let mut curr_pos = PARENT_SIZE * 2;
     let parents = DirstateParents {
@@ -63,12 +68,12 @@
         };
 
         if let Some(copy_path) = copy {
-            copy_map.insert(
-                HgPath::new(path).to_owned(),
-                HgPath::new(copy_path).to_owned(),
-            );
+            copies.push((
+                HgPathBuf::from_bytes(path),
+                HgPathBuf::from_bytes(copy_path),
+            ));
         };
-        state_map.insert(
+        entries.push((
             HgPath::new(path).to_owned(),
             DirstateEntry {
                 state,
@@ -76,11 +81,11 @@
                 size,
                 mtime,
             },
-        );
+        ));
         curr_pos = curr_pos + MIN_ENTRY_SIZE + (path_len);
     }
 
-    Ok(parents)
+    Ok((parents, entries, copies))
 }
 
 /// `now` is the duration in seconds since the Unix epoch
@@ -287,12 +292,11 @@
 
         let mut new_state_map: StateMap = FastHashMap::default();
         let mut new_copy_map: CopyMap = FastHashMap::default();
-        let new_parents = parse_dirstate(
-            &mut new_state_map,
-            &mut new_copy_map,
-            result.as_slice(),
-        )
-        .unwrap();
+        let (new_parents, entries, copies) =
+            parse_dirstate(result.as_slice()).unwrap();
+        new_state_map.extend(entries);
+        new_copy_map.extend(copies);
+
         assert_eq!(
             (parents, state_map, copymap),
             (new_parents, new_state_map, new_copy_map)
@@ -362,12 +366,11 @@
 
         let mut new_state_map: StateMap = FastHashMap::default();
         let mut new_copy_map: CopyMap = FastHashMap::default();
-        let new_parents = parse_dirstate(
-            &mut new_state_map,
-            &mut new_copy_map,
-            result.as_slice(),
-        )
-        .unwrap();
+        let (new_parents, entries, copies) =
+            parse_dirstate(result.as_slice()).unwrap();
+        new_state_map.extend(entries);
+        new_copy_map.extend(copies);
+
         assert_eq!(
             (parents, state_map, copymap),
             (new_parents, new_state_map, new_copy_map)
@@ -405,12 +408,10 @@
 
         let mut new_state_map: StateMap = FastHashMap::default();
         let mut new_copy_map: CopyMap = FastHashMap::default();
-        let new_parents = parse_dirstate(
-            &mut new_state_map,
-            &mut new_copy_map,
-            result.as_slice(),
-        )
-        .unwrap();
+        let (new_parents, entries, copies) =
+            parse_dirstate(result.as_slice()).unwrap();
+        new_state_map.extend(entries);
+        new_copy_map.extend(copies);
 
         assert_eq!(
             (
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
@@ -364,11 +364,9 @@
             return Ok(None);
         }
 
-        let parents = parse_dirstate(
-            &mut self.state_map,
-            &mut self.copy_map,
-            file_contents,
-        )?;
+        let (parents, entries, copies) = parse_dirstate(file_contents)?;
+        self.state_map.extend(entries);
+        self.copy_map.extend(copies);
 
         if !self.dirty_parents {
             self.set_parents(&parents);



To: acezar, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20200721/879eafeb/attachment-0001.html>