Patchwork D9425: copies-rust: move the mapping merging into a else clause

login
register
mail settings
Submitter phabricator
Date Nov. 27, 2020, 4:12 p.m.
Message ID <differential-rev-PHID-DREV-j62a2behq4xbsoegjn4y-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/47704/
State Superseded
Headers show

Comments

phabricator - Nov. 27, 2020, 4:12 p.m.
Alphare created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  We are going to add more cases, to it is time to stop using early returns and to
  move everything in a single if/elif/else block for clarity.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/copy_tracing.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-core/src/copy_tracing.rs b/rust/hg-core/src/copy_tracing.rs
--- a/rust/hg-core/src/copy_tracing.rs
+++ b/rust/hg-core/src/copy_tracing.rs
@@ -497,73 +497,74 @@ 
     oracle: &mut AncestorOracle<A>,
 ) -> TimeStampedPathCopies {
     if minor.is_empty() {
-        return major;
+        major
     } else if major.is_empty() {
-        return minor;
-    }
-    let mut override_minor = Vec::new();
-    let mut override_major = Vec::new();
+        minor
+    } else {
+        let mut override_minor = Vec::new();
+        let mut override_major = Vec::new();
 
-    let mut to_major = |k: &HgPathBuf, v: &TimeStampedPathCopy| {
-        override_major.push((k.clone(), v.clone()))
-    };
-    let mut to_minor = |k: &HgPathBuf, v: &TimeStampedPathCopy| {
-        override_minor.push((k.clone(), v.clone()))
-    };
+        let mut to_major = |k: &HgPathBuf, v: &TimeStampedPathCopy| {
+            override_major.push((k.clone(), v.clone()))
+        };
+        let mut to_minor = |k: &HgPathBuf, v: &TimeStampedPathCopy| {
+            override_minor.push((k.clone(), v.clone()))
+        };
 
-    // The diff function leverage detection of the identical subpart if minor
-    // and major has some common ancestors. This make it very fast is most
-    // case.
-    //
-    // In case where the two map are vastly different in size, the current
-    // approach is still slowish because the iteration will iterate over
-    // all the "exclusive" content of the larger on. This situation can be
-    // frequent when the subgraph of revision we are processing has a lot
-    // of roots. Each roots adding they own fully new map to the mix (and
-    // likely a small map, if the path from the root to the "main path" is
-    // small.
-    //
-    // We could do better by detecting such situation and processing them
-    // differently.
-    for d in minor.diff(&major) {
-        match d {
-            DiffItem::Add(k, v) => to_minor(k, v),
-            DiffItem::Remove(k, v) => to_major(k, v),
-            DiffItem::Update { old, new } => {
-                let (dest, src_major) = new;
-                let (_, src_minor) = old;
-                match compare_value(
-                    changes, oracle, dest, src_minor, src_major,
-                ) {
-                    MergePick::Major => to_minor(dest, src_major),
-                    MergePick::Minor => to_major(dest, src_minor),
-                    // If the two entry are identical, no need to do
-                    // anything (but diff should not have yield them)
-                    MergePick::Any => unreachable!(),
+        // The diff function leverage detection of the identical subpart if
+        // minor and major has some common ancestors. This make it very
+        // fast is most case.
+        //
+        // In case where the two map are vastly different in size, the current
+        // approach is still slowish because the iteration will iterate over
+        // all the "exclusive" content of the larger on. This situation can be
+        // frequent when the subgraph of revision we are processing has a lot
+        // of roots. Each roots adding they own fully new map to the mix (and
+        // likely a small map, if the path from the root to the "main path" is
+        // small.
+        //
+        // We could do better by detecting such situation and processing them
+        // differently.
+        for d in minor.diff(&major) {
+            match d {
+                DiffItem::Add(k, v) => to_minor(k, v),
+                DiffItem::Remove(k, v) => to_major(k, v),
+                DiffItem::Update { old, new } => {
+                    let (dest, src_major) = new;
+                    let (_, src_minor) = old;
+                    match compare_value(
+                        changes, oracle, dest, src_minor, src_major,
+                    ) {
+                        MergePick::Major => to_minor(dest, src_major),
+                        MergePick::Minor => to_major(dest, src_minor),
+                        // If the two entry are identical, no need to do
+                        // anything (but diff should not have yield them)
+                        MergePick::Any => unreachable!(),
+                    }
                 }
-            }
-        };
-    }
+            };
+        }
 
-    let updates;
-    let mut result;
-    if override_major.is_empty() {
-        result = major
-    } else if override_minor.is_empty() {
-        result = minor
-    } else {
-        if override_minor.len() < override_major.len() {
-            updates = override_minor;
-            result = minor;
+        let updates;
+        let mut result;
+        if override_major.is_empty() {
+            result = major
+        } else if override_minor.is_empty() {
+            result = minor
         } else {
-            updates = override_major;
-            result = major;
+            if override_minor.len() < override_major.len() {
+                updates = override_minor;
+                result = minor;
+            } else {
+                updates = override_major;
+                result = major;
+            }
+            for (k, v) in updates {
+                result.insert(k, v);
+            }
         }
-        for (k, v) in updates {
-            result.insert(k, v);
-        }
+        result
     }
-    result
 }
 
 /// represent the side that should prevail when merging two