Work queue optimization to make time complexity less than quadratic

jj-vcs · Jan 16, 2023 · 2fa1275 · 2fa1275
1 parent 38d9f34
commit 2fa1275
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
@@ -137,21 +137,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 0276d3d7c24d b
     Duplicated ebd06dba20ec as 9dbeec2f035d d
+    Duplicated 1394f625cbbd as 0276d3d7c24d b
     Duplicated 921dde6e55c0 as 137eb3f539b4 e
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     o   137eb3f539b4   e
     |\  
-    o | 9dbeec2f035d   d
-    | o 0276d3d7c24d   b
+    o | 0276d3d7c24d   b
+    | o 9dbeec2f035d   d
     | | @   921dde6e55c0   e
     | | |\  
     | | o | ebd06dba20ec   d
     | |/ /  
-    |/| |   
-    o | | c0cb3a0b73e7   c
+    | o | c0cb3a0b73e7   c
     |/ /  
     | o 1394f625cbbd   b
     |/  
@@ -173,26 +172,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 78ad46d75eb9 a
     Duplicated ebd06dba20ec as ec62710c4d22 d
     Duplicated 921dde6e55c0 as c2ced31735e8 e
-    Duplicated 2443ea76b0b1 as 78ad46d75eb9 a
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    o 78ad46d75eb9   a
-    | o   c2ced31735e8   e
-    | |\  
-    | o | ec62710c4d22   d
+    o   c2ced31735e8   e
+    |\  
+    o | ec62710c4d22   d
+    | | o 78ad46d75eb9   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)
     "###);
@@ -202,17 +201,17 @@ fn test_duplicate_many() {
     let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "a:"]);
     insta::assert_snapshot!(stdout, @r###"
     Duplicated 2443ea76b0b1 as c6f7f8c4512e a
+    Duplicated 1394f625cbbd as aa2687406dd2 b
     Duplicated c0cb3a0b73e7 as 08844614d525 c
     Duplicated ebd06dba20ec as e26642da6fa0 d
-    Duplicated 1394f625cbbd as aa2687406dd2 b
     Duplicated 921dde6e55c0 as 3e94fa8469f7 e
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     o   3e94fa8469f7   e
     |\  
-    o | aa2687406dd2   b
-    | o e26642da6fa0   d
-    | o 08844614d525   c
+    o | e26642da6fa0   d
+    o | 08844614d525   c
+    | o aa2687406dd2   b
     |/  
     o c6f7f8c4512e   a
     | @   921dde6e55c0   e