Speed up apply_edits. Instead of undo()ing all current commands and then (#299)

apply()ing the new ones, ignore any common prefix between the two
command-lists.

map_model/src/edits/mod.rs

@@ -215,12 +215,12 @@ impl EditCmd {
         }
     }
 
-    // Must be idempotent. True if it actually did anything.
+    // Must be idempotent
-    fn apply(&self, effects: &mut EditEffects, map: &mut Map, timer: &mut Timer) -> bool {
+    fn apply(&self, effects: &mut EditEffects, map: &mut Map, timer: &mut Timer) {
         match self {
             EditCmd::ChangeRoad { r, ref new, .. } => {
                 if map.get_r_edit(*r) == new.clone() {
-                    return false;
+                    return;
                 }
 
                 let road = &mut map.roads[r.0];
@@ -259,7 +259,6 @@ impl EditCmd {
 
                     recalculate_turns(i.id, map, effects, timer);
                 }
-                true
             }
             EditCmd::ChangeIntersection {
                 i,
@@ -267,7 +266,7 @@ impl EditCmd {
                 ref old,
             } => {
                 if map.get_i_edit(*i) == new.clone() {
-                    return false;
+                    return;
                 }
 
                 map.stop_signs.remove(i);
@@ -296,44 +295,30 @@ impl EditCmd {
                 if old == &EditIntersection::Closed || new == &EditIntersection::Closed {
                     recalculate_turns(*i, map, effects, timer);
                 }
-                true
             }
             EditCmd::ChangeRouteSchedule { id, new, .. } => {
                 map.bus_routes[id.0].spawn_times = new.clone();
-                true
             }
         }
     }
 
-    // Must be idempotent. True if it actually did anything.
+    fn undo(self) -> EditCmd {
-    fn undo(&self, effects: &mut EditEffects, map: &mut Map, timer: &mut Timer) -> bool {
         match self {
-            EditCmd::ChangeRoad {
+            EditCmd::ChangeRoad { r, old, new } => EditCmd::ChangeRoad {
                 r,
-                ref old,
+                old: new,
-                ref new,
+                new: old,
-            } => EditCmd::ChangeRoad {
+            },
-                r: *r,
+            EditCmd::ChangeIntersection { i, old, new } => EditCmd::ChangeIntersection {
-                old: new.clone(),
-                new: old.clone(),
-            }
-            .apply(effects, map, timer),
-            EditCmd::ChangeIntersection {
                 i,
-                ref old,
+                old: new,
-                ref new,
+                new: old,
-            } => EditCmd::ChangeIntersection {
+            },
-                i: *i,
-                old: new.clone(),
-                new: old.clone(),
-            }
-            .apply(effects, map, timer),
             EditCmd::ChangeRouteSchedule { id, old, new } => EditCmd::ChangeRouteSchedule {
-                id: *id,
+                id,
-                old: new.clone(),
+                old: new,
-                new: old.clone(),
+                new: old,
-            }
+            },
-            .apply(effects, map, timer),
         }
     }
 }
@@ -469,33 +454,35 @@ impl Map {
         BTreeSet<TurnID>,
         BTreeSet<IntersectionID>,
     ) {
-        // TODO More efficient ways to do this: given two sets of edits, produce a smaller diff.
-        // Simplest strategy: Remove common prefix.
         let mut effects = EditEffects::new();
 
-        // First undo all existing edits.
+        // We need to undo() all of the current commands in reverse order, then apply() all of the
-        let mut undo = std::mem::replace(&mut self.edits.commands, Vec::new());
+        // new commands. But in many cases, new_edits is just the current edits with a few commands
-        undo.reverse();
+        // at the end. So a simple optimization with equivalent behavior is to skip the common
-        let mut undid = 0;
+        // prefix of commands.
-        for cmd in &undo {
+        let mut start_at_idx = 0;
-            if cmd.undo(&mut effects, self, timer) {
+        for (cmd1, cmd2) in self.edits.commands.iter().zip(new_edits.commands.iter()) {
-                undid += 1;
+            if cmd1 == cmd2 {
+                start_at_idx += 1;
+            } else {
+                break;
             }
         }
-        timer.note(format!("Undid {} / {} existing edits", undid, undo.len()));
+
+        // Undo existing edits
+        for _ in start_at_idx..self.edits.commands.len() {
+            self.edits
+                .commands
+                .pop()
+                .unwrap()
+                .undo()
+                .apply(&mut effects, self, timer);
+        }
 
         // Apply new edits.
-        let mut applied = 0;
+        for cmd in &new_edits.commands[start_at_idx..] {
-        for cmd in &new_edits.commands {
+            cmd.apply(&mut effects, self, timer);
-            if cmd.apply(&mut effects, self, timer) {
-                applied += 1;
         }
-        }
-        timer.note(format!(
-            "Applied {} / {} new edits",
-            applied,
-            new_edits.commands.len()
-        ));
 
         // Might need to update bus stops.
         if enforce_valid {