Avoid cloning a car's path when creating it. #368

- 8 hours of downtown: 135s down to 126s
- prebaking: still hovering around 3m4s, plus or minus a few seconds

sim/src/mechanics/driving.rs

@@ -81,20 +81,20 @@ impl DrivingSimState {
         sim
     }
 
-    /// True if it worked
+    /// None if it worked, otherwise returns the CreateCar unmodified for possible retry.
     pub fn start_car_on_lane(
         &mut self,
         now: Time,
-        params: CreateCar,
+        mut params: CreateCar,
         map: &Map,
         intersections: &IntersectionSimState,
         parking: &ParkingSimState,
         scheduler: &mut Scheduler,
-    ) -> bool {
+    ) -> Option<CreateCar> {
         let first_lane = params.router.head().as_lane();
 
         if !intersections.nobody_headed_towards(first_lane, map.get_l(first_lane).src_i) {
-            return false;
+            return Some(params);
         }
         if let Some(idx) = self.queues[&Traversable::Lane(first_lane)].get_idx_to_insert_car(
             params.start_dist,
@@ -156,7 +156,9 @@ impl DrivingSimState {
                             car.router.get_end_dist(),
                             first_lane
                         );
-                        return false;
+                        params.router = car.router;
+                        params.vehicle = car.vehicle;
+                        return Some(params);
                     }
                 }
 
@@ -171,9 +173,9 @@ impl DrivingSimState {
                 queue.reserved_length += car.vehicle.length + FOLLOWING_DISTANCE;
             }
             self.cars.insert(car.vehicle.id, car);
-            return true;
+            return None;
         }
-        false
+        Some(params)
     }
     /// State transitions for this car:
     ///

sim/src/sim/mod.rs

@@ -430,70 +430,79 @@ impl Sim {
                     .start_trip(self.time, id, trip_spec, maybe_req, maybe_path, &mut ctx);
             }
             Command::SpawnCar(create_car, retry_if_no_room) => {
-                if self.driving.start_car_on_lane(
+                // create_car contains a Path, which is expensive to clone. We need different parts
+                // of create_car after attempting start_car_on_lane. Make copies just of those
+                // here. In no case do we ever clone the path.
+                let id = create_car.vehicle.id;
+                let maybe_route = create_car.maybe_route;
+                let trip_and_person = create_car.trip_and_person;
+                let maybe_parked_car = create_car.maybe_parked_car.clone();
+                let req = create_car.req.clone();
+
+                if let Some(create_car) = self.driving.start_car_on_lane(
                     self.time,
-                    create_car.clone(),
+                    create_car,
                     map,
                     &self.intersections,
                     &self.parking,
                     &mut self.scheduler,
                 ) {
-                    if let Some((trip, person)) = create_car.trip_and_person {
+                    // Starting the car failed for some reason.
-                        self.trips
+                    if retry_if_no_room {
-                            .agent_starting_trip_leg(AgentID::Car(create_car.vehicle.id), trip);
+                        // TODO Record this in the trip log
+                        self.scheduler.push(
+                            self.time + BLIND_RETRY_TO_SPAWN,
+                            Command::SpawnCar(create_car, retry_if_no_room),
+                        );
+                    } else {
+                        // Buses don't use Command::SpawnCar, so this must exist.
+                        let (trip, person) = create_car.trip_and_person.unwrap();
+                        // Have to redeclare for the borrow checker
+                        let mut ctx = Ctx {
+                            parking: &mut self.parking,
+                            intersections: &mut self.intersections,
+                            cap: &mut self.cap,
+                            scheduler: &mut self.scheduler,
+                            map,
+                        };
+                        self.trips.cancel_trip(
+                            self.time,
+                            trip,
+                            format!(
+                                "no room to spawn car for {} by {}, not retrying",
+                                trip, person
+                            ),
+                            Some(create_car.vehicle),
+                            &mut ctx,
+                        );
+                    }
+                } else {
+                    // Creating the car succeeded.
+                    if let Some((trip, person)) = trip_and_person {
+                        self.trips.agent_starting_trip_leg(AgentID::Car(id), trip);
                         events.push(Event::TripPhaseStarting(
                             trip,
                             person,
-                            Some(create_car.req.clone()),
+                            Some(req),
-                            if create_car.vehicle.id.1 == VehicleType::Car {
+                            if id.1 == VehicleType::Car {
                                 TripPhaseType::Driving
                             } else {
                                 TripPhaseType::Biking
                             },
                         ));
                     }
-                    if let Some(parked_car) = create_car.maybe_parked_car {
+                    if let Some(parked_car) = maybe_parked_car {
                         if let ParkingSpot::Offstreet(b, _) = parked_car.spot {
                             // Buses don't start in parking garages, so trip must exist
-                            events.push(Event::PersonLeavesBuilding(
+                            events.push(Event::PersonLeavesBuilding(trip_and_person.unwrap().1, b));
-                                create_car.trip_and_person.unwrap().1,
-                                b,
-                            ));
                         }
                         self.parking.remove_parked_car(parked_car);
                     }
-                    if let Some(route) = create_car.maybe_route {
+                    if let Some(route) = maybe_route {
-                        self.transit.bus_created(create_car.vehicle.id, route);
+                        self.transit.bus_created(id, route);
                     }
                     self.analytics
-                        .record_demand(create_car.router.get_path(), map);
+                        .record_demand(self.driving.get_path(id).unwrap(), map);
-                } else if retry_if_no_room {
-                    // TODO Record this in the trip log
-                    self.scheduler.push(
-                        self.time + BLIND_RETRY_TO_SPAWN,
-                        Command::SpawnCar(create_car, retry_if_no_room),
-                    );
-                } else {
-                    // Buses don't use Command::SpawnCar, so this must exist.
-                    let (trip, person) = create_car.trip_and_person.unwrap();
-                    // Have to redeclare for the borrow checker
-                    let mut ctx = Ctx {
-                        parking: &mut self.parking,
-                        intersections: &mut self.intersections,
-                        cap: &mut self.cap,
-                        scheduler: &mut self.scheduler,
-                        map,
-                    };
-                    self.trips.cancel_trip(
-                        self.time,
-                        trip,
-                        format!(
-                            "no room to spawn car for {} by {}, not retrying",
-                            trip, person
-                        ),
-                        Some(create_car.vehicle),
-                        &mut ctx,
-                    );
                 }
             }
             Command::SpawnPed(create_ped) => {