Skip to content

Commit 3cfea82

Browse files
vpaturetvpaturet
authored
Merge pull request opentripplanner#7455 from entur/avoid-linkedlist-in-nearbystop
Reduce allocation in NearbyStop access/egress path
2 parents 705b80b + 7af76b4 commit 3cfea82

4 files changed

Lines changed: 198 additions & 13 deletions

File tree

‎application/src/main/java/org/opentripplanner/routing/graphfinder/ChronologicalGraphPath.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package org.opentripplanner.routing.graphfinder;
2+
3+
import java.util.ArrayList;
4+
import java.util.Collections;
5+
import java.util.List;
6+
import org.opentripplanner.street.model.edge.Edge;
7+
import org.opentripplanner.street.search.state.State;
8+
9+
/**
10+
* Extracts the traversed edges and effective walk distance from a {@link State} chain produced by
11+
* an A* street search. The state chain is a linked list from the final state back to the origin via
12+
* {@link State#getBackState()}/{@link State#getBackEdge()}.
13+
* <p>
14+
* This utility encapsulates the direction-dependent ordering: for depart-after searches the chain
15+
* yields edges in reverse chronological order (newest first), while for arriveBy searches the chain
16+
* already yields edges in chronological order.
17+
* <p>
18+
* Implementation note: an earlier design relied on {@link org.opentripplanner.astar.model.GraphPath}
19+
* which allocates a {@code LinkedList<State>} and, for depart-after searches, a full reversed copy
20+
* of the state chain via {@code State.reverse()}. This implementation avoids both by collecting
21+
* only the edges into a single {@code ArrayList} and reversing it in place.
22+
*/
23+
class ChronologicalGraphPath {
24+
25+
private final List<Edge> edges;
26+
private final double effectiveWalkDistance;
27+
28+
private ChronologicalGraphPath(List<Edge> edges, double effectiveWalkDistance) {
29+
this.edges = edges;
30+
this.effectiveWalkDistance = effectiveWalkDistance;
31+
}
32+
33+
/**
34+
* Walk the state chain and collect edges in chronological order (origin → destination), summing
35+
* up the effective walk distance along the way.
36+
*/
37+
static ChronologicalGraphPath of(State state) {
38+
double walkDistance = 0.0;
39+
var edges = new ArrayList<Edge>();
40+
for (State cur = state; cur != null; cur = cur.getBackState()) {
41+
Edge backEdge = cur.getBackEdge();
42+
if (backEdge != null && cur.getBackState() != null) {
43+
walkDistance += backEdge.getEffectiveWalkDistance();
44+
edges.add(backEdge);
45+
}
46+
}
47+
// For depart-after, edges are in reverse chronological order; reverse to chronological.
48+
// For arriveBy, the A* searched backward so edges are already chronological.
49+
if (!state.getRequest().arriveBy()) {
50+
// We considered using `ArrayList.reversed()` (view) here, but there is no significant performance gain
51+
// and the graph deserialization would break. See PR #7455.
52+
Collections.reverse(edges);
53+
}
54+
return new ChronologicalGraphPath(edges, walkDistance);
55+
}
56+
57+
List<Edge> edges() {
58+
return edges;
59+
}
60+
61+
double effectiveWalkDistance() {
62+
return effectiveWalkDistance;
63+
}
64+
}

‎application/src/main/java/org/opentripplanner/routing/graphfinder/NearbyStop.java‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
package org.opentripplanner.routing.graphfinder;
22

