Use a HashMap to store driving queues. Modest speedup on the downtown

benchmark from ~101s to ~96s. #368

abstutil/src/serde.rs

@@ -2,32 +2,37 @@ use crate::MultiMap;
 use serde::de::DeserializeOwned;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use std::cmp::Ord;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashMap};
 use std::convert::TryFrom;
 use std::error::Error;
 
+/// Stringifies an object to nicely formatted JSON.
 pub fn to_json<T: Serialize>(obj: &T) -> String {
     serde_json::to_string_pretty(obj).unwrap()
 }
 
+/// Stringifies an object to terse JSON.
 pub fn to_json_terse<T: Serialize>(obj: &T) -> String {
     serde_json::to_string(obj).unwrap()
 }
 
+/// Deserializes an object from a JSON string.
 pub fn from_json<T: DeserializeOwned>(raw: &Vec<u8>) -> Result<T, Box<dyn Error>> {
     serde_json::from_slice(raw).map_err(|x| x.into())
 }
 
+/// Deserializes an object from the bincode format.
 pub fn from_binary<T: DeserializeOwned>(raw: &Vec<u8>) -> Result<T, Box<dyn Error>> {
     bincode::deserialize(raw).map_err(|x| x.into())
 }
 
+/// The number of bytes for an object serialized to bincode.
 pub fn serialized_size_bytes<T: Serialize>(obj: &T) -> usize {
     bincode::serialized_size(obj).unwrap() as usize
 }
 
-// For BTreeMaps with struct keys. See https://github.com/serde-rs/json/issues/402.
-
+/// Serializes a BTreeMap as a list of tuples. Necessary when the keys are structs; see
+/// https://github.com/serde-rs/json/issues/402.
 pub fn serialize_btreemap<S: Serializer, K: Serialize, V: Serialize>(
     map: &BTreeMap<K, V>,
     s: S,
@@ -35,6 +40,8 @@ pub fn serialize_btreemap<S: Serializer, K: Serialize, V: Serialize>(
     map.iter().collect::<Vec<(_, _)>>().serialize(s)
 }
 
+/// Deserializes a BTreeMap from a list of tuples. Necessary when the keys are structs; see
+/// https://github.com/serde-rs/json/issues/402.
 pub fn deserialize_btreemap<
     'de,
     D: Deserializer<'de>,
@@ -51,6 +58,35 @@ pub fn deserialize_btreemap<
     Ok(map)
 }
 
+/// Serializes a HashMap as a list of tuples, first sorting by the keys. This ensures the
+/// serialized form is deterministic.
+pub fn serialize_hashmap<S: Serializer, K: Serialize + Ord, V: Serialize>(
+    map: &HashMap<K, V>,
+    s: S,
+) -> Result<S::Ok, S::Error> {
+    let mut list: Vec<(&K, &V)> = map.iter().collect();
+    list.sort_by_key(|(k, _)| k.clone());
+    list.serialize(s)
+}
+
+/// Deserializes a HashMap from a list of tuples.
+pub fn deserialize_hashmap<
+    'de,
+    D: Deserializer<'de>,
+    K: Deserialize<'de> + std::hash::Hash + Eq,
+    V: Deserialize<'de>,
+>(
+    d: D,
+) -> Result<HashMap<K, V>, D::Error> {
+    let vec = <Vec<(K, V)>>::deserialize(d)?;
+    let mut map = HashMap::new();
+    for (k, v) in vec {
+        map.insert(k, v);
+    }
+    Ok(map)
+}
+
+/// Serializes a MultiMap.
 pub fn serialize_multimap<
     S: Serializer,
     K: Serialize + Eq + Ord + Clone,
@@ -63,6 +99,7 @@ pub fn serialize_multimap<
     map.raw_map().iter().collect::<Vec<(_, _)>>().serialize(s)
 }
 
+/// Deserializes a MultiMap.
 pub fn deserialize_multimap<
     'de,
     D: Deserializer<'de>,
@@ -81,6 +118,8 @@ pub fn deserialize_multimap<
     Ok(map)
 }
 
