Lower graph build memory by processing GTFS shapes more efficiently

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

@@ -2,7 +2,7 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -45,7 +45,7 @@ public class GeometryProcessor {
 
   private static final Logger LOG = LoggerFactory.getLogger(GeometryProcessor.class);
   private static final GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory();
-  private final OtpTransitServiceBuilder transitService;
+  private final OtpTransitServiceBuilder builder;
   // this is a thread-safe implementation
   private final Map<ShapeSegmentKey, LineString> geometriesByShapeSegmentKey =
     new ConcurrentHashMap<>();
@@ -57,13 +57,11 @@ public class GeometryProcessor {
   private final DataImportIssueStore issueStore;
 
   public GeometryProcessor(
-    // TODO OTP2 - Operate on the builder, not the transit service and move the execution of
-    //           - this to where the builder is in context.
-    OtpTransitServiceBuilder transitService,
+    OtpTransitServiceBuilder builder,
     double maxStopToShapeSnapDistance,
     DataImportIssueStore issueStore
   ) {
-    this.transitService = transitService;
+    this.builder = builder;
     this.maxStopToShapeSnapDistance = maxStopToShapeSnapDistance > 0
       ? maxStopToShapeSnapDistance
       : 150;
@@ -88,7 +86,7 @@ public List<LineString> createHopGeometries(Trip trip) {
     }
 
     return Arrays.asList(
-      createGeometry(trip.getShapeId(), transitService.getStopTimesSortedByTrip().get(trip))
+      createGeometry(trip.getShapeId(), builder.getStopTimesSortedByTrip().get(trip))
     );
   }
 
@@ -465,27 +463,32 @@ private LineString getSegmentGeometry(
    * they are unnecessary, and 2) they define 0-length line segments which cause JTS location
    * indexed line to return a segment location of NaN, which we do not want.
    */
-  private List<ShapePoint> getUniqueShapePointsForShapeId(FeedScopedId shapeId) {
-    List<ShapePoint> points = new ArrayList<>(transitService.getShapePoints().get(shapeId));
-    Collections.sort(points);
-    ArrayList<ShapePoint> filtered = new ArrayList<>(points.size());
+  private Collection<ShapePoint> getUniqueShapePointsForShapeId(FeedScopedId shapeId) {
+    var points = builder.getShapePoints().get(shapeId);
+    ArrayList<ShapePoint> filtered = new ArrayList<>();
     ShapePoint last = null;
+    int currentSeq = Integer.MIN_VALUE;
     for (ShapePoint sp : points) {
-      if (last == null || last.getSequence() != sp.getSequence()) {
-        if (last != null && last.getLat() == sp.getLat() && last.getLon() == sp.getLon()) {
+      if (sp.sequence() < currentSeq) {
+        // this should never happen, because the GTFS import should make sure they are already in order.
+        // therefore this just a safety check to detect a programmer error.
+        throw new IllegalStateException(
+          "Shape %s is not sorted in order of sequence. This indicates a bug in OTP.".formatted(
+              shapeId
+            )
+        );
+      }
+      if (last == null || last.sequence() != sp.sequence()) {
+        if (last != null && last.sameCoordinates(sp)) {
           LOG.trace("pair of identical shape points (skipping): {} {}", last, sp);
         } else {
           filtered.add(sp);
         }
       }
       last = sp;
+      currentSeq = sp.sequence();
     }
-    if (filtered.size() != points.size()) {
-      filtered.trimToSize();
-      return filtered;
-    } else {
-      return points;
-    }
+    return filtered;
   }
 
   private LineString getLineStringForShapeId(FeedScopedId shapeId) {
@@ -495,7 +498,7 @@ private LineString getLineStringForShapeId(FeedScopedId shapeId) {
       return geometry;
     }
 
-    List<ShapePoint> points = getUniqueShapePointsForShapeId(shapeId);
+    var points = getUniqueShapePointsForShapeId(shapeId);
     if (points.size() < 2) {
       return null;
     }
@@ -506,8 +509,8 @@ private LineString getLineStringForShapeId(FeedScopedId shapeId) {
 
     int i = 0;
     for (ShapePoint point : points) {
-      coordinates[i] = new Coordinate(point.getLon(), point.getLat());
-      distances[i] = point.getDistTraveled();
+      coordinates[i] = point.coordinate();
+      distances[i] = point.distTraveled();
       if (!point.isDistTraveledSet()) hasAllDistances = false;
       i++;
     }

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

@@ -135,7 +135,7 @@ public void buildGraph() {
       for (GtfsBundle gtfsBundle : gtfsBundles) {
         GtfsMutableRelationalDao gtfsDao = loadBundle(gtfsBundle);
 
-        final String feedId = gtfsBundle.getFeedId();
+        var feedId = gtfsBundle.getFeedId();
         verifyUniqueFeedId(gtfsBundle, feedIdsEncountered, feedId);
 
         feedIdsEncountered.put(feedId, gtfsBundle);
@@ -145,10 +145,9 @@ public void buildGraph() {
           feedId,
           issueStore,
           gtfsBundle.parameters().discardMinTransferTimes(),
-          gtfsDao,
           gtfsBundle.parameters().stationTransferPreference()
         );
-        mapper.mapStopTripAndRouteDataIntoBuilder();
+        mapper.mapStopTripAndRouteDataIntoBuilder(gtfsDao);
 
         OtpTransitServiceBuilder builder = mapper.getBuilder();
         var fareRulesData = mapper.fareRulesData();
@@ -168,7 +167,7 @@ public void buildGraph() {
         );
 
         // We need to run this after the cleaning of the data, as stop indices might have changed
-        mapper.mapAndAddTransfersToBuilder();
+        mapper.mapAndAddTransfersToBuilder(gtfsDao);
 
         GeometryProcessor geometryProcessor = new GeometryProcessor(
           builder,
@@ -310,7 +309,9 @@ private void addTimetableRepositoryToGraph(
   }
 
   private GtfsMutableRelationalDao loadBundle(GtfsBundle gtfsBundle) throws IOException {
-    StoreImpl store = new StoreImpl(new GtfsRelationalDaoImpl());
+    var dao = new GtfsRelationalDaoImpl();
+    dao.setPackShapePoints(true);
+    StoreImpl store = new StoreImpl(dao);
     store.open();
     LOG.info("reading {}", gtfsBundle.feedInfo());
 

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

@@ -0,0 +1,53 @@
+package org.opentripplanner.gtfs.mapping;
+
+import gnu.trove.list.TDoubleList;
+import gnu.trove.list.TIntList;
+import gnu.trove.list.array.TDoubleArrayList;
+import gnu.trove.list.array.TIntArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.opentripplanner.model.ShapePoint;
+
+/**
+ * A representation that stores GTFS shape points in a memory-efficient way and allows you to
+ * iterate over them for further processing.
+ * <p>
+ * The fields of ShapePoints are stored densely in automatically expanding Trove primitive lists.
+ * When later needed, they are reconstituted into objects and sorted on their sequence number.
+ * Only an iterator over the sorted collection escapes rather than the collection itself, providing
+ * some assurance that the objects will be quickly garbage collected.
+ * <p>
+ * This class is package-private but implements Iterable, so you should use that as the return type
+ * of the mapping process.
+ */
+class CompactShape implements Iterable<ShapePoint> {
+
+  private static final int INITIAL_CAPACITY = 50;
+  private static final double NO_DISTANCE = -9999;
+
+  private final TIntList seqs = new TIntArrayList(INITIAL_CAPACITY);
+  private final TDoubleList lats = new TDoubleArrayList(INITIAL_CAPACITY);
+  private final TDoubleList lons = new TDoubleArrayList(INITIAL_CAPACITY);
+  private final TDoubleList dists = new TDoubleArrayList(INITIAL_CAPACITY);
+
+  public void addPoint(org.onebusaway.gtfs.model.ShapePoint shapePoint) {
+    seqs.add(shapePoint.getSequence());
+    lats.add(shapePoint.getLat());
+    lons.add(shapePoint.getLon());
+    dists.add(shapePoint.isDistTraveledSet() ? shapePoint.getDistTraveled() : NO_DISTANCE);
+  }
+
+  @Override
+  public Iterator<ShapePoint> iterator() {
+    return IntStream.range(0, lats.size())
+      .mapToObj(i -> {
+        double dist = dists.get(i);
+        return new ShapePoint(seqs.get(i), lats.get(i), lons.get(i), dist < 0 ? null : dist);
+      })
+      .sorted()
+      .iterator();
+  }
+}

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

@@ -9,11 +9,11 @@
 import java.util.Collection;
 import java.util.function.Function;
 import org.onebusaway.gtfs.model.Stop;
+import org.onebusaway.gtfs.services.GtfsDao;
 import org.onebusaway.gtfs.services.GtfsRelationalDao;
 import org.opentripplanner.ext.fares.model.FareRulesData;
 import org.opentripplanner.framework.application.OTPFeature;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
-import org.opentripplanner.model.ShapePoint;
 import org.opentripplanner.model.impl.OtpTransitServiceBuilder;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.site.RegularStop;
@@ -77,8 +77,6 @@ public class GTFSToOtpTransitServiceMapper {
 
   private final DataImportIssueStore issueStore;
 
-  private final GtfsRelationalDao data;
-
   private final OtpTransitServiceBuilder builder;
 
   private final FareRulesData fareRulesBuilder = new FareRulesData();
@@ -91,7 +89,6 @@ public GTFSToOtpTransitServiceMapper(
     String feedId,
     DataImportIssueStore issueStore,
     boolean discardMinTransferTimes,
-    GtfsRelationalDao data,
     StopTransferPriority stationTransferPreference
   ) {
     var idFactory = new IdFactory(feedId);
@@ -101,7 +98,6 @@ public GTFSToOtpTransitServiceMapper(
     Function<FeedScopedId, Station> stationLookup = id -> builder.getStations().get(id);
     Function<FeedScopedId, RegularStop> stopLookup = id -> builder.getStops().get(id);
 
-    this.data = data;
     this.discardMinTransferTimes = discardMinTransferTimes;
     serviceCalendarMapper = new ServiceCalendarMapper(idFactory);
     serviceCalendarDateMapper = new ServiceCalendarDateMapper(idFactory);
@@ -161,7 +157,7 @@ public FareRulesData fareRulesData() {
     return fareRulesBuilder;
   }
 
-  public void mapStopTripAndRouteDataIntoBuilder() {
+  public void mapStopTripAndRouteDataIntoBuilder(GtfsRelationalDao data) {
     translationHelper.importTranslations(data.getAllTranslations(), data.getAllFeedInfos());
 
     builder.getAgenciesById().addAll(agencyMapper.map(data.getAllAgencies()));
@@ -170,9 +166,10 @@ public void mapStopTripAndRouteDataIntoBuilder() {
     builder.getFeedInfos().addAll(feedInfoMapper.map(data.getAllFeedInfos()));
     builder.getFrequencies().addAll(frequencyMapper.map(data.getAllFrequencies()));
     builder.getRoutes().addAll(routeMapper.map(data.getAllRoutes()));
-    for (ShapePoint shapePoint : shapePointMapper.map(data.getAllShapePoints())) {
-      builder.getShapePoints().put(shapePoint.getShapeId(), shapePoint);
-    }
+    var shapes = shapePointMapper.map(data.getAllShapePoints());
+    builder.getShapePoints().putAll(shapes);
+    // shape points is a large collection, so after mapping it can be cleared
+    data.getAllShapePoints().clear();
 
     mapGtfsStopsToOtpTypes(data.getAllStops());
 
@@ -229,7 +226,7 @@ private void mapGtfsStopsToOtpTypes(Collection<Stop> stops) {
   /**
    * Note! Trip-pattens must be added BEFORE mapping transfers
    */
-  public void mapAndAddTransfersToBuilder() {
+  public void mapAndAddTransfersToBuilder(GtfsDao data) {
     TransferMapper transferMapper = new TransferMapper(
       routeMapper,
       stationMapper,

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

@@ -3,44 +3,27 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import org.opentripplanner.model.ShapePoint;
-import org.opentripplanner.utils.collection.MapUtils;
+import org.checkerframework.checker.units.qual.C;
+import org.opentripplanner.transit.model.framework.FeedScopedId;
 
 /** Responsible for mapping GTFS ShapePoint into the OTP model. */
 class ShapePointMapper {
 
   private final IdFactory idFactory;
-  private final Map<org.onebusaway.gtfs.model.ShapePoint, ShapePoint> mappedShapePoints =
-    new HashMap<>();
 
   ShapePointMapper(IdFactory idFactory) {
     this.idFactory = idFactory;
   }
 
-  Collection<ShapePoint> map(Collection<org.onebusaway.gtfs.model.ShapePoint> allShapePoints) {
-    return MapUtils.mapToList(allShapePoints, this::map);
-  }
-
-  /** Map from GTFS to OTP model, {@code null} safe. */
-  ShapePoint map(org.onebusaway.gtfs.model.ShapePoint orginal) {
-    return orginal == null ? null : mappedShapePoints.computeIfAbsent(orginal, this::doMap);
-  }
-
-  private ShapePoint doMap(org.onebusaway.gtfs.model.ShapePoint rhs) {
-    ShapePoint lhs = new ShapePoint();
-
-    lhs.setShapeId(idFactory.createId(rhs.getShapeId(), "shape point"));
-    lhs.setSequence(rhs.getSequence());
-    lhs.setLat(rhs.getLat());
-    lhs.setLon(rhs.getLon());
-    lhs.setDistTraveled(rhs.getDistTraveled());
-
-    // Skip mapping of proxy
-    // private transient StopTimeProxy proxy;
-    if (rhs.getProxy() != null) {
-      throw new IllegalStateException("Did not expect proxy to be set! Data: " + rhs);
+  Map<FeedScopedId, CompactShape> map(
+    Collection<org.onebusaway.gtfs.model.ShapePoint> allShapePoints
+  ) {
+    var ret = new HashMap<FeedScopedId, CompactShape>();
+    for (var shapePoint : allShapePoints) {
+      var shapeId = idFactory.createId(shapePoint.getShapeId(), "shape point");
+      var shape = ret.computeIfAbsent(shapeId, id -> new CompactShape());
+      shape.addPoint(shapePoint);
     }
-
-    return lhs;
+    return ret;
   }
 }

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

@@ -88,7 +88,6 @@ private StopTime doMap(org.onebusaway.gtfs.model.StopTime rhs) {
     lhs.setTimepoint(rhs.getTimepoint());
     lhs.setStopSequence(rhs.getStopSequence());
     lhs.setStopHeadsign(stopHeadsign);
-    lhs.setRouteShortName(rhs.getRouteShortName());
     lhs.setPickupType(PickDropMapper.map(rhs.getPickupType()));
     lhs.setDropOffType(PickDropMapper.map(rhs.getDropOffType()));
     lhs.setShapeDistTraveled(rhs.getShapeDistTraveled());

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

@@ -2,7 +2,6 @@
 
 import com.google.common.collect.Multimap;
 import java.util.Collection;
-import java.util.List;
 import java.util.Map;
 import org.opentripplanner.ext.flex.trip.FlexTrip;
 import org.opentripplanner.model.transfer.ConstrainedTransfer;
@@ -52,20 +51,12 @@ public interface OtpTransitService {
    */
   Collection<FeedScopedId> getAllServiceIds();
 
-  List<ShapePoint> getShapePointsForShapeId(FeedScopedId shapeId);
-
   Collection<Entrance> getAllEntrances();
 
   Collection<PathwayNode> getAllPathwayNodes();
 
   Collection<BoardingArea> getAllBoardingAreas();
 
-  /**
-   * @return the list of {@link StopTime} objects associated with the trip, sorted by {@link
-   * StopTime#getStopSequence()}
-   */
-  List<StopTime> getStopTimesForTrip(Trip trip);
-
   Collection<ConstrainedTransfer> getAllTransfers();
 
   Collection<TripPattern> getTripPatterns();