Speed up IslandPruningModule with a cheaper edge-traversability check

application/src/main/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningModule.java

@@ -6,11 +6,10 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
+import java.util.Set;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
@@ -21,7 +20,6 @@
 import org.opentripplanner.street.model.StreetMode;
 import org.opentripplanner.street.model.StreetTraversalPermission;
 import org.opentripplanner.street.model.edge.AreaEdge;
-import org.opentripplanner.street.model.edge.AreaGroup;
 import org.opentripplanner.street.model.edge.Edge;
 import org.opentripplanner.street.model.edge.StreetEdge;
 import org.opentripplanner.street.model.vertex.StreetVertex;
@@ -31,6 +29,7 @@
 import org.opentripplanner.street.search.request.StreetSearchRequest;
 import org.opentripplanner.street.search.state.State;
 import org.opentripplanner.transit.service.TransitRepository;
+import org.opentripplanner.utils.collection.StreamUtils;
 import org.opentripplanner.utils.time.DurationUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -95,27 +94,18 @@ public void buildGraph() {
     // note that visibility vertices must not be removed from the graph
     // because serialization will break. Edge lists are reconstructed
     // only for graph vertices after loading the graph
-    HashSet<AreaGroup> areas = new HashSet<>();
-    HashSet<Vertex> visibilityVertices = new HashSet<>();
-
-    for (AreaEdge ae : graph.findEdges(AreaEdge.class)) {
-      areas.add(ae.getArea());
-    }
-    for (AreaGroup a : areas) {
-      visibilityVertices.addAll(a.visibilityVertices());
-    }
+    Set<Vertex> visibilityVertices = StreamUtils.ofIterable(graph.findEdges(AreaEdge.class))
+      .distinct()
+      .flatMap(v -> v.getArea().visibilityVertices().stream())
+      .collect(Collectors.toSet());
 
     int removed = 0;
-    List<Vertex> toRemove = new LinkedList<>();
     for (Vertex v : graph.getVerticesOfType(StreetVertex.class)) {
       if (v.getDegreeOut() + v.getDegreeIn() == 0 && !visibilityVertices.contains(v)) {
-        toRemove.add(v);
+        graph.remove(v);
+        removed += 1;
       }
     }
-    for (Vertex v : toRemove) {
-      graph.remove(v);
-      removed += 1;
-    }
     LOG.info("Removed {} edgeless street vertices", removed);
 
     LOG.info(
@@ -287,28 +277,54 @@ private void collectNeighbourVertices(
       if (!(gv instanceof StreetVertex)) {
         continue;
       }
-      State s0 = new State(gv, request);
+      State s0 = null;
       for (Edge e : gv.getOutgoing()) {
-        if (
-          e instanceof StreetEdge se && shouldMatchNoThruType != se.isNoThruTraffic(traverseMode)
-        ) {
-          continue;
-        }
-        State[] states = e.traverse(s0);
-        if (State.isEmpty(states)) {
-          continue;
-        }
-        for (State state : states) {
-          Vertex out = state.getVertex();
+        if (e instanceof StreetEdge se) {
+          if (shouldMatchNoThruType != se.isNoThruTraffic(traverseMode)) {
+            continue;
+          }
+          if (!canTraverse(se, traverseMode)) {
+            continue;
+          }
+          Vertex out = se.getToVertex();
           neighborsForVertex.put(gv, out);
 
           // note: this assumes that edges are bi-directional. Maybe explicit state traversal is needed for CAR mode.
           neighborsForVertex.put(out, gv);
+        } else {
+          // Fall back to a real traversal for edge types (eg. escalators, pathways, vehicle
+          // rental/parking edges) that don't behave like a plain permission-gated street edge.
+          if (s0 == null) {
+            s0 = new State(gv, request);
+          }
+          State[] states = e.traverse(s0);
+          if (State.isEmpty(states)) {
+            continue;
+          }
+          for (State state : states) {
+            Vertex out = state.getVertex();
+            neighborsForVertex.put(gv, out);
+            neighborsForVertex.put(out, gv);
+          }
         }
       }
     }
   }
 
+  /**
+   * Cheap connectivity-only equivalent of {@link StreetEdge#traverse}, for the plain WALK/
+   * BICYCLE/CAR travel modes this module cares about: a permission (incl. barrier vertex) check,
+   * with the same "walk the bike if it can't be ridden" fallback {@link StreetEdge#traverse}
+   * applies. This avoids allocating a {@link State}/{@code StateEditor} and computing speed/cost,
+   * none of which this module reads - it only needs to know whether the edge can be used at all.
+   */
+  private static boolean canTraverse(StreetEdge edge, TraverseMode traverseMode) {
+    if (traverseMode == TraverseMode.BICYCLE) {
+      return edge.canTraverse(TraverseMode.BICYCLE) || edge.canTraverse(TraverseMode.WALK);
+    }
+    return edge.canTraverse(traverseMode);
+  }
+
   private int collectSubGraphs(
     ArrayMultimap<Vertex, Vertex> neighborsForVertex,
     // put new subgraphs here

application/src/test-fixtures/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningBenchmark.java

@@ -0,0 +1,141 @@
+package org.opentripplanner.graph_builder.module.islandpruning;
+
+import java.io.File;
+import java.io.InputStream;
+import java.lang.management.ManagementFactory;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.time.Duration;
+import java.time.Instant;
+import org.opentripplanner.framework.io.HttpHeaders;
+import org.opentripplanner.framework.io.OtpHttpClientFactory;
+import org.opentripplanner.street.graph.Graph;
+import org.opentripplanner.street.model.edge.StreetEdge;
+import org.opentripplanner.street.model.vertex.StreetVertex;
+import org.opentripplanner.utils.collection.ListUtils;
+import org.opentripplanner.utils.time.DurationUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Standalone (non-JUnit) benchmark for {@link IslandPruningModule}, run manually - it downloads a
+ * full country OSM extract, builds a street graph from it, and times island pruning on it.
+ * {@link IslandPruningModule} is CPU/memory sensitive at country scale, and that only shows up on
+ * real-sized data - the unit tests only exercise small fixtures.
+ * <p>
+ * Run with (bump -Xmx as needed for the chosen extract):
+ * <pre>
+ * MAVEN_OPTS="-Xmx6g" mvn --projects application exec:java \
+ *   -Dexec.mainClass="org.opentripplanner.graph_builder.module.islandpruning.IslandPruningBenchmark" \
+ *   -Dexec.classpathScope=test
+ * </pre>
+ * The OSM extract is downloaded once into {@code -Dbenchmark.dataDir} (default:
+ * {@code $TMPDIR/otp-benchmark}) and reused on subsequent runs. Override the extract with
+ * {@code -Dbenchmark.osmUrl=<url>}.
+ */
+public class IslandPruningBenchmark {
+
+  private static final Logger LOG = LoggerFactory.getLogger(IslandPruningBenchmark.class);
+
+  private static final String DEFAULT_URL =
+    "https://download.geofabrik.de/europe/norway-latest.osm.pbf";
+
+  private static final Duration DOWNLOAD_TIMEOUT = Duration.ofMinutes(5);
+
+  public static void main(String[] args) throws Exception {
+    File osmFile = downloadIfMissing(
+      System.getProperty("benchmark.osmUrl", DEFAULT_URL),
+      Path.of(
+        System.getProperty(
+          "benchmark.dataDir",
+          System.getProperty("java.io.tmpdir", "/tmp") + "/otp-benchmark"
+        )
+      )
+    );
+
+    System.out.println("Building street graph from " + osmFile + " ...");
+    Instant osmStart = Instant.now();
+    Graph graph = IslandPruningUtils.buildStreetGraph(osmFile);
+    Duration osmDuration = Duration.between(osmStart, Instant.now());
+
+    int streetVertices = graph.getVerticesOfType(StreetVertex.class).size();
+    int streetEdges = countEdges(graph);
+    System.out.printf(
+      "OSM module: %s, %d street vertices, %d street edges%n",
+      DurationUtils.durationToStr(osmDuration),
+      streetVertices,
+      streetEdges
+    );
+
+    System.gc();
+    long heapBefore = usedHeapBytes();
+
+    System.out.println("Running island pruning ...");
+    Instant pruneStart = Instant.now();
+    IslandPruningUtils.prune(graph, IslandPruningParameters.DEFAULTS);
+    Duration pruneDuration = Duration.between(pruneStart, Instant.now());
+
+    long heapAfterPruning = usedHeapBytes();
+    System.gc();
+    long heapAfterGc = usedHeapBytes();
+
+    System.out.println();
+    System.out.println("==== Results ====");
+    System.out.printf("OSM module:      %8s%n", DurationUtils.durationToStr(osmDuration));
+    System.out.printf("Island pruning:  %8s%n", DurationUtils.durationToStr(pruneDuration));
+    System.out.printf(
+      "Heap during pruning:  %,d MB (before pruning) -> %,d MB (right after, pre-GC)%n",
+      heapBefore / 1_000_000,
+      heapAfterPruning / 1_000_000
+    );
+    System.out.printf("Heap after forced GC post-pruning: %,d MB%n", heapAfterGc / 1_000_000);
+    System.out.printf(
+      "Remaining street vertices/edges after pruning: %d / %d%n",
+      graph.getVerticesOfType(StreetVertex.class).size(),
+      countEdges(graph)
+    );
+  }
+
+  private static int countEdges(Graph graph) {
+    return ListUtils.countIterable(graph.findEdges(StreetEdge.class));
+  }
+
+  private static long usedHeapBytes() {
+    return ManagementFactory.getMemoryMXBean().getHeapMemoryUsage().getUsed();
+  }
+
+  private static File downloadIfMissing(String url, Path dataDir) throws Exception {
+    Files.createDirectories(dataDir);
+    String fileName = url.substring(url.lastIndexOf('/') + 1);
+    Path target = dataDir.resolve(fileName);
+
+    if (Files.exists(target) && Files.size(target) > 0) {
+      System.out.printf(
+        "Using cached OSM extract at %s (%,d MB)%n",
+        target,
+        Files.size(target) / 1_000_000
+      );
+      return target.toFile();
+    }
+
+    System.out.println("Downloading " + url + " to " + target + " ...");
+    Path tmp = dataDir.resolve(fileName + ".part");
+    try (var clientFactory = new OtpHttpClientFactory()) {
+      var client = clientFactory.create(LOG);
+      try (
+        InputStream in = client.getAsInputStream(
+          URI.create(url),
+          DOWNLOAD_TIMEOUT,
+          HttpHeaders.empty()
+        )
+      ) {
+        Files.copy(in, tmp, StandardCopyOption.REPLACE_EXISTING);
+      }
+    }
+    Files.move(tmp, target, StandardCopyOption.REPLACE_EXISTING);
+    System.out.printf("Download complete: %,d MB%n", Files.size(target) / 1_000_000);
+    return target.toFile();
+  }
+}

application/src/test-fixtures/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningUtils.java

@@ -13,25 +13,33 @@ class IslandPruningUtils {
 
   static GraphSummarizer buildOsmGraph(File osmFile, IslandPruningParameters parameters) {
     try {
-      var graph = new Graph();
-      var osmProvider = new DefaultOsmProvider(osmFile, true);
-
-      var osmModule = OsmModuleTestFactory.of(osmProvider)
-        .withGraph(graph)
-        .builder()
-        .withEdgeNamer(new TestNamer())
-        .build();
-
-      osmModule.buildGraph();
-
+      var graph = buildStreetGraph(osmFile);
       prune(graph, parameters);
-
       return new GraphSummarizer(graph);
     } catch (Exception e) {
       throw new RuntimeException(e);
     }
   }
 
+  /**
+   * Builds a street graph from a single OSM file, without running island pruning. Split out of
+   * {@link #buildOsmGraph} so callers (eg. a standalone benchmark) can time OSM import and island
+   * pruning separately.
+   */
+  static Graph buildStreetGraph(File osmFile) {
+    var graph = new Graph();
+    var osmProvider = new DefaultOsmProvider(osmFile, true);
+
+    var osmModule = OsmModuleTestFactory.of(osmProvider)
+      .withGraph(graph)
+      .builder()
+      .withEdgeNamer(new TestNamer())
+      .build();
+
+    osmModule.buildGraph();
+    return graph;
+  }
+
   /**
    * Runs {@link IslandPruningModule} against an already-built graph.
    */

application/src/test-fixtures/java/org/opentripplanner/street/model/StreetModelForTest.java

@@ -134,6 +134,27 @@ public static StreetEdge areaEdge(
       .buildAndConnect();
   }
 
+  public static void areaEdge(
+    IntersectionVertex from,
+    IntersectionVertex to,
+    AreaGroup area,
+    boolean back
+  ) {
+    var geometry = GeometryUtils.getGeometryFactory().createLineString(new Coordinate[] {
+      from.getCoordinate(),
+      to.getCoordinate(),
+    });
+    new AreaEdgeBuilder()
+      .withFromVertex(from)
+      .withToVertex(to)
+      .withGeometry(geometry)
+      .withName("area boundary")
+      .withPermission(StreetTraversalPermission.PEDESTRIAN)
+      .withBack(back)
+      .withArea(area)
+      .buildAndConnect();
+  }
+
   public static StreetEdge streetEdge(
     StreetVertex from,
     StreetVertex to,

application/src/test/java/org/opentripplanner/graph_builder/module/islandpruning/moduletests/VisibilityVertexRetainedTest.java

@@ -0,0 +1,48 @@
+package org.opentripplanner.graph_builder.module.islandpruning.moduletests;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.opentripplanner.street.model.StreetModelForTest.areaEdge;
+import static org.opentripplanner.street.model.StreetModelForTest.bidirectional;
+import static org.opentripplanner.street.model.StreetModelForTest.intersectionVertex;
+
+import java.util.Set;
+import org.junit.jupiter.api.Test;
+import org.opentripplanner.graph_builder.module.islandpruning.IslandPruningEnvironment;
+import org.opentripplanner.graph_builder.module.islandpruning.IslandPruningParameters;
+import org.opentripplanner.street.model.edge.AreaGroup;
+
+/**
+ * After pruning, street vertices without any edges are removed from the graph. Visibility vertices
+ * of a walkable area are exempt even when edgeless, since the area still references them and
+ * graph serialization breaks if they are missing.
+ */
+class VisibilityVertexRetainedTest {
+
+  @Test
+  void edgelessVisibilityVertexIsRetained() {
+    // Main street network: a small square of four intersections, large enough to never be
+    // considered for pruning. The side a-b runs along a walkable area.
+    var a = intersectionVertex(0, 0);
+    var b = intersectionVertex(0, 1);
+    var c = intersectionVertex(1, 1);
+    var d = intersectionVertex(1, 0);
+
+    // A visibility vertex of the area which has no edges of its own.
+    var visibilityVertex = intersectionVertex(0.5, 0.5);
+    // An unrelated vertex without any edges.
+    var orphan = intersectionVertex(10, 10);
+
+    var area = AreaGroup.of(null).withVisibilityVertices(Set.of(visibilityVertex)).build();
+    areaEdge(a, b, area, false);
+    areaEdge(b, a, area, true);
+    bidirectional(b, c);
+    bidirectional(c, d);
+    bidirectional(d, a);
+
+    var summarizer = IslandPruningEnvironment.of(a, b, c, d, visibilityVertex, orphan).prune(
+      IslandPruningParameters.DEFAULTS
+    );
+
+    assertThat(summarizer.graph().getVertices()).containsExactly(a, b, c, d, visibilityVertex);
+  }
+}