Merge pull request opentripplanner#7480 from opentripplanner/flex-graph-path-perf

Optimize computation of flex paths

Author: leonardehrenfried

application/src/ext-test/java/org/opentripplanner/ext/flex/flexpathcalculator/StateToFlexPathMapperTest.java

@@ -0,0 +1,30 @@
+package org.opentripplanner.ext.flex.flexpathcalculator;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.opentripplanner.street.search.state.TestStateBuilder.ofDriving;
+
+import org.junit.jupiter.api.Test;
+
+class StateToFlexPathMapperTest {
+
+  private static final int EPSILON = 100;
+
+  @Test
+  void departAt() {
+    var state = ofDriving().streetEdge().streetEdge().streetEdge().build();
+    var flexPath = StateToFlexPathMapper.map(state);
+    assertThat(flexPath.distanceMeters).isWithin(EPSILON).of(471509);
+    assertEquals(42100, flexPath.durationSeconds);
+    assertEquals("LINESTRING (1 1, 2 2, 3 3, 4 4)", flexPath.getGeometry().toString());
+  }
+
+  @Test
+  void arriveBy() {
+    var state = ofDriving().streetEdge().streetEdge().streetEdge().build().reverse();
+    var flexPath = StateToFlexPathMapper.map(state);
+    assertThat(flexPath.distanceMeters).isWithin(EPSILON).of(471509);
+    assertEquals(42100, flexPath.durationSeconds);
+    assertEquals("LINESTRING (1 1, 2 2, 3 3, 4 4)", flexPath.getGeometry().toString());
+  }
+}

application/src/ext/java/org/opentripplanner/ext/flex/flexpathcalculator/StateToFlexPathMapper.java

@@ -0,0 +1,61 @@
+package org.opentripplanner.ext.flex.flexpathcalculator;
+
+import com.google.common.collect.Iterables;
+import java.util.function.Supplier;
+import org.locationtech.jts.geom.LineString;
+import org.opentripplanner.street.geometry.GeometryUtils;
+import org.opentripplanner.street.model.edge.Edge;
+import org.opentripplanner.street.search.state.State;
+
+/**
+ * Extracts the geometry and distance from a {@link State} chain produced by
+ * an A* street search. The state chain is a linked list from the final state back to the origin via
+ * {@link State#getBackState()}/{@link State#getBackEdge()}.
+ * <p>
+ * This utility encapsulates the direction-dependent ordering: for depart-after searches the chain
+ * yields a geometry in reverse chronological order (newest first), while for arriveBy searches the chain
+ * already yields edges in chronological order.
+ * Implementation note: an earlier design relied on {@link org.opentripplanner.astar.model.GraphPath}
+ * to extract the list of edges in chronological order / reverse chronological order.
+ * The current implementation is optimized for reducing memory allocation and CPU usage.
+ */
+class StateToFlexPathMapper {
+
+  /**
+   * Walk the state chain and collect edges in chronological order (origin → destination), summing
+   * up the distance along the way.
+   */
+  static FlexPath map(State state) {
+    double distance_m = 0.0;
+
+    // TODO: compute this during traversal of the edges in a follow up PR
+    for (var backEdge : state.listBackEdges()) {
+      distance_m += backEdge.getDistanceMeters();
+    }
+
+    // computing the linestring from the graph path is a surprisingly expensive operation
+    // so we delay it until it's actually needed. since most flex paths are never shown to the user
+    // this improves performance quite a bit.
+
+    Supplier<LineString> geometrySupplier = () -> {
+      if (state.getRequest().arriveBy()) {
+        var geometries = Iterables.transform(state.listBackEdges(), Edge::getGeometry);
+        return GeometryUtils.concatenateLineStrings(geometries);
+      } else {
+        var geometries = Iterables.transform(state.listBackEdges(), StateToFlexPathMapper::reverse);
+        return GeometryUtils.concatenateLineStrings(geometries).reverse();
+      }
+    };
+
+    return new FlexPath((int) distance_m, (int) state.getElapsedTimeSeconds(), geometrySupplier);
+  }
+
+  private static LineString reverse(Edge e) {
+    var geom = e.getGeometry();
+    if (geom == null) {
+      return null;
+    } else {
+      return geom.reverse();
+    }
+  }
+}

application/src/ext/java/org/opentripplanner/ext/flex/flexpathcalculator/StreetFlexPathCalculator.java

@@ -3,11 +3,9 @@
 import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
-import org.opentripplanner.astar.model.GraphPath;
 import org.opentripplanner.astar.model.ShortestPathTree;
 import org.opentripplanner.astar.strategy.DurationSkipEdgeStrategy;
 import org.opentripplanner.framework.application.OTPRequestTimeoutException;
-import org.opentripplanner.street.geometry.GeometryUtils;
 import org.opentripplanner.street.model.StreetMode;
 import org.opentripplanner.street.model.edge.Edge;
 import org.opentripplanner.street.model.vertex.Vertex;
@@ -61,20 +59,12 @@ public FlexPath calculateFlexPath(
       cache.put(originVertex, shortestPathTree);
     }
 
