diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningModule.java b/application/src/main/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningModule.java index 5fb3635b463..137bcedf175 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningModule.java +++ b/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 areas = new HashSet<>(); - HashSet visibilityVertices = new HashSet<>(); - - for (AreaEdge ae : graph.findEdges(AreaEdge.class)) { - areas.add(ae.getArea()); - } - for (AreaGroup a : areas) { - visibilityVertices.addAll(a.visibilityVertices()); - } + Set visibilityVertices = StreamUtils.ofIterable(graph.findEdges(AreaEdge.class)) + .distinct() + .flatMap(v -> v.getArea().visibilityVertices().stream()) + .collect(Collectors.toSet()); int removed = 0; - List 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 neighborsForVertex, // put new subgraphs here diff --git a/application/src/test-fixtures/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningBenchmark.java b/application/src/test-fixtures/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningBenchmark.java new file mode 100644 index 00000000000..fd0ee24806b --- /dev/null +++ b/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. + *

+ * Run with (bump -Xmx as needed for the chosen extract): + * 

+ * MAVEN_OPTS="-Xmx6g" mvn --projects application exec:java \
+ *   -Dexec.mainClass="org.opentripplanner.graph_builder.module.islandpruning.IslandPruningBenchmark" \
+ *   -Dexec.classpathScope=test
+ *
+ * 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=}. + */ +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(); + } +} diff --git a/application/src/test-fixtures/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningUtils.java b/application/src/test-fixtures/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningUtils.java index dc85cbb786b..b0a87ab6a9d 100644 --- a/application/src/test-fixtures/java/org/opentripplanner/graph_builder/module/islandpruning/IslandPruningUtils.java +++ b/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. */ diff --git a/application/src/test-fixtures/java/org/opentripplanner/street/model/StreetModelForTest.java b/application/src/test-fixtures/java/org/opentripplanner/street/model/StreetModelForTest.java index ce2a67ca2bf..61b3ecb02b5 100644 --- a/application/src/test-fixtures/java/org/opentripplanner/street/model/StreetModelForTest.java +++ b/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, diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/islandpruning/moduletests/VisibilityVertexRetainedTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/islandpruning/moduletests/VisibilityVertexRetainedTest.java new file mode 100644 index 00000000000..a48fce57f9f --- /dev/null +++ b/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); + } +}