Skip to content

Commit 800aeab

Browse files
leonardehrenfriedleonardehrenfried
authored
Merge pull request #6752 from leonardehrenfried/efficient-shapes
Lower graph build memory by processing GTFS shapes more efficiently
2 parents a38d957 + fd05541 commit 800aeab

File tree

19 files changed

+366
-286
lines changed

19 files changed

+366
-286
lines changed

‎application/src/main/java/org/opentripplanner/graph_builder/module/geometry/GeometryProcessor.java‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import java.util.ArrayList;
44
import java.util.Arrays;
5-
import java.util.Collections;
5+
import java.util.Collection;
66
import java.util.Iterator;
77
import java.util.LinkedList;
88
import java.util.List;
@@ -45,7 +45,7 @@ public class GeometryProcessor {
4545

4646
private static final Logger LOG = LoggerFactory.getLogger(GeometryProcessor.class);
4747
private static final GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory();
48-
private final OtpTransitServiceBuilder transitService;
48+
private final OtpTransitServiceBuilder builder;
4949
// this is a thread-safe implementation
5050
private final Map<ShapeSegmentKey, LineString> geometriesByShapeSegmentKey =
5151
new ConcurrentHashMap<>();
@@ -57,13 +57,11 @@ public class GeometryProcessor {
5757
private final DataImportIssueStore issueStore;
5858

5959
public GeometryProcessor(
60-
// TODO OTP2 - Operate on the builder, not the transit service and move the execution of
61-
// - this to where the builder is in context.
62-
OtpTransitServiceBuilder transitService,
60+
OtpTransitServiceBuilder builder,
6361
double maxStopToShapeSnapDistance,
6462
DataImportIssueStore issueStore
6563
) {
66-
this.transitService = transitService;
64+
this.builder = builder;
6765
this.maxStopToShapeSnapDistance = maxStopToShapeSnapDistance > 0
6866
? maxStopToShapeSnapDistance
6967
: 150;
@@ -88,7 +86,7 @@ public List<LineString> createHopGeometries(Trip trip) {
8886
}
8987

9088
return Arrays.asList(
91-
createGeometry(trip.getShapeId(), transitService.getStopTimesSortedByTrip().get(trip))
89+
createGeometry(trip.getShapeId(), builder.getStopTimesSortedByTrip().get(trip))
9290
);
9391
}
9492

@@ -465,27 +463,32 @@ private LineString getSegmentGeometry(
465463
* they are unnecessary, and 2) they define 0-length line segments which cause JTS location
466464
* indexed line to return a segment location of NaN, which we do not want.
467465
*/
468-
private List<ShapePoint> getUniqueShapePointsForShapeId(FeedScopedId shapeId) {
469-
List<ShapePoint> points = new ArrayList<>(transitService.getShapePoints().get(shapeId));
470-
Collections.sort(points);
471-
ArrayList<ShapePoint> filtered = new ArrayList<>(points.size());
466+
private Collection<ShapePoint> getUniqueShapePointsForShapeId(FeedScopedId shapeId) {
467+
var points = builder.getShapePoints().get(shapeId);
468+
ArrayList<ShapePoint> filtered = new ArrayList<>();
472469
ShapePoint last = null;
470+
int currentSeq = Integer.MIN_VALUE;
473471
for (ShapePoint sp : points) {
474-
if (last == null || last.getSequence() != sp.getSequence()) {
475-
if (last != null && last.getLat() == sp.getLat() && last.getLon() == sp.getLon()) {
472+
if (sp.sequence() < currentSeq) {
473+
// this should never happen, because the GTFS import should make sure they are already in order.
474+
// therefore this just a safety check to detect a programmer error.
475+
throw new IllegalStateException(
476+
"Shape %s is not sorted in order of sequence. This indicates a bug in OTP.".formatted(
477+
shapeId
478+
)
479+
);
480+
}
481+
if (last == null || last.sequence() != sp.sequence()) {
482+
if (last != null && last.sameCoordinates(sp)) {
476483
LOG.trace("pair of identical shape points (skipping): {} {}", last, sp);
477484
} else {
478485
filtered.add(sp);
479486
}
480487
}
481488
last = sp;
489+
currentSeq = sp.sequence();
482490
}
483-
if (filtered.size() != points.size()) {
484-
filtered.trimToSize();
485-
return filtered;
486-
} else {
487-
return points;
488-
}
491+
return filtered;
489492
}
490493

491494
private LineString getLineStringForShapeId(FeedScopedId shapeId) {
@@ -495,7 +498,7 @@ private LineString getLineStringForShapeId(FeedScopedId shapeId) {
495498
return geometry;
496499
}
497500

498-
List<ShapePoint> points = getUniqueShapePointsForShapeId(shapeId);
501+
var points = getUniqueShapePointsForShapeId(shapeId);
499502
if (points.size() < 2) {
500503
return null;
501504
}
@@ -506,8 +509,8 @@ private LineString getLineStringForShapeId(FeedScopedId shapeId) {
506509

507510
int i = 0;
508511
for (ShapePoint point : points) {
509-
coordinates[i] = new Coordinate(point.getLon(), point.getLat());
510-
distances[i] = point.getDistTraveled();
512+
coordinates[i] = point.coordinate();
513+
distances[i] = point.distTraveled();
511514
if (!point.isDistTraveledSet()) hasAllDistances = false;
512515
i++;
513516
}

‎application/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsModule.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public void buildGraph() {
135135
for (GtfsBundle gtfsBundle : gtfsBundles) {
136136
GtfsMutableRelationalDao gtfsDao = loadBundle(gtfsBundle);
137137

138-
final String feedId = gtfsBundle.getFeedId();
138+
var feedId = gtfsBundle.getFeedId();
139139
verifyUniqueFeedId(gtfsBundle, feedIdsEncountered, feedId);
140140

141141
feedIdsEncountered.put(feedId, gtfsBundle);
@@ -145,10 +145,9 @@ public void buildGraph() {
145145
feedId,
146146
issueStore,
147147
gtfsBundle.parameters().discardMinTransferTimes(),
148-
gtfsDao,
149148
gtfsBundle.parameters().stationTransferPreference()
150149
);
151-
mapper.mapStopTripAndRouteDataIntoBuilder();
150+
mapper.mapStopTripAndRouteDataIntoBuilder(gtfsDao);
152151

153152
OtpTransitServiceBuilder builder = mapper.getBuilder();
154153
var fareRulesData = mapper.fareRulesData();
@@ -168,7 +167,7 @@ public void buildGraph() {
168167
);
169168

170169
// We need to run this after the cleaning of the data, as stop indices might have changed
171-
mapper.mapAndAddTransfersToBuilder();
170+
mapper.mapAndAddTransfersToBuilder(gtfsDao);
172171

173172
GeometryProcessor geometryProcessor = new GeometryProcessor(
174173
builder,
@@ -310,7 +309,9 @@ private void addTimetableRepositoryToGraph(
310309
}
311310

312311
private GtfsMutableRelationalDao loadBundle(GtfsBundle gtfsBundle) throws IOException {
313-
StoreImpl store = new StoreImpl(new GtfsRelationalDaoImpl());
312+
var dao = new GtfsRelationalDaoImpl();
313+
dao.setPackShapePoints(true);
314+
StoreImpl store = new StoreImpl(dao);
314315
store.open();
315316
LOG.info("reading {}", gtfsBundle.feedInfo());
316317

‎application/src/main/java/org/opentripplanner/gtfs/mapping/CompactShape.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package org.opentripplanner.gtfs.mapping;
2+
3+
import gnu.trove.list.TDoubleList;
4+
import gnu.trove.list.TIntList;
5+
import gnu.trove.list.array.TDoubleArrayList;
6+
import gnu.trove.list.array.TIntArrayList;
7+
import java.util.Collections;
8+
import java.util.Iterator;
9+
import java.util.List;
10+
import java.util.stream.Collectors;
11+
import java.util.stream.IntStream;
12+
import org.opentripplanner.model.ShapePoint;
13+
14+
/**
15+
* A representation that stores GTFS shape points in a memory-efficient way and allows you to
16+
* iterate over them for further processing.
17+
* <p>
18+
* The fields of ShapePoints are stored densely in automatically expanding Trove primitive lists.
19+
* When later needed, they are reconstituted into objects and sorted on their sequence number.
20+
* Only an iterator over the sorted collection escapes rather than the collection itself, providing
21+
* some assurance that the objects will be quickly garbage collected.
22+
* <p>
23+
* This class is package-private but implements Iterable, so you should use that as the return type
24+
* of the mapping process.
25+
*/
26+
class CompactShape implements Iterable<ShapePoint> {
27+
28+
private static final int INITIAL_CAPACITY = 50;
29+
private static final double NO_DISTANCE = -9999;
30+
31+
private final TIntList seqs = new TIntArrayList(INITIAL_CAPACITY);
32+
private final TDoubleList lats = new TDoubleArrayList(INITIAL_CAPACITY);
33+
private final TDoubleList lons = new TDoubleArrayList(INITIAL_CAPACITY);
34+
private final TDoubleList dists = new TDoubleArrayList(INITIAL_CAPACITY);
35+
36+
public void addPoint(org.onebusaway.gtfs.model.ShapePoint shapePoint) {
37+
seqs.add(shapePoint.getSequence());
38+
lats.add(shapePoint.getLat());
39+
lons.add(shapePoint.getLon());
40+
dists.add(shapePoint.isDistTraveledSet() ? shapePoint.getDistTraveled() : NO_DISTANCE);
41+
}
42+
43+
@Override
44+
public Iterator<ShapePoint> iterator() {
45+
return IntStream.range(0, lats.size())
46+
.mapToObj(i -> {
47+
double dist = dists.get(i);
48+
return new ShapePoint(seqs.get(i), lats.get(i), lons.get(i), dist < 0 ? null : dist);
49+
})
50+
.sorted()
51+
.iterator();
52+
}
53+
}

‎application/src/main/java/org/opentripplanner/gtfs/mapping/GTFSToOtpTransitServiceMapper.java‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@
99
import java.util.Collection;
1010
import java.util.function.Function;
1111
import org.onebusaway.gtfs.model.Stop;
12+
import org.onebusaway.gtfs.services.GtfsDao;
1213
import org.onebusaway.gtfs.services.GtfsRelationalDao;
1314
import org.opentripplanner.ext.fares.model.FareRulesData;
1415
import org.opentripplanner.framework.application.OTPFeature;
1516
import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
16-
import org.opentripplanner.model.ShapePoint;
1717
import org.opentripplanner.model.impl.OtpTransitServiceBuilder;
1818
import org.opentripplanner.transit.model.framework.FeedScopedId;
1919
import org.opentripplanner.transit.model.site.RegularStop;
@@ -77,8 +77,6 @@ public class GTFSToOtpTransitServiceMapper {
7777

7878
private final DataImportIssueStore issueStore;
7979

80-
private final GtfsRelationalDao data;
81-
8280
private final OtpTransitServiceBuilder builder;
8381

8482
private final FareRulesData fareRulesBuilder = new FareRulesData();
@@ -93,7 +91,6 @@ public GTFSToOtpTransitServiceMapper(
9391
String feedId,
9492
DataImportIssueStore issueStore,
9593
boolean discardMinTransferTimes,
96-
GtfsRelationalDao data,
9794
StopTransferPriority stationTransferPreference
9895
) {
9996
var idFactory = new IdFactory(feedId);
@@ -103,7 +100,6 @@ public GTFSToOtpTransitServiceMapper(
103100
Function<FeedScopedId, Station> stationLookup = id -> builder.getStations().get(id);
104101
Function<FeedScopedId, RegularStop> stopLookup = id -> builder.getStops().get(id);
105102

106-
this.data = data;
107103
this.discardMinTransferTimes = discardMinTransferTimes;
108104
serviceCalendarMapper = new ServiceCalendarMapper(idFactory);
109105
serviceCalendarDateMapper = new ServiceCalendarDateMapper(idFactory);
@@ -164,7 +160,7 @@ public FareRulesData fareRulesData() {
164160
return fareRulesBuilder;
165161
}
166162

167-
public void mapStopTripAndRouteDataIntoBuilder() {
163+
public void mapStopTripAndRouteDataIntoBuilder(GtfsRelationalDao data) {
168164
translationHelper.importTranslations(data.getAllTranslations(), data.getAllFeedInfos());
169165

170166
builder.getAgenciesById().addAll(agencyMapper.map(data.getAllAgencies()));
@@ -173,9 +169,10 @@ public void mapStopTripAndRouteDataIntoBuilder() {
173169
builder.getFeedInfos().addAll(feedInfoMapper.map(data.getAllFeedInfos()));
174170
builder.getFrequencies().addAll(frequencyMapper.map(data.getAllFrequencies()));
175171
builder.getRoutes().addAll(routeMapper.map(data.getAllRoutes()));
176-
for (ShapePoint shapePoint : shapePointMapper.map(data.getAllShapePoints())) {
177-
builder.getShapePoints().put(shapePoint.getShapeId(), shapePoint);
178-
}
172+
var shapes = shapePointMapper.map(data.getAllShapePoints());
173+
builder.getShapePoints().putAll(shapes);
174+
// shape points is a large collection, so after mapping it can be cleared
175+
data.getAllShapePoints().clear();
179176

180177
mapGtfsStopsToOtpTypes(data.getAllStops());
181178

@@ -233,7 +230,7 @@ private void mapGtfsStopsToOtpTypes(Collection<Stop> stops) {
233230
/**
234231
* Note! Trip-pattens must be added BEFORE mapping transfers
235232
*/
236-
public void mapAndAddTransfersToBuilder() {
233+
public void mapAndAddTransfersToBuilder(GtfsDao data) {
237234
TransferMapper transferMapper = new TransferMapper(
238235
routeMapper,
239236
stationMapper,

‎application/src/main/java/org/opentripplanner/gtfs/mapping/ShapePointMapper.java‎

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,44 +3,27 @@
33
import java.util.Collection;
44
import java.util.HashMap;
55
import java.util.Map;
6-
import org.opentripplanner.model.ShapePoint;
7-
import org.opentripplanner.utils.collection.MapUtils;
6+
import org.checkerframework.checker.units.qual.C;
7+
import org.opentripplanner.transit.model.framework.FeedScopedId;
88

99
/** Responsible for mapping GTFS ShapePoint into the OTP model. */
1010
class ShapePointMapper {
1111

1212
private final IdFactory idFactory;
13-
private final Map<org.onebusaway.gtfs.model.ShapePoint, ShapePoint> mappedShapePoints =
14-
new HashMap<>();
1513

1614
ShapePointMapper(IdFactory idFactory) {
1715
this.idFactory = idFactory;
1816
}
1917

20-
Collection<ShapePoint> map(Collection<org.onebusaway.gtfs.model.ShapePoint> allShapePoints) {
21-
return MapUtils.mapToList(allShapePoints, this::map);
22-
}
23-
24-
/** Map from GTFS to OTP model, {@code null} safe. */
25-
ShapePoint map(org.onebusaway.gtfs.model.ShapePoint orginal) {
26-
return orginal == null ? null : mappedShapePoints.computeIfAbsent(orginal, this::doMap);
27-
}
28-
29-
private ShapePoint doMap(org.onebusaway.gtfs.model.ShapePoint rhs) {
30-
ShapePoint lhs = new ShapePoint();
31-
32-
lhs.setShapeId(idFactory.createId(rhs.getShapeId(), "shape point"));
33-
lhs.setSequence(rhs.getSequence());
34-
lhs.setLat(rhs.getLat());
35-
lhs.setLon(rhs.getLon());
36-
lhs.setDistTraveled(rhs.getDistTraveled());
37-
38-
// Skip mapping of proxy
39-
// private transient StopTimeProxy proxy;
40-
if (rhs.getProxy() != null) {
41-
throw new IllegalStateException("Did not expect proxy to be set! Data: " + rhs);
18+
Map<FeedScopedId, CompactShape> map(
19+
Collection<org.onebusaway.gtfs.model.ShapePoint> allShapePoints
20+
) {
21+
var ret = new HashMap<FeedScopedId, CompactShape>();
22+
for (var shapePoint : allShapePoints) {
23+
var shapeId = idFactory.createId(shapePoint.getShapeId(), "shape point");
24+
var shape = ret.computeIfAbsent(shapeId, id -> new CompactShape());
25+
shape.addPoint(shapePoint);
4226
}
43-
44-
return lhs;
27+
return ret;
4528
}
4629
}

‎application/src/main/java/org/opentripplanner/gtfs/mapping/StopTimeMapper.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ private StopTime doMap(org.onebusaway.gtfs.model.StopTime rhs) {
8888
lhs.setTimepoint(rhs.getTimepoint());
8989
lhs.setStopSequence(rhs.getStopSequence());
9090
lhs.setStopHeadsign(stopHeadsign);
91-
lhs.setRouteShortName(rhs.getRouteShortName());
9291
lhs.setPickupType(PickDropMapper.map(rhs.getPickupType()));
9392
lhs.setDropOffType(PickDropMapper.map(rhs.getDropOffType()));
9493
lhs.setShapeDistTraveled(rhs.getShapeDistTraveled());

‎application/src/main/java/org/opentripplanner/model/OtpTransitService.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import com.google.common.collect.Multimap;
44
import java.util.Collection;
5-
import java.util.List;
65
import java.util.Map;
76
import org.opentripplanner.ext.flex.trip.FlexTrip;
87
import org.opentripplanner.model.transfer.ConstrainedTransfer;
@@ -52,20 +51,12 @@ public interface OtpTransitService {
5251
*/
5352
Collection<FeedScopedId> getAllServiceIds();
5453

55-
List<ShapePoint> getShapePointsForShapeId(FeedScopedId shapeId);
56-
5754
Collection<Entrance> getAllEntrances();
5855

5956
Collection<PathwayNode> getAllPathwayNodes();
6057

6158
Collection<BoardingArea> getAllBoardingAreas();
6259

63-
/**
64-
* @return the list of {@link StopTime} objects associated with the trip, sorted by {@link
65-
* StopTime#getStopSequence()}
66-
*/
67-
List<StopTime> getStopTimesForTrip(Trip trip);
68-
6960
Collection<ConstrainedTransfer> getAllTransfers();
7061

7162
Collection<TripPattern> getTripPatterns();

0 commit comments

Comments
 (0)