Performance improvements #2242
Performance improvements #2242
Conversation
… append is faster for Vector
…is an issue with Actor Logging that it has to compute arguments)
…meone just wanted to log `beamVehicleType.id`
REASY
requested review from
colinsheppard,
JustinPihony,
haitamlaarabi and
wrashid
| @@ -150,15 +150,15 @@ class VehicleCentricMatchingForRideHail( | |||
| } | |||
|
|
|||
| private def greedyAssignment(trips: List[AssignmentKey]): List[AssignmentKey] = { | |||
| val Rok = mutable.ListBuffer.empty[CustomerRequest] | |||
| val Vok = mutable.ListBuffer.empty[VehicleAndSchedule] | |||
| val Rok = mutable.HashSet.empty[CustomerRequest] | |||
There was a problem hiding this comment.
@haitamlaarabi please, take a look to these changes. It should not break anything, but still.
| @@ -177,8 +177,8 @@ object AlonsoMoraPoolingAlgForRideHail { | |||
| solutionSpaceSizePerVehicle: Int | |||
| ): List[(RideHailTrip, VehicleAndSchedule, Double)] = { | |||
| import scala.collection.mutable.{ListBuffer => MListBuffer} | |||
| val Rok = MListBuffer.empty[CustomerRequest] | |||
| val Vok = MListBuffer.empty[VehicleAndSchedule] | |||
| val Rok = collection.mutable.HashSet.empty[CustomerRequest] | |||
There was a problem hiding this comment.
@haitamlaarabi please, take a look to these changes. It should not break anything, but still.
src/main/scala/beam/agentsim/agents/ridehail/RideHailVehicleManager.scala
Show resolved
Hide resolved
| VehicleAllocations(allocResponses) | ||
| } | ||
|
|
||
| val repositioningManager: RepositioningManager = createRepositioningManager() | ||
| logger.info(s"Using ${repositioningManager.getClass.getSimpleName} as RepositioningManager") | ||
|
|
||
| def findDepotsForVehiclesInNeedOfRefueling(cavOnly: Boolean = true): Vector[(Id[Vehicle], ParkingStall)] = { | ||
| val idleVehicleIdsAndLocation: Vector[(Id[Vehicle], RideHailAgentLocation)] = |
There was a problem hiding this comment.
This seems dangerous to push back to the caller. If the filtering of excluded is required then it should be handled inside of the implementation itself.
There was a problem hiding this comment.
@JustinPihony not sure why this is dangerous? I've changed the contract - now you have to provide idleVehicles. Maybe I should rename it just to vehicles?
There was a problem hiding this comment.
@colinsheppard what do you think?
There was a problem hiding this comment.
Yah....if the contract is fine to change then I'm ok with that - but it is definitely a contract change - if we are ok with the potential for non-idle and exclusionary coming in in other means then that's fine. But if it should always be idle excluded then it is not ok.
| @@ -81,8 +81,11 @@ class DemandFollowingRepositioningManager(val beamServices: BeamServices, val ri | |||
| createClusters | |||
| } | |||
|
|
|||
| def repositionVehicles(tick: Int): Vector[(Id[Vehicle], Location)] = { | |||
| val nonRepositioningIdleVehicles = rideHailManager.vehicleManager.getIdleVehiclesAndFilterOutExluded.values | |||
There was a problem hiding this comment.
Same as above - we shouldn't push this to the calling method - unless it is NOT an implementation detail.
There was a problem hiding this comment.
We're repositioning only idle vehicles so it seems ok for me to pass the vehicles to this method.
|
test! |
Main changes in PR:
Run params:
production/sfbay/smart/smart-b-hightech-2040.conf, for 5 iterationsResults
Profiling tool results
Baseline overall
Baseline 1-th iteration
Optimized overall
Optimized 1-th iteration
This change is