+/// Serializes a `usize` as a `u32` to save space. Useful when you need `usize` for indexing, but
+/// the values don't exceed 2^32.
 pub fn serialize_usize<S: Serializer>(x: &usize, s: S) -> Result<S::Ok, S::Error> {
     if let Ok(x) = u32::try_from(*x) {
         x.serialize(s)
@@ -89,6 +128,7 @@ pub fn serialize_usize<S: Serializer>(x: &usize, s: S) -> Result<S::Ok, S::Error
     }
 }
 
+/// Deserializes a `usize` from a `u32`.
 pub fn deserialize_usize<'de, D: Deserializer<'de>>(d: D) -> Result<usize, D::Error> {
     let x = <u32>::deserialize(d)?;
     Ok(x as usize)

sim/src/mechanics/driving.rs

@@ -1,8 +1,8 @@
-use std::collections::{BTreeMap, BTreeSet, HashSet, VecDeque};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque};
 
 use serde::{Deserialize, Serialize};
 
-use abstutil::{deserialize_btreemap, serialize_btreemap, FixedMap, IndexableKey};
+use abstutil::{deserialize_hashmap, serialize_hashmap, FixedMap, IndexableKey};
 use geom::{Distance, Duration, PolyLine, Speed, Time};
 use map_model::{LaneID, Map, Path, PathStep, Traversable};
 
@@ -30,11 +30,14 @@ pub struct DrivingSimState {
     // gain much faster lookup, which has shown dramatic speedups in the scenarios being run so
     // far.
     cars: FixedMap<CarID, Car>,
+    // Note this uses a HashMap for faster lookup. Although the order of iterating over the HashMap
+    // is random, determinism in the simulation is preserved, because nothing iterates over
+    // everything.
     #[serde(
-        serialize_with = "serialize_btreemap",
-        deserialize_with = "deserialize_btreemap"
+        serialize_with = "serialize_hashmap",
+        deserialize_with = "deserialize_hashmap"
     )]
-    queues: BTreeMap<Traversable, Queue>,
+    queues: HashMap<Traversable, Queue>,
     events: Vec<Event>,
 
     recalc_lanechanging: bool,