33
import java.time.Duration;
4-
import java.util.ArrayList;
54
import java.util.Collections;
65
import java.util.List;
76
import java.util.Locale;
87
import java.util.Objects;
9-
import org.opentripplanner.astar.model.GraphPath;
108
import org.opentripplanner.street.model.edge.Edge;
119
import org.opentripplanner.street.search.state.State;
1210
import org.opentripplanner.transit.model.site.StopLocation;
@@ -35,14 +33,8 @@ public NearbyStop(StopLocation stop, double distance, List<Edge> edges, State st
3533
* away it is and the geometry of the path leading up to the given State.
3634
*/
3735
public static NearbyStop nearbyStopForState(State state, StopLocation stop) {
38-
double effectiveWalkDistance = 0.0;
39-
var graphPath = new GraphPath<>(state);
40-
var edges = new ArrayList<Edge>();
41-
for (Edge edge : graphPath.edges) {
42-
effectiveWalkDistance += edge.getEffectiveWalkDistance();
43-
edges.add(edge);
44-
}
45-
return new NearbyStop(stop, effectiveWalkDistance, edges, state);
36+
var result = ChronologicalGraphPath.of(state);
37+
return new NearbyStop(stop, result.effectiveWalkDistance(), result.edges(), state);
4638
}
4739

4840
/**

‎application/src/test/java/org/opentripplanner/routing/graphfinder/ChronologicalGraphPathTest.java‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package org.opentripplanner.routing.graphfinder;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertSame;
5+
import static org.junit.jupiter.api.Assertions.assertTrue;
6+
7+
import java.time.Instant;
8+
import org.junit.jupiter.api.Test;
9+
import org.opentripplanner.street.model.StreetMode;
10+
import org.opentripplanner.street.model.StreetModelForTest;
11+
import org.opentripplanner.street.model.edge.StreetEdge;
12+
import org.opentripplanner.street.model.vertex.StreetVertex;
13+
import org.opentripplanner.street.search.request.StreetSearchRequest;
14+
import org.opentripplanner.street.search.state.State;
15+
16+
class ChronologicalGraphPathTest {
17+
18+
private static final Instant START_TIME = Instant.parse("2024-01-15T12:00:00Z");
19+
20+
@Test
21+
void departAfterEdgesInChronologicalOrder() {
22+
var graph = createThreeEdgeGraph();
23+
var state = walkForward(graph);
24+
25+
var result = ChronologicalGraphPath.of(state);
26+
27+
assertEquals(3, result.edges().size());
28+
assertSame(graph.e1, result.edges().get(0));
29+
assertSame(graph.e2, result.edges().get(1));
30+
assertSame(graph.e3, result.edges().get(2));
31+
}
32+
33+
@Test
34+
void arriveByEdgesInChronologicalOrder() {
35+
var graph = createThreeEdgeGraph();
36+
var state = walkBackward(graph);
37+
38+
var result = ChronologicalGraphPath.of(state);
39+
40+
assertEquals(3, result.edges().size());
41+
assertSame(graph.e1, result.edges().get(0));
42+
assertSame(graph.e2, result.edges().get(1));
43+
assertSame(graph.e3, result.edges().get(2));
44+
}
45+
46+
@Test
47+
void bothDirectionsProduceIdenticalEdges() {
48+
var graph = createThreeEdgeGraph();
49+
50+
var forward = ChronologicalGraphPath.of(walkForward(graph));
51+
var backward = ChronologicalGraphPath.of(walkBackward(graph));
52+
53+
assertEquals(forward.edges(), backward.edges());
54+
assertEquals(forward.effectiveWalkDistance(), backward.effectiveWalkDistance(), 1e-9);
55+
}
56+
57+
@Test
58+
void zeroEdgesForInitialState() {
59+
var v1 = StreetModelForTest.intersectionVertex(
60+
"ChronologicalGraphPathTest_solo",
61+
59.910,
62+
10.750
63+
);
64+
var request = StreetSearchRequest.of()
65+
.withMode(StreetMode.WALK)
66+
.withArriveBy(false)
67+
.withStartTime(START_TIME)
68+
.build();
69+
var state = new State(v1, request);
70+
71+
var result = ChronologicalGraphPath.of(state);
72+
73+
assertEquals(0, result.edges().size());
74+
assertEquals(0.0, result.effectiveWalkDistance(), 1e-9);
75+
}
76+
77+
@Test
78+
void effectiveWalkDistanceIsPositive() {
79+
var graph = createThreeEdgeGraph();
80+
var state = walkForward(graph);
81+
82+
var result = ChronologicalGraphPath.of(state);
83+
84+
assertTrue(result.effectiveWalkDistance() > 0);
85+
}
86+
87+
private State walkForward(ThreeEdgeGraph g) {
88+
var request = StreetSearchRequest.of()
89+
.withMode(StreetMode.WALK)
90+
.withArriveBy(false)
91+
.withStartTime(START_TIME)
92+
.build();
93+
var s0 = new State(g.v1, request);
94+
var s1 = g.e1.traverse(s0)[0];
95+
var s2 = g.e2.traverse(s1)[0];
96+
return g.e3.traverse(s2)[0];
97+
}
98+
99+
private State walkBackward(ThreeEdgeGraph g) {
100+
var request = StreetSearchRequest.of()
101+
.withMode(StreetMode.WALK)
102+
.withArriveBy(true)
103+
.withStartTime(START_TIME)
104+
.build();
105+
var s0 = new State(g.v4, request);
106+
var s1 = g.e3.traverse(s0)[0];
107+
var s2 = g.e2.traverse(s1)[0];
108+
return g.e1.traverse(s2)[0];
109+
}
110+
111+
private static ThreeEdgeGraph createThreeEdgeGraph() {
112+
var v1 = StreetModelForTest.intersectionVertex("ChronologicalGraphPathTest_1", 59.910, 10.750);
113+
var v2 = StreetModelForTest.intersectionVertex("ChronologicalGraphPathTest_2", 59.911, 10.751);
114+
var v3 = StreetModelForTest.intersectionVertex("ChronologicalGraphPathTest_3", 59.912, 10.752);
115+
var v4 = StreetModelForTest.intersectionVertex("ChronologicalGraphPathTest_4", 59.913, 10.753);
116+
var e1 = StreetModelForTest.streetEdge(v1, v2);
117+
var e2 = StreetModelForTest.streetEdge(v2, v3);
118+
var e3 = StreetModelForTest.streetEdge(v3, v4);
119+
return new ThreeEdgeGraph(v1, v2, v3, v4, e1, e2, e3);
120+
}
121+
122+
private record ThreeEdgeGraph(
123+
StreetVertex v1,
124+
StreetVertex v2,
125+
StreetVertex v3,
126+
StreetVertex v4,
127+
StreetEdge e1,
128+
StreetEdge e2,
129+
StreetEdge e3
130+
) {}
131+
}

‎application/src/test/java/org/opentripplanner/routing/graphfinder/NearbyStopTest.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88

99
class NearbyStopTest {
1010

11-
private static TimetableRepositoryForTest MODEL = TimetableRepositoryForTest.of();
12-
13-
// TODO Add tests for all public methods in NearbyStop here
11+
private static final TimetableRepositoryForTest MODEL = TimetableRepositoryForTest.of();
1412

1513
@Test
1614
void testIsBetter() {

0 commit comments

Comments
 (0)