diff --git a/apps/ltn/src/route_planner.rs b/apps/ltn/src/route_planner.rs index 9210d65fe5..7172e5998e 100644 --- a/apps/ltn/src/route_planner.rs +++ b/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, draw_routes: ToggleZoomed, labels: DrawRoadLabels, + // TODO We could save the no-filter variations map-wide + pathfinder_cache: PathfinderCache, } impl TripManagementState 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, ¶ms, 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, ¶ms, 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 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> { diff --git a/map_model/src/lib.rs b/map_model/src/lib.rs index d72dfe5d04..c3f58e87d2 100644 --- a/map_model/src/lib.rs +++ b/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}; diff --git a/map_model/src/map.rs b/map_model/src/map.rs index 7c8894f559..b93921f4cd 100644 --- a/map_model/src/map.rs +++ b/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. diff --git a/map_model/src/pathfind/mod.rs b/map_model/src/pathfind/mod.rs index f5b81e8d13..4dc1c20e43 100644 --- a/map_model/src/pathfind/mod.rs +++ b/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, diff --git a/map_model/src/pathfind/pathfinder.rs b/map_model/src/pathfind/pathfinder.rs index 475e1775b6..13f7c8a286 100644 --- a/map_model/src/pathfind/pathfinder.rs +++ b/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 { + 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 + } +}