jj duplicate: Optimization to make time complexity less than quadratic

jj-vcs · Jan 17, 2023 · cd1d0ab · cd1d0ab
1 parent 2a25b2a
commit cd1d0ab
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 38 deletions.
diff --git a/src/commands.rs b/src/commands.rs
@@ -2060,27 +2060,36 @@ fn cmd_duplicate(
     let mut tx = workspace_command
         .start_transaction(&format!("duplicating {} commit(s)", to_duplicate.len()));
     let mut_repo = tx.mut_repo();
-    while !to_duplicate.is_empty() {
-        let mut original_commit = None;
+
+    let mut work_stack = vec![];
+    while let Some(next_commit_if_work_stack_empty) = to_duplicate.last() {
         // Find a commit whose parents we either don't plan to duplicate or have already
-        // duplicated.
-        // This reimplements the heads() revset function, but we will optimize it
-        // shortly.
-        for commit in to_duplicate.iter() {
-            if !commit
+        // duplicated. The optimization of following a chain of parents makes this
+        // linear in the number of commits to duplicate.
+        let mut counter = 0;
+        let original_commit = loop {
+            if counter > to_duplicate.len() {
+                panic!(
+                    "Found a cycle: every commit has a parent that is also in the set of commits \
+                     to duplicate"
+                );
+            }
+            counter += 1;
+            let candidate = work_stack
+                .pop()
+                .unwrap_or_else(|| next_commit_if_work_stack_empty.clone());
+            if let Some(parent_in_set) = candidate
                 .parents()
-                .iter()
-                .any(|parent| to_duplicate.contains(parent))
+                .into_iter()
+                .find(|parent| to_duplicate.contains(parent))
             {
-                original_commit = Some(commit.clone());
-                break;
+                work_stack.push(candidate);
+                work_stack.push(parent_in_set);
+            } else {
+                break candidate;
             }
-        }
+        };
 
-        let original_commit = original_commit.expect(
-            "Found a cycle: every commit has a parent that is also in the set of commits to \
-             duplicate",
-        );
         let new_parents = original_commit
             .parents()
             .iter()

diff --git a/tests/test_duplicate_command.rs b/tests/test_duplicate_command.rs
@@ -156,21 +156,20 @@ fn test_duplicate_many() {
     test_env.jj_cmd_success(&repo_path, &["undo"]);
     let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "b:", "d:"]);
     insta::assert_snapshot!(stdout, @r###"
-    Duplicated 1394f625cbbd as fa167d18a83a b
     Duplicated ebd06dba20ec as 2181781b4f81 d
+    Duplicated 1394f625cbbd as fa167d18a83a b
     Duplicated 921dde6e55c0 as 0f7430f2727a e
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     o   0f7430f2727a   e
     |\  
-    o | 2181781b4f81   d
-    | o fa167d18a83a   b
+    o | fa167d18a83a   b
+    | o 2181781b4f81   d
     | | @   921dde6e55c0   e
     | | |\  
     | | o | ebd06dba20ec   d
     | |/ /  
-    |/| |   
-    o | | c0cb3a0b73e7   c
+    | o | c0cb3a0b73e7   c
     |/ /  
     | o 1394f625cbbd   b
     |/  
@@ -192,26 +191,26 @@ fn test_duplicate_many() {
     "###);
     let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "d:", "a"]);
     insta::assert_snapshot!(stdout, @r###"
+    Duplicated 2443ea76b0b1 as c6f7f8c4512e a
     Duplicated ebd06dba20ec as d94e4c55a68b d
     Duplicated 921dde6e55c0 as 9bd4389f5d47 e
-    Duplicated 2443ea76b0b1 as c6f7f8c4512e a
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    o c6f7f8c4512e   a
-    | o   9bd4389f5d47   e
-    | |\  
-    | o | d94e4c55a68b   d
+    o   9bd4389f5d47   e
+    |\  
+    o | d94e4c55a68b   d
+    | | o c6f7f8c4512e   a
     | | | @   921dde6e55c0   e
     | | | |\  
-    | | | |/  
-    | | |/|   
+    | | |_|/  
+    | |/| |   
     | | | o ebd06dba20ec   d
-    | | |/  
-    | |/|   
-    | o | c0cb3a0b73e7   c
-    | | o 1394f625cbbd   b
-    | |/  
-    | o 2443ea76b0b1   a
+    | |_|/  
+    |/| |   
+    o | | c0cb3a0b73e7   c
+    | o | 1394f625cbbd   b
+    |/ /  
+    o | 2443ea76b0b1   a
     |/  
     o 000000000000   (no description set)
     "###);
@@ -221,17 +220,17 @@ fn test_duplicate_many() {
     let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "a:"]);
     insta::assert_snapshot!(stdout, @r###"
     Duplicated 2443ea76b0b1 as 0fe67a05989e a
+    Duplicated 1394f625cbbd as e13ac0adabdf b
     Duplicated c0cb3a0b73e7 as df53fa589286 c
     Duplicated ebd06dba20ec as 2f2442db08eb d
-    Duplicated 1394f625cbbd as e13ac0adabdf b
     Duplicated 921dde6e55c0 as ee8fe64ed254 e
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     o   ee8fe64ed254   e
     |\  
-    o | e13ac0adabdf   b
-    | o 2f2442db08eb   d
-    | o df53fa589286   c
+    o | 2f2442db08eb   d
+    o | df53fa589286   c
+    | o e13ac0adabdf   b
     |/  
     o 0fe67a05989e   a
     | @   921dde6e55c0   e