From 2905ca605d7fb76f0f0080b3a0f8545a2f41f2d8 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Wed, 29 Sep 2021 11:38:18 -0700 Subject: [PATCH] Cache the Dijkstra pathfinder for custom RoutingParams. This dramatically speeds up the bike network routing tool, since it's now restricted to just a few params. #743 --- game/src/debug/routes.rs | 7 ++- game/src/ungap/route/results.rs | 2 +- map_model/src/map.rs | 13 +++- map_model/src/pathfind/pathfinder.rs | 92 ++++++++++++++++++++-------- 4 files changed, 85 insertions(+), 29 deletions(-) diff --git a/game/src/debug/routes.rs b/game/src/debug/routes.rs index a4594f75c2..32869b0f7e 100644 --- a/game/src/debug/routes.rs +++ b/game/src/debug/routes.rs @@ -55,7 +55,12 @@ impl RouteExplorer { if let Some((ref goal, _, ref mut preview)) = self.goal { *preview = Drawable::empty(ctx); if let Some(polygon) = TripEndpoint::path_req(self.start, *goal, mode, &app.primary.map) - .and_then(|req| app.primary.map.pathfind_with_params(req, ¶ms).ok()) + .and_then(|req| { + app.primary + .map + .pathfind_with_params(req, ¶ms, false) + .ok() + }) .and_then(|path| path.trace(&app.primary.map)) .map(|pl| pl.make_polygons(NORMAL_LANE_THICKNESS)) { diff --git a/game/src/ungap/route/results.rs b/game/src/ungap/route/results.rs index c804252366..555d07751f 100644 --- a/game/src/ungap/route/results.rs +++ b/game/src/ungap/route/results.rs @@ -89,7 +89,7 @@ impl RouteResults { for pair in waypoints.windows(2) { if let Some(path) = TripEndpoint::path_req(pair[0], pair[1], TripMode::Bike, map) - .and_then(|req| map.pathfind_with_params(req, &routing_params).ok()) + .and_then(|req| map.pathfind_with_params(req, &routing_params, true).ok()) { total_distance += path.total_length(); total_time += path.estimate_duration(map, Some(map_model::MAX_BIKE_SPEED)); diff --git a/map_model/src/map.rs b/map_model/src/map.rs index 64bbf98ff1..32437ad20c 100644 --- a/map_model/src/map.rs +++ b/map_model/src/map.rs @@ -570,8 +570,14 @@ impl Map { pub fn pathfind(&self, req: PathRequest) -> Result { self.pathfind_v2(req)?.into_v1(self) } - pub fn pathfind_with_params(&self, req: PathRequest, params: &RoutingParams) -> Result { - self.pathfind_v2_with_params(req, params)?.into_v1(self) + pub fn pathfind_with_params( + &self, + req: PathRequest, + params: &RoutingParams, + cache_custom: bool, + ) -> Result { + self.pathfind_v2_with_params(req, params, cache_custom)? + .into_v1(self) } pub fn pathfind_v2(&self, req: PathRequest) -> Result { assert!(!self.pathfinder_dirty); @@ -583,10 +589,11 @@ impl Map { &self, req: PathRequest, params: &RoutingParams, + cache_custom: bool, ) -> Result { assert!(!self.pathfinder_dirty); self.pathfinder - .pathfind_with_params(req.clone(), params, self) + .pathfind_with_params(req.clone(), params, cache_custom, self) .ok_or_else(|| anyhow!("can't fulfill {}", req)) } pub fn should_use_transit( diff --git a/map_model/src/pathfind/pathfinder.rs b/map_model/src/pathfind/pathfinder.rs index 18e9a6dce4..49b5257af9 100644 --- a/map_model/src/pathfind/pathfinder.rs +++ b/map_model/src/pathfind/pathfinder.rs @@ -1,8 +1,10 @@ +use std::cell::RefCell; use std::collections::HashMap; use serde::{Deserialize, Serialize}; +use thread_local::ThreadLocal; -use abstutil::Timer; +use abstutil::{Timer, VecMap}; use geom::Duration; use crate::pathfind::engine::CreateEngine; @@ -13,7 +15,7 @@ use crate::{ RoutingParams, }; -#[derive(Clone, Serialize, Deserialize)] +#[derive(Serialize, Deserialize)] pub struct Pathfinder { car_graph: VehiclePathfinder, bike_graph: VehiclePathfinder, @@ -22,7 +24,29 @@ pub struct Pathfinder { walking_graph: SidewalkPathfinder, walking_with_transit_graph: SidewalkPathfinder, + // These params cover the main graphs params: RoutingParams, + + // Callers can opt into caching with pathfind_with_params + // TODO VecMap is probably fast enough. RoutingParams is annoying to implement Hash. + #[serde(skip_serializing, skip_deserializing)] + cached_alternatives: ThreadLocal>>, +} + +// Implemented manually to deal with the ThreadLocal +impl Clone for Pathfinder { + fn clone(&self) -> Self { + Self { + car_graph: self.car_graph.clone(), + bike_graph: self.bike_graph.clone(), + bus_graph: self.bus_graph.clone(), + train_graph: self.train_graph.clone(), + walking_graph: self.walking_graph.clone(), + walking_with_transit_graph: self.walking_with_transit_graph.clone(), + params: self.params.clone(), + cached_alternatives: ThreadLocal::new(), + } + } } impl Pathfinder { @@ -37,6 +61,7 @@ impl Pathfinder { walking_graph: SidewalkPathfinder::empty(), walking_with_transit_graph: SidewalkPathfinder::empty(), params: RoutingParams::default(), + cached_alternatives: ThreadLocal::new(), } } @@ -94,6 +119,7 @@ impl Pathfinder { walking_with_transit_graph, params, + cached_alternatives: ThreadLocal::new(), } } @@ -131,40 +157,58 @@ impl Pathfinder { /// Finds a path from a start to an end for a certain type of agent. pub fn pathfind(&self, req: PathRequest, map: &Map) -> Option { - self.pathfind_with_params(req, map.routing_params(), map) + self.pathfind_with_params(req, map.routing_params(), false, map) } /// Finds a path from a start to an end for a certain type of agent. May use custom routing - /// parameters. + /// parameters. If caching is requested and custom routing parameters are used, then the + /// intermediate graph is saved to speed up future calls with the same routing parameters. pub fn pathfind_with_params( &self, req: PathRequest, params: &RoutingParams, + cache_custom: bool, map: &Map, ) -> Option { - if params != &self.params { - // If the params differ from the ones baked into the map, the CHs won't match. This - // should only be happening from the debug UI; be very obnoxious if we start calling it - // from the simulation or something else. - let mut timer = - Timer::new(format!("Pathfinding slowly for {} with custom params", req)); - let tmp_pathfinder = Pathfinder::new_for_one_mode( - map, - params.clone(), - CreateEngine::Dijkstra, - req.constraints, - &mut timer, - ); - return tmp_pathfinder.pathfind_with_params(req, params, map); + let constraints = req.constraints; + if params == &self.params { + return match constraints { + PathConstraints::Pedestrian => self.walking_graph.pathfind(req, map), + PathConstraints::Car => self.car_graph.pathfind(req, map), + PathConstraints::Bike => self.bike_graph.pathfind(req, map), + PathConstraints::Bus => self.bus_graph.pathfind(req, map), + PathConstraints::Train => self.train_graph.pathfind(req, map), + }; } - match req.constraints { - PathConstraints::Pedestrian => self.walking_graph.pathfind(req, map), - PathConstraints::Car => self.car_graph.pathfind(req, map), - PathConstraints::Bike => self.bike_graph.pathfind(req, map), - PathConstraints::Bus => self.bus_graph.pathfind(req, map), - PathConstraints::Train => self.train_graph.pathfind(req, map), + // If the params differ from the ones baked into the map, the CHs won't match. Do we have a + // cached alternative? + if let Some(alt) = self + .cached_alternatives + .get_or(|| RefCell::new(VecMap::new())) + .borrow() + .get(&(constraints, params.clone())) + { + return alt.pathfind_with_params(req, params, false, map); } + + // If somebody's repeatedly calling this without caching, log very obnoxiously. + let mut timer = Timer::new(format!("Pathfinding slowly for {} with custom params", req)); + let tmp_pathfinder = Pathfinder::new_for_one_mode( + map, + params.clone(), + CreateEngine::Dijkstra, + constraints, + &mut timer, + ); + let result = tmp_pathfinder.pathfind_with_params(req, params, false, map); + if cache_custom { + self.cached_alternatives + .get_or(|| RefCell::new(VecMap::new())) + .borrow_mut() + .push((constraints, params.clone()), tmp_pathfinder); + } + result } pub fn all_costs_from(