Cache custom pathfinders directly for the LTN route planning tool. #852

apps/ltn/src/route_planner.rs

@@ -3,7 +3,7 @@ use map_gui::tools::{
     cmp_dist, cmp_duration, DrawRoadLabels, InputWaypoints, TripManagement, TripManagementState,
     WaypointID,
 };
-use map_model::{PathfinderCaching, NORMAL_LANE_THICKNESS};
+use map_model::{PathfinderCache, NORMAL_LANE_THICKNESS};
 use synthpop::{TripEndpoint, TripMode};
 use widgetry::mapspace::{ToggleZoomed, World};
 use widgetry::{
@@ -21,6 +21,8 @@ pub struct RoutePlanner {
     world: World<WaypointID>,
     draw_routes: ToggleZoomed,
     labels: DrawRoadLabels,
+    // TODO We could save the no-filter variations map-wide
+    pathfinder_cache: PathfinderCache,
 }
 
 impl TripManagementState<App> for RoutePlanner {
@@ -49,6 +51,7 @@ impl RoutePlanner {
             world: World::unbounded(),
             draw_routes: ToggleZoomed::empty(ctx),
             labels: DrawRoadLabels::only_major_roads(),
+            pathfinder_cache: PathfinderCache::new(),
         };
 
         if let Some(current_name) = &app.session.current_trip_name {
@@ -136,9 +139,10 @@ impl RoutePlanner {
                 if let Some((path, pl)) =
                     TripEndpoint::path_req(pair[0], pair[1], TripMode::Drive, map)
                         .and_then(|req| {
-                            map.pathfind_with_params(req, &params, PathfinderCaching::CacheDijkstra)
-                                .ok()
+                            self.pathfinder_cache
+                                .pathfind_with_params(map, req, params.clone())
                         })
+                        .and_then(|path| path.into_v1(map).ok())
                         .and_then(|path| path.trace(map).map(|pl| (path, pl)))
                 {
                     let shape = pl.make_polygons(5.0 * NORMAL_LANE_THICKNESS);
@@ -172,13 +176,15 @@ impl RoutePlanner {
             let color = colors::PLAN_ROUTE_BEFORE;
             let mut params = map.routing_params().clone();
             params.main_road_penalty = app.session.main_road_penalty;
+
             for pair in self.waypoints.get_waypoints().windows(2) {
                 if let Some((path, pl)) =
                     TripEndpoint::path_req(pair[0], pair[1], TripMode::Drive, map)
                         .and_then(|req| {
-                            map.pathfind_with_params(req, &params, PathfinderCaching::CacheDijkstra)
-                                .ok()
+                            self.pathfinder_cache
+                                .pathfind_with_params(map, req, params.clone())
                         })
+                        .and_then(|path| path.into_v1(map).ok())
                         .and_then(|path| path.trace(map).map(|pl| (path, pl)))
                 {
                     let shape = pl.make_polygons(5.0 * NORMAL_LANE_THICKNESS);
@@ -298,12 +304,6 @@ impl State<App> for RoutePlanner {
             self.labels.draw(g, app);
         }
     }
-
-    fn on_destroy(&mut self, _: &mut EventCtx, app: &mut App) {
-        // We'll cache a custom pathfinder per set of avoided roads. Avoid leaking memory by
-        // clearing this out
-        app.map.clear_custom_pathfinder_cache();
-    }
 }
 
 fn help() -> Vec<&'static str> {

map_model/src/lib.rs

@@ -61,7 +61,7 @@ pub use crate::objects::turn::{Turn, TurnID, TurnPriority, TurnType};
 pub use crate::objects::zone::{AccessRestrictions, Zone};
 pub use crate::pathfind::uber_turns::{IntersectionCluster, UberTurn};
 pub use crate::pathfind::{
-    Path, PathConstraints, PathRequest, PathStep, PathStepV2, PathV2, Pathfinder,
+    Path, PathConstraints, PathRequest, PathStep, PathStepV2, PathV2, Pathfinder, PathfinderCache,
     PathfinderCaching, RoutingParams,
 };
 pub use crate::traversable::{Position, Traversable, MAX_BIKE_SPEED, MAX_WALKING_SPEED};

map_model/src/map.rs

@@ -581,11 +581,6 @@ impl Map {
         self.pathfinder.should_use_transit(self, start, end)
     }
 
-    /// Clear any pathfinders with custom RoutingParams, created previously with `cache_custom`
-    pub fn clear_custom_pathfinder_cache(&self) {
-        self.pathfinder.clear_custom_pathfinder_cache();
-    }
-
     /// Return the cost of a single path, and also a mapping from every directed road to the cost
     /// of getting there from the same start. This can be used to understand why an alternative
     /// route wasn't chosen.

map_model/src/pathfind/mod.rs

@@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize};
 use geom::Duration;
 
 pub use self::engine::CreateEngine;
-pub use self::pathfinder::{Pathfinder, PathfinderCaching};
+pub use self::pathfinder::{Pathfinder, PathfinderCache, PathfinderCaching};
 pub use self::v1::{Path, PathRequest, PathStep};
 pub use self::v2::{PathStepV2, PathV2};
 pub use self::vehicles::vehicle_cost;
@@ -164,7 +164,7 @@ pub(crate) fn zone_cost(mvmnt: MovementID, constraints: PathConstraints, map: &M
 /// Tuneable parameters for all types of routing.
 // These will maybe become part of the PathRequest later, but that's an extremely invasive and
 // space-expensive change right now.
-#[derive(Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)]
 pub struct RoutingParams {
     // For all vehicles. This is added to the cost of a movement as an additional delay.
     pub unprotected_turn_penalty: Duration,

map_model/src/pathfind/pathfinder.rs

@@ -35,8 +35,7 @@ pub struct Pathfinder {
 
 /// When pathfinding with different `RoutingParams` is done, a temporary pathfinder must be
 /// created. This specifies what type of pathfinder and whether to cache it.
-///
-/// `clear_custom_pathfinder_cache` can be used to later clean up any cached pathfinders.
+// TODO Deprecated
 #[derive(Clone, Copy, PartialEq)]
 pub enum PathfinderCaching {
     /// Create a fast-to-build but slow-to-use Dijkstra-based pathfinder and don't cache it
@@ -261,14 +260,6 @@ impl Pathfinder {
         result
     }
 
-    // TODO Deprecated
-    pub fn clear_custom_pathfinder_cache(&self) {
-        self.cached_alternatives
-            .get_or(|| RefCell::new(VecMap::new()))
-            .borrow_mut()
-            .clear();
-    }
-
     pub fn all_costs_from(
         &self,
         req: PathRequest,
@@ -322,3 +313,40 @@ impl Pathfinder {
         timer.stop("apply edits to pedestrian using transit pathfinding");
     }
 }
+
+/// For callers needing to request paths with a variety of RoutingParams. The caller is in charge
+/// of the lifetime, so they can clear it out when appropriate.
+pub struct PathfinderCache {
+    cache: VecMap<(PathConstraints, RoutingParams), Pathfinder>,
+}
+
+impl PathfinderCache {
+    pub fn new() -> Self {
+        Self {
+            cache: VecMap::new(),
+        }
+    }
+
+    /// New pathfinders will be created as-needed using Dijkstra's, no spammy logging
+    pub fn pathfind_with_params(
+        &mut self,
+        map: &Map,
+        req: PathRequest,
+        params: RoutingParams,
+    ) -> Option<PathV2> {
+        if let Some(pathfinder) = self.cache.get(&(req.constraints, params.clone())) {
+            return pathfinder.pathfind_v2(req, map);
+        }
+
+        let pathfinder = Pathfinder::new_limited(
+            map,
+            params.clone(),
+            CreateEngine::Dijkstra,
+            vec![req.constraints],
+            &mut Timer::throwaway(),
+        );
+        let result = pathfinder.pathfind_v2(req.clone(), map);
+        self.cache.push((req.constraints, params), pathfinder);
+        result
+    }
+}