@@ -50,7 +53,7 @@ impl DrivingSimState {
     pub fn new(map: &Map, opts: &SimOptions) -> DrivingSimState {
         let mut sim = DrivingSimState {
             cars: FixedMap::new(),
-            queues: BTreeMap::new(),
+            queues: HashMap::new(),
             events: Vec::new(),
             recalc_lanechanging: opts.recalc_lanechanging,
             handle_uber_turns: opts.handle_uber_turns,
@@ -217,7 +220,7 @@ impl DrivingSimState {
 
         if !need_distances {
             // We need to mutate two different cars in one case. To avoid fighting the borrow
-            // checker, temporarily move one of them out of the BTreeMap.
+            // checker, temporarily move one of them out of the map.
             let mut car = self.cars.remove(&id).unwrap();
             // Responsibility of update_car to manage scheduling stuff!
             need_distances = self.update_car_without_distances(&mut car, now, ctx, transit);
@@ -234,7 +237,7 @@ impl DrivingSimState {
             let idx = dists.iter().position(|(c, _)| *c == id).unwrap();
 
             // We need to mutate two different cars in some cases. To avoid fighting the borrow
-            // checker, temporarily move one of them out of the BTreeMap.
+            // checker, temporarily move one of them out of the map.
             let mut car = self.cars.remove(&id).unwrap();
             // Responsibility of update_car_with_distances to manage scheduling stuff!
             if self
@@ -835,6 +838,7 @@ impl DrivingSimState {
         }
     }
 
+    /// Note the ordering of results is non-deterministic!
     pub fn get_unzoomed_agents(&self, now: Time, map: &Map) -> Vec<UnzoomedAgent> {
         let mut result = Vec::new();
 
@@ -867,6 +871,7 @@ impl DrivingSimState {
         self.cars.contains_key(&id)
     }
 
+    /// Note the ordering of results is non-deterministic!
     pub fn get_all_draw_cars(
         &self,
         now: Time,

sim/src/mechanics/intersection.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, BTreeSet, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 
 use serde::{Deserialize, Serialize};
 
@@ -289,7 +289,7 @@ impl IntersectionSimState {
         maybe_cars_and_queues: Option<(
             &Car,
             &FixedMap<CarID, Car>,
-            &mut BTreeMap<Traversable, Queue>,
+            &mut HashMap<Traversable, Queue>,
         )>,
     ) -> bool {
         let req = Request { agent, turn };
@@ -660,7 +660,7 @@ impl IntersectionSimState {
         &mut self,
         req: &Request,
         map: &Map,
-        maybe_cars_and_queues: Option<(&FixedMap<CarID, Car>, &BTreeMap<Traversable, Queue>)>,
+        maybe_cars_and_queues: Option<(&FixedMap<CarID, Car>, &HashMap<Traversable, Queue>)>,
     ) -> bool {
         let turn = map.get_t(req.turn);
         let mut cycle_detected = false;
@@ -713,7 +713,7 @@ impl IntersectionSimState {
     fn detect_conflict_cycle(
         &self,
         car: CarID,
-        pair: (&FixedMap<CarID, Car>, &BTreeMap<Traversable, Queue>),
+        pair: (&FixedMap<CarID, Car>, &HashMap<Traversable, Queue>),
     ) -> Option<HashSet<CarID>> {
         let (cars, queues) = pair;
 

sim/src/mechanics/queue.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, BTreeSet, VecDeque};
+use std::collections::{BTreeSet, HashMap, VecDeque};
 
 use serde::{Deserialize, Serialize};
 
@@ -43,7 +43,7 @@ impl Queue {
         &self,
         now: Time,
         cars: &FixedMap<CarID, Car>,
-        queues: &BTreeMap<Traversable, Queue>,
+        queues: &HashMap<Traversable, Queue>,
     ) -> Option<(CarID, Distance)> {
         self.inner_get_last_car_position(now, cars, queues, &mut BTreeSet::new(), None)
     }
@@ -53,7 +53,7 @@ impl Queue {
         &self,
         now: Time,
         cars: &FixedMap<CarID, Car>,
-        queues: &BTreeMap<Traversable, Queue>,
+        queues: &HashMap<Traversable, Queue>,
     ) -> Vec<(CarID, Distance)> {
         let mut all_cars = vec![];
         self.inner_get_last_car_position(
@@ -70,7 +70,7 @@ impl Queue {
         &self,
         now: Time,
         cars: &FixedMap<CarID, Car>,
-        queues: &BTreeMap<Traversable, Queue>,
+        queues: &HashMap<Traversable, Queue>,
         recursed_queues: &mut BTreeSet<Traversable>,
         mut intermediate_results: Option<&mut Vec<(CarID, Distance)>>,
     ) -> Option<(CarID, Distance)> {
@@ -204,7 +204,7 @@ impl Queue {
         vehicle_len: Distance,
         now: Time,
         cars: &FixedMap<CarID, Car>,
-        queues: &BTreeMap<Traversable, Queue>,
+        queues: &HashMap<Traversable, Queue>,
     ) -> Option<usize> {
         if self.laggy_head.is_none() && self.cars.is_empty() {
             return Some(0);

sim/src/router.rs

@@ -1,7 +1,7 @@
 //! For vehicles only, not pedestrians. Follows a Path from map_model, but can opportunistically
 //! lane-change to avoid a slow lane, can can handle re-planning to look for available parking.
 
-use std::collections::BTreeMap;
+use std::collections::HashMap;
 
 use serde::{Deserialize, Serialize};
 
@@ -334,7 +334,7 @@ impl Router {
 
     pub fn opportunistically_lanechange(
         &mut self,
-        queues: &BTreeMap<Traversable, Queue>,
+        queues: &HashMap<Traversable, Queue>,
         map: &Map,
         handle_uber_turns: bool,
     ) {