From ea27a5d6661c0ac71ddd306ec259983cee623f00 Mon Sep 17 00:00:00 2001 From: Michael Behrisch Date: Fri, 10 Nov 2023 22:23:49 +0100 Subject: [PATCH] fixing memory leak and refactoring #14034 #12 --- src/router/ROPerson.cpp | 57 +++++++++++++++++++---------------------- src/router/ROPerson.h | 26 ++++--------------- 2 files changed, 32 insertions(+), 51 deletions(-) diff --git a/src/router/ROPerson.cpp b/src/router/ROPerson.cpp index 821fc899d16e..8a76dfabbe66 100644 --- a/src/router/ROPerson.cpp +++ b/src/router/ROPerson.cpp @@ -325,7 +325,8 @@ ROPerson::PersonTrip::getDuration() const { bool ROPerson::computeIntermodal(SUMOTime time, const RORouterProvider& provider, - PersonTrip* const trip, const ROVehicle* const veh, MsgHandler* const errorHandler) { + const PersonTrip* const trip, const ROVehicle* const veh, + std::vector& resultItems, MsgHandler* const errorHandler) { const double speed = getMaxSpeed() * trip->getWalkFactor(); std::vector result; provider.getIntermodalRouter().compute(trip->getOrigin(), trip->getDestination(), @@ -358,9 +359,9 @@ ROPerson::computeIntermodal(SUMOTime time, const RORouterProvider& provider, } } if (&item == &result.back() && trip->getStopDest() == "") { - trip->addTripItem(new Walk(start, item.edges, item.cost, item.exitTimes, depPos, arrPos)); + resultItems.push_back(new Walk(start, item.edges, item.cost, item.exitTimes, depPos, arrPos)); } else { - trip->addTripItem(new Walk(start, item.edges, item.cost, item.exitTimes, depPos, arrPos, item.destStop)); + resultItems.push_back(new Walk(start, item.edges, item.cost, item.exitTimes, depPos, arrPos, item.destStop)); } } else if (veh != nullptr && item.line == veh->getID()) { double cost = item.cost; @@ -369,16 +370,16 @@ ROPerson::computeIntermodal(SUMOTime time, const RORouterProvider& provider, route->setProbability(1); veh->getRouteDefinition()->addLoadedAlternative(route); carUsed = true; - } else if (trip->needsRouting()) { + } else if (resultItems.empty()) { // if this is the first plan item the initial taxi waiting time wasn't added yet const double taxiWait = STEPS2TIME(string2time(OptionsCont::getOptions().getString("persontrip.taxi.waiting-time"))); cost += taxiWait; } - trip->addTripItem(new Ride(start, item.edges.front(), item.edges.back(), veh->getID(), trip->getGroup(), cost, item.arrivalPos, item.length, item.destStop)); + resultItems.push_back(new Ride(start, item.edges.front(), item.edges.back(), veh->getID(), trip->getGroup(), cost, item.arrivalPos, item.length, item.destStop)); } else { // write origin for first element of the plan - const ROEdge* origin = trip == myPlan.front() && trip->needsRouting() ? trip->getOrigin() : nullptr; - trip->addTripItem(new Ride(start, origin, nullptr, item.line, trip->getGroup(), item.cost, item.arrivalPos, item.length, item.destStop, item.intended, TIME2STEPS(item.depart))); + const ROEdge* origin = trip == myPlan.front() && resultItems.empty() ? trip->getOrigin() : nullptr; + resultItems.push_back(new Ride(start, origin, nullptr, item.line, trip->getGroup(), item.cost, item.arrivalPos, item.length, item.destStop, item.intended, TIME2STEPS(item.depart))); } } start += TIME2STEPS(item.cost); @@ -399,36 +400,32 @@ ROPerson::computeRoute(const RORouterProvider& provider, for (PlanItem* const it : myPlan) { if (it->needsRouting()) { PersonTrip* trip = static_cast(it); - std::vector& vehicles = trip->getVehicles(); + const std::vector& vehicles = trip->getVehicles(); + std::vector resultItems; + std::vector best; + const ROVehicle* bestVeh = nullptr; if (vehicles.empty()) { - computeIntermodal(time, provider, trip, nullptr, errorHandler); + computeIntermodal(time, provider, trip, nullptr, best, errorHandler); } else { double bestCost = std::numeric_limits::infinity(); - PersonTrip* best = nullptr; - ROVehicle* bestVeh = nullptr; - for (std::vector::iterator v = vehicles.begin(); v != vehicles.end();) { - if (!computeIntermodal(time, provider, trip, *v, errorHandler)) { - delete (*v)->getRouteDefinition(); - delete *v; - v = vehicles.erase(v); - } else { - const double cost = trip->getCost(); - if (cost < bestCost) { - bestCost = cost; - if (best != nullptr) { - delete best; - } - best = static_cast(trip->clone()); - bestVeh = *v; + for (const ROVehicle* const v : vehicles) { + const bool carUsed = computeIntermodal(time, provider, trip, v, resultItems, errorHandler); + double cost = 0.; + for (TripItem* const it : resultItems) { + cost += it->getCost(); + } + if (cost < bestCost) { + bestCost = cost; + while (!best.empty()) { + delete best.back(); + best.pop_back(); } - trip->clearItems(); - ++v; + best.swap(resultItems); + bestVeh = carUsed ? v : nullptr; } } - if (best != nullptr) { - trip->copyItems(best, bestVeh); - } } + trip->setItems(best, bestVeh); } time += it->getDuration(); } diff --git a/src/router/ROPerson.h b/src/router/ROPerson.h index 1a609be75f6a..0440fb06f879 100644 --- a/src/router/ROPerson.h +++ b/src/router/ROPerson.h @@ -365,26 +365,9 @@ class ROPerson : public RORoutable { return myTripItems.empty(); } - double getCost() const { - double result = 0; - for (TripItem* const it : myTripItems) { - result += it->getCost(); - } - return result; - } - - void clearItems() { - for (TripItem* const it : myTripItems) { - delete it; - } - myTripItems.clear(); - } - - void copyItems(PersonTrip* trip, ROVehicle* veh) { - for (TripItem* const it : myTripItems) { - delete it; - } - myTripItems = trip->myTripItems; + void setItems(std::vector& newItems, const ROVehicle* const veh) { + assert(myTripItems.empty()); + myTripItems.swap(newItems); for (auto it = myVehicles.begin(); it != myVehicles.end();) { if (*it != veh) { delete (*it)->getRouteDefinition(); @@ -460,7 +443,8 @@ class ROPerson : public RORoutable { private: bool computeIntermodal(SUMOTime time, const RORouterProvider& provider, - PersonTrip* const trip, const ROVehicle* const veh, MsgHandler* const errorHandler); + const PersonTrip* const trip, const ROVehicle* const veh, + std::vector& resultItems, MsgHandler* const errorHandler); private: /**