From 5e0214982f539220bd11f30b8ef6031529806fa6 Mon Sep 17 00:00:00 2001 From: Marcel Rieser Date: Wed, 11 Oct 2023 10:25:16 +0200 Subject: [PATCH] get rid of DataContainerProvider There were a Map-based and an Array-based implementation of DataContainerProvider. Since we have Indices in Ids and use an IdMap, the Map-based lookup is essentially also an array-based lookup. --- .../ArrayBasedDataContainerProvider.java | 82 ------------------- .../DataContainerProvider.java | 37 --------- .../MapBasedDataContainerProvider.java | 53 ------------ .../TravelTimeCalculator.java | 77 ++++------------- 4 files changed, 18 insertions(+), 231 deletions(-) delete mode 100644 matsim/src/main/java/org/matsim/core/trafficmonitoring/ArrayBasedDataContainerProvider.java delete mode 100644 matsim/src/main/java/org/matsim/core/trafficmonitoring/DataContainerProvider.java delete mode 100644 matsim/src/main/java/org/matsim/core/trafficmonitoring/MapBasedDataContainerProvider.java diff --git a/matsim/src/main/java/org/matsim/core/trafficmonitoring/ArrayBasedDataContainerProvider.java b/matsim/src/main/java/org/matsim/core/trafficmonitoring/ArrayBasedDataContainerProvider.java deleted file mode 100644 index 870e47c41c0..00000000000 --- a/matsim/src/main/java/org/matsim/core/trafficmonitoring/ArrayBasedDataContainerProvider.java +++ /dev/null @@ -1,82 +0,0 @@ -/* *********************************************************************** * - * project: org.matsim.* - * ArrayBasedDataContainerProvider.java - * * - * *********************************************************************** * - * * - * copyright : (C) 2013 by the members listed in the COPYING, * - * LICENSE and WARRANTY file. * - * email : info at matsim dot org * - * * - * *********************************************************************** * - * * - * This program is free software; you can redistribute it and/or modify * - * it under the terms of the GNU General Public License as published by * - * the Free Software Foundation; either version 2 of the License, or * - * (at your option) any later version. * - * See also COPYING, LICENSE and WARRANTY file * - * * - * *********************************************************************** */ - -package org.matsim.core.trafficmonitoring; - -import org.matsim.api.core.v01.Id; -import org.matsim.api.core.v01.network.Link; -import org.matsim.api.core.v01.network.Network; -import org.matsim.core.router.priorityqueue.HasIndex; - -import java.util.Map; - -/** - * Uses an array to store DataContainer object for the TravelTimeCalculator. - * It is tried to get a DataContainer's position in the array is taken from the - * the Link, which is possible if routing networks are used (there, link implement - * the HasIndex interface). - * - * If regular network are used, calls are forwarded to a MapBasedDataContainerProvider, - * which represents the lookup approach used so far. - * - * @author cdobler - */ -class ArrayBasedDataContainerProvider implements DataContainerProvider { - - private final TravelTimeData[] arrayLinkData; - private final DataContainerProvider delegate; - - public ArrayBasedDataContainerProvider(Map, TravelTimeData> linkData, TravelTimeDataFactory ttDataFactory, - Network network) { - this.arrayLinkData = new TravelTimeData[network.getLinks().size()]; - this.delegate = new MapBasedDataContainerProvider(linkData, ttDataFactory); - } - - /* - * This method is called from the EventHandler part of the TravelTimeCalculator. - * There, only link ids are available. We cannot optimize this. - */ - @Override - public TravelTimeData getTravelTimeData(final Id linkId, final boolean createIfMissing) { - return this.delegate.getTravelTimeData(linkId, createIfMissing); - } - - /* - * This method is called from the TravelTime part of the TravelTimeCalculator. - * There, links are available. we can optimize this by using an array instead of a map. - * - * Probably pre-initialize all DataContainers to avoid the null-check? - */ - @Override - public TravelTimeData getTravelTimeData(Link link, boolean createIfMissing) { - if (link instanceof HasIndex) { - int index = ((HasIndex) link).getArrayIndex(); - TravelTimeData data = this.arrayLinkData[index]; - if (data == null) { - data = this.delegate.getTravelTimeData(link, createIfMissing); - this.arrayLinkData[index] = data; - } - return data; - } else { - return this.delegate.getTravelTimeData(link, createIfMissing); - } - } - -} diff --git a/matsim/src/main/java/org/matsim/core/trafficmonitoring/DataContainerProvider.java b/matsim/src/main/java/org/matsim/core/trafficmonitoring/DataContainerProvider.java deleted file mode 100644 index f308d815669..00000000000 --- a/matsim/src/main/java/org/matsim/core/trafficmonitoring/DataContainerProvider.java +++ /dev/null @@ -1,37 +0,0 @@ -/* *********************************************************************** * - * project: org.matsim.* - * DataContainerProvider.java - * * - * *********************************************************************** * - * * - * copyright : (C) 2013 by the members listed in the COPYING, * - * LICENSE and WARRANTY file. * - * email : info at matsim dot org * - * * - * *********************************************************************** * - * * - * This program is free software; you can redistribute it and/or modify * - * it under the terms of the GNU General Public License as published by * - * the Free Software Foundation; either version 2 of the License, or * - * (at your option) any later version. * - * See also COPYING, LICENSE and WARRANTY file * - * * - * *********************************************************************** */ - -package org.matsim.core.trafficmonitoring; - -import org.matsim.api.core.v01.Id; -import org.matsim.api.core.v01.network.Link; - -interface DataContainerProvider { - - /* - * This method is called from the EventHandler part of the TravelTimeCalculator. - */ - /*package*/ TravelTimeData getTravelTimeData(final Id linkId, final boolean createIfMissing); - - /* - * This method is called from the TravelTime part of the TravelTimeCalculator. - */ - /*package*/ TravelTimeData getTravelTimeData(final Link link, final boolean createIfMissing); -} diff --git a/matsim/src/main/java/org/matsim/core/trafficmonitoring/MapBasedDataContainerProvider.java b/matsim/src/main/java/org/matsim/core/trafficmonitoring/MapBasedDataContainerProvider.java deleted file mode 100644 index 391c7cd60c4..00000000000 --- a/matsim/src/main/java/org/matsim/core/trafficmonitoring/MapBasedDataContainerProvider.java +++ /dev/null @@ -1,53 +0,0 @@ -/* *********************************************************************** * - * project: org.matsim.* - * MapBasedDataContainerProvider.java - * * - * *********************************************************************** * - * * - * copyright : (C) 2013 by the members listed in the COPYING, * - * LICENSE and WARRANTY file. * - * email : info at matsim dot org * - * * - * *********************************************************************** * - * * - * This program is free software; you can redistribute it and/or modify * - * it under the terms of the GNU General Public License as published by * - * the Free Software Foundation; either version 2 of the License, or * - * (at your option) any later version. * - * See also COPYING, LICENSE and WARRANTY file * - * * - * *********************************************************************** */ - -package org.matsim.core.trafficmonitoring; - -import org.matsim.api.core.v01.Id; -import org.matsim.api.core.v01.network.Link; - -import java.util.Map; - -class MapBasedDataContainerProvider implements DataContainerProvider { - - private final Map, TravelTimeData> linkData; - private final TravelTimeDataFactory ttDataFactory; - - public MapBasedDataContainerProvider(Map, TravelTimeData> linkData, TravelTimeDataFactory ttDataFactory) { - this.linkData = linkData; - this.ttDataFactory = ttDataFactory; - } - - @Override - public TravelTimeData getTravelTimeData(final Id linkId, final boolean createIfMissing) { - TravelTimeData data = this.linkData.get(linkId); - if ((null == data) && createIfMissing) { - data = this.ttDataFactory.createTravelTimeData(linkId) ; - this.linkData.put(linkId, data); - } - return data; - } - - @Override - public TravelTimeData getTravelTimeData(Link link, boolean createIfMissing) { - return this.getTravelTimeData(link.getId(), createIfMissing); - } - -} diff --git a/matsim/src/main/java/org/matsim/core/trafficmonitoring/TravelTimeCalculator.java b/matsim/src/main/java/org/matsim/core/trafficmonitoring/TravelTimeCalculator.java index 9c243c87116..68e60af776e 100644 --- a/matsim/src/main/java/org/matsim/core/trafficmonitoring/TravelTimeCalculator.java +++ b/matsim/src/main/java/org/matsim/core/trafficmonitoring/TravelTimeCalculator.java @@ -82,13 +82,10 @@ public final class TravelTimeCalculator implements LinkEnterEventHandler, LinkLe private final int numSlots; TimeSlotComputation aggregator; - private IdMap linkData; private Map, Id>, TravelTimeData> linkToLinkData; - private final DataContainerProvider dataContainerProvider; - private final Map, LinkEnterEvent> linkEnterEvents; private final Set> vehiclesToIgnore; @@ -105,7 +102,7 @@ public final class TravelTimeCalculator implements LinkEnterEventHandler, LinkLe @Inject private QSimConfigGroup qsimConfig ; TravelTimeGetter travelTimeGetter ; - @Deprecated // user builder instead. kai, feb'19 + @Deprecated // use builder instead. kai, feb'19 public static TravelTimeCalculator create(Network network, TravelTimeCalculatorConfigGroup group) { TravelTimeCalculator calculator = new TravelTimeCalculator(network, group); configure(calculator, group, network); @@ -237,22 +234,9 @@ private TravelTimeCalculator(final Network network, final double timeslice, fina this.aggregator = new TimeSlotComputation(this.numSlots, this.timeSlice); this.travelTimeGetter = new AveragingTravelTimeGetter( this.aggregator ) ; this.ttDataFactory = new TravelTimeDataArrayFactory(network, this.numSlots); - if (this.calculateLinkTravelTimes){ + if (this.calculateLinkTravelTimes) { this.linkData = new IdMap<>(Link.class); - - /* - * So far, link data objects were stored in a HashMap. This lookup strategy is used - * by a MapBasedDataContainerProvider. - * When ArrayRoutingNetworks are used (as the FastRouter implementations do), the - * getArrayIndex() methods from the RoutingLinks can be used to lookup the link - * data objects in an array. This approach is implemented by the ArrayBasedDataContainerProvider. - * Using a ArrayBasedDataContainerProvider instead of a MapBasedDataContainerProvider - * increases the routing performance by 20-30%. - * cdobler, oct'13 - */ - // this.dataContainerProvider = new MapBasedDataContainerProvider(linkData, ttDataFactory); - this.dataContainerProvider = new ArrayBasedDataContainerProvider(linkData, ttDataFactory, network); - } else this.dataContainerProvider = null; + } if (this.calculateLinkToLinkTravelTimes){ // assume that every link has 2 outgoing links as default this.linkToLinkData = new ConcurrentHashMap<>((int) (network.getLinks().size() * 1.4 * 2)); @@ -289,7 +273,7 @@ public void handleEvent(final LinkLeaveEvent e) { if (this.calculateLinkTravelTimes) { LinkEnterEvent oldEvent = this.linkEnterEvents.get(e.getVehicleId()); if (oldEvent != null) { - TravelTimeData data = this.dataContainerProvider.getTravelTimeData(e.getLinkId(), true); + TravelTimeData data = this.getTravelTimeData(e.getLinkId(), true); double enterTime = oldEvent.getTime(); final int timeSlot = this.aggregator.getTimeSlotIndex(enterTime ); @@ -332,7 +316,7 @@ public void handleEvent(VehicleArrivesAtFacilityEvent event) { public void handleEvent(VehicleAbortsEvent event) { LinkEnterEvent e = this.linkEnterEvents.remove(event.getVehicleId()); if (e != null) { - TravelTimeData data = this.dataContainerProvider.getTravelTimeData(e.getLinkId(), true); + TravelTimeData data = this.getTravelTimeData(e.getLinkId(), true); data.setNeedsConsolidation( true ); // this.aggregator.addStuckEventTravelTime(data, e.getTime(), event.getTime()); @@ -351,6 +335,15 @@ public void handleEvent(VehicleAbortsEvent event) { if (filterAnalyzedModes) this.vehiclesToIgnore.remove(event.getVehicleId()); } + private TravelTimeData getTravelTimeData(final Id linkId, final boolean createIfMissing) { + TravelTimeData data = this.linkData.get(linkId); + if ((null == data) && createIfMissing) { + data = this.ttDataFactory.createTravelTimeData(linkId); + this.linkData.put(linkId, data); + } + return data; + } + private TravelTimeData getLinkToLinkTravelTimeData( Tuple, Id> fromLinkToLink ) { TravelTimeData data = this.linkToLinkData.get(fromLinkToLink); if ( null == data ) { @@ -360,48 +353,14 @@ private TravelTimeData getLinkToLinkTravelTimeData( Tuple, Id> fr return data; } - /* - * Use the link as argument here! In case the DataContainer is array-based and the link is from a routing network, - * the DataContainer uses the link's index to access its data structures instead of performing a map lookup, which - * increases the router performance by 20-30%! - * cdobler, aug'17 - */ - private double getLinkTravelTime(final Link link, final double time) { + private double getLinkTravelTime(final Id linkId, final double time) { if (this.calculateLinkTravelTimes) { - TravelTimeData data = this.dataContainerProvider.getTravelTimeData(link, true); + TravelTimeData data = this.getTravelTimeData(linkId, true); if ( data.isNeedingConsolidation() ) { consolidateData(data); } return this.travelTimeGetter.getTravelTime( data, time ); - - /* - * Workaround for jumps in returned travel times due to time bin approach? - * Should not be necessary when using linear interpolated travel times. - */ - // DataContainer data = this.dataContainerProvider.getTravelTimeInfo(link, true); - // if (data.needsConsolidation) { - // consolidateData(data); - // } - // double travelTime = this.aggregator.getTravelTime(data, time); - // - // // in case there is no previous time bin - // if (time <= this.timeSlice) return travelTime; - // - // int index = this.aggregator.getTimeSlotIndex(time); - // double previousBinEndTime = index * this.timeSlice; - // - // // calculate travel time when starting at the last second of the previous time slot - // double previousTravelTime = this.aggregator.getTravelTime(data, time - this.timeSlice); - // - // double prev = previousBinEndTime + previousTravelTime; - // double now = time + travelTime; - // if (now >= prev) { - // return travelTime; - // } - // else { - // return prev - time; // ensure travel time not shorter than travel time from the previous bin - // } } throw new IllegalStateException("No link travel time is available " + "if calculation is switched off by config option!"); @@ -511,7 +470,7 @@ public double getLinkTravelTime(Link link, double time, Person person, Vehicle v linkTtimeFromVehicle = link.getLength() / vehicleType.getMaximumVelocity(); } } - double linkTTimeFromObservation = TravelTimeCalculator.this.getLinkTravelTime(link, time); + double linkTTimeFromObservation = TravelTimeCalculator.this.getLinkTravelTime(link.getId(), time); return Math.max( linkTtimeFromVehicle, linkTTimeFromObservation) ; // yyyyyy should this not be min? kai/janek, may'19 // No, it is correct. It is preventing the router to route with an empirical speed from @@ -549,7 +508,7 @@ public double getLinkToLinkTravelTime(Link fromLink, Link toLink, double time, P } @Deprecated // use builder.configure(config) instead. kai, feb'19 - public void setTtDataFactory( TravelTimeDataFactory ttDataFactory ){ + void setTtDataFactory( TravelTimeDataFactory ttDataFactory ){ // yyyyyy this is currently here for a test, but should be removed. kai, feb'19 this.ttDataFactory = ttDataFactory; }