-    GraphPath<State, Edge, Vertex> path = shortestPathTree.getPath(destinationVertex);
-    if (path == null) {
+    var state = shortestPathTree.getState(destinationVertex);
+    if (state == null) {
       return null;
     }
 
-    int distance = (int) path.edges.stream().mapToDouble(Edge::getDistanceMeters).sum();
-    int duration = path.getDuration();
-
-    // computing the linestring from the graph path is a surprisingly expensive operation
-    // so we delay it until it's actually needed. since most flex paths are never shown to the user
-    // this improves performance quite a bit.
-    return new FlexPath(distance, duration, () ->
-      GeometryUtils.concatenateLineStrings(path.edges, Edge::getGeometry)
-    );
+    return StateToFlexPathMapper.map(state);
   }
 
   private ShortestPathTree<State, Edge, Vertex> routeToMany(Vertex vertex) {

application/src/test-fixtures/java/org/opentripplanner/street/search/state/TestStateBuilder.java

@@ -73,7 +73,7 @@ public static TestStateBuilder ofWalking() {
   }
 
   /**
-   * Create an initial state that start in a car.
+   * Create an initial state that starts in a car.
    */
   public static TestStateBuilder ofDriving() {
     return new TestStateBuilder(StreetMode.CAR);

astar/src/main/java/org/opentripplanner/astar/model/GraphPath.java

@@ -7,6 +7,9 @@
 
 /**
  * A shortest path on the graph.
+ * <p>
+ * WARNING: This class is often a hotspot as it eagerly traverses the state chain. Avoid
+ * as much as possible.
  */
 public class GraphPath<
   State extends AStarState<State, Edge, Vertex>,

astar/src/main/java/org/opentripplanner/astar/model/ShortestPathTree.java

@@ -6,6 +6,7 @@
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import javax.annotation.Nullable;
 import org.opentripplanner.astar.spi.AStarEdge;
 import org.opentripplanner.astar.spi.AStarState;
 import org.opentripplanner.astar.spi.AStarVertex;
@@ -141,6 +142,7 @@ public boolean add(State newState) {
    * @return a 'best' state at that vertex
    */
   @SuppressWarnings("unchecked")
+  @Nullable
   public State getState(Vertex dest) {
     Object existing = stateSets.get(dest);
     if (existing == null) {

street/src/main/java/org/opentripplanner/street/geometry/GeometryUtils.java

@@ -1,5 +1,6 @@
 package org.opentripplanner.street.geometry;
 
+import com.google.common.collect.Iterables;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -8,6 +9,7 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 import org.geojson.GeoJsonObject;
 import org.geojson.LngLatAlt;
 import org.locationtech.jts.algorithm.ConvexHull;
@@ -70,19 +72,29 @@ public static LineString makeLineString(WgsCoordinate... coordinates) {
     return makeLineString(Arrays.stream(coordinates).map(WgsCoordinate::asJtsCoordinate).toList());
   }
 
+  /// Convert an iterable of T by applying a mapping function to each element and concatenate the
+  /// resulting LineStrings.
+  ///
+  /// For the best performance and lowest number of allocations pass in an [Iterable] rather
+  /// than a materialized [Collection].
   public static <T> LineString concatenateLineStrings(
-    List<T> inputObjects,
+    Iterable<T> inputObjects,
     Function<T, LineString> mapper
   ) {
-    return concatenateLineStrings(inputObjects.stream().map(mapper).toList());
+    return concatenateLineStrings(Iterables.transform(inputObjects, mapper::apply));
   }
 
-  public static LineString concatenateLineStrings(List<LineString> lineStrings) {
+  /// Convert an [Iterable] of [LineString] by applying a mapping function to each element and
+  /// concatenate the resulting LineStrings.
+  ///
+  /// For the best performance and lowest number of allocations pass in an [Iterable] rather
+  /// than a materialized [Collection].
+  public static LineString concatenateLineStrings(Iterable<LineString> lineStrings) {
     GeometryFactory factory = getGeometryFactory();
     Predicate<Coordinate[]> nonZeroLength = coordinates -> coordinates.length != 0;
+
     return factory.createLineString(
-      lineStrings
-        .stream()
+      StreamSupport.stream(lineStrings.spliterator(), false)
         .filter(Objects::nonNull)
         .map(LineString::getCoordinates)
         .filter(nonZeroLength)

street/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequestBuilder.java

@@ -163,5 +163,4 @@ Instant startTimeOrNow() {
   public StreetSearchRequest build() {
     return new StreetSearchRequest(this);
   }
-
 }

street/src/main/java/org/opentripplanner/street/search/state/BackEdgeIterator.java

@@ -0,0 +1,32 @@
+package org.opentripplanner.street.search.state;
+
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import org.opentripplanner.street.model.edge.Edge;
+
+/**
+ * Iterator for traversing back edges in a state chain without any extra allocations.
+ */
+class BackEdgeIterator implements Iterator<Edge> {
+
+  private State current;
+
+  public BackEdgeIterator(State state) {
+    current = state;
+  }
+
+  @Override
+  public boolean hasNext() {
+    return current != null && current.getBackState() != null;
+  }
+
+  @Override
+  public Edge next() {
+    if(!hasNext()){
+      throw new NoSuchElementException("No more back edges available");
+    }
+    var ret = current;
+    current = current.getBackState();
+    return ret.getBackEdge();
+  }
+}

street/src/main/java/org/opentripplanner/street/search/state/State.java

@@ -490,6 +490,13 @@ protected State clone() {
     return ret;
   }
 
+  /**
+   * Returns an efficient iterable that allows traversing the edge chain backwards.
+   */
+  public Iterable<Edge> listBackEdges() {
+    return () -> new BackEdgeIterator(this);
+  }
+
   public String toString() {
     return ToStringBuilder.of(State.class)
       .addDateTime("time", getTime())