Skip to content

Commit e14da30

Browse files
michazmichaz
authored
Work around slow PriorityQueue.remove (graphhopper#2092)
* Work around slow PriorityQueue.remove * Finish comment * Remove unused code * Typo * Test U-turns with time penalty
1 parent 600c795 commit e14da30

File tree

2 files changed

+108
-9
lines changed

2 files changed

+108
-9
lines changed

‎isochrone/src/main/java/com/graphhopper/isochrone/algorithm/ShortestPathTree.java

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@
4545
* is different from the function for search. This implementation uses a second queue to keep track of
4646
* the termination criterion.
4747
*
48+
* IMPLEMENTATION NOTE:
49+
* util.PriorityQueue doesn't support efficient removes. We work around this by giving the labels
50+
* a deleted flag, not remove()ing them, and popping deleted elements off both queues.
51+
* Note to self/others: If you think this optimization is not needed, please test it with a scenario
52+
* where updates actually occur a lot, such as using finite, non-zero u-turn costs.
53+
*
4854
* @author Peter Karich
4955
* @author Michael Zilske
5056
*/
@@ -63,6 +69,7 @@ public static class IsoLabel {
6369
this.parent = parent;
6470
}
6571

72+
public boolean deleted = false;
6673
public int node;
6774
public int edge;
6875
public double weight;
@@ -138,10 +145,12 @@ public void search(int from, final Consumer<IsoLabel> consumer) {
138145
EdgeFilter filter = reverseFlow ? inEdgeFilter : outEdgeFilter;
139146
while (!finished()) {
140147
currentLabel = queueByWeighting.poll();
141-
queueByZ.remove(currentLabel);
148+
if (currentLabel.deleted)
149+
continue;
142150
if (getExploreValue(currentLabel) <= limit) {
143151
consumer.accept(currentLabel);
144152
}
153+
currentLabel.deleted = true;
145154
visitedNodes++;
146155

147156
EdgeIterator iter = edgeExplorer.setBaseNode(currentLabel.node);
@@ -167,13 +176,9 @@ public void search(int from, final Consumer<IsoLabel> consumer) {
167176
queueByWeighting.add(nextLabel);
168177
queueByZ.add(nextLabel);
169178
} else if (nextLabel.weight > nextWeight) {
170-
queueByWeighting.remove(nextLabel);
171-
queueByZ.remove(nextLabel);
172-
nextLabel.edge = iter.getEdge();
173-
nextLabel.weight = nextWeight;
174-
nextLabel.distance = nextDistance;
175-
nextLabel.time = nextTime;
176-
nextLabel.parent = currentLabel;
179+
nextLabel.deleted = true;
180+
nextLabel = new IsoLabel(iter.getAdjNode(), iter.getEdge(), nextWeight, nextTime, nextDistance, currentLabel);
181+
fromMap.put(nextTraversalId, nextLabel);
177182
queueByWeighting.add(nextLabel);
178183
queueByZ.add(nextLabel);
179184
}
@@ -226,7 +231,11 @@ private double getExploreValue(IsoLabel label) {
226231

227232
@Override
228233
protected boolean finished() {
229-
return queueByZ.isEmpty() || getExploreValue(queueByZ.peek()) >= limit;
234+
while (queueByZ.peek() != null && queueByZ.peek().deleted)
235+
queueByZ.poll();
236+
if (queueByZ.peek() == null)
237+
return true;
238+
return getExploreValue(queueByZ.peek()) >= limit;
230239
}
231240

232241
@Override

‎isochrone/src/test/java/com/graphhopper/isochrone/algorithm/ShortestPathTreeTest.java

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,25 @@
2828
*/
2929
public class ShortestPathTreeTest {
3030

31+
private static class TimeBasedUTurnCost implements TurnCostProvider {
32+
33+
private final int turnMillis;
34+
35+
public TimeBasedUTurnCost(int turnMillis) {
36+
this.turnMillis = turnMillis;
37+
}
38+
39+
@Override
40+
public double calcTurnWeight(int inEdge, int viaNode, int outEdge) {
41+
return calcTurnMillis(inEdge, viaNode, outEdge) / 1000.0;
42+
}
43+
44+
@Override
45+
public long calcTurnMillis(int inEdge, int viaNode, int outEdge) {
46+
return inEdge == outEdge ? turnMillis : 0;
47+
}
48+
}
49+
3150
public static final TurnCostProvider FORBIDDEN_UTURNS = new TurnCostProvider() {
3251
@Override
3352
public double calcTurnWeight(int inEdge, int viaNode, int outEdge) {
@@ -45,6 +64,7 @@ public long calcTurnMillis(int inEdge, int viaNode, int outEdge) {
4564
private final FlagEncoder carEncoder = encodingManager.getEncoder("car");
4665
private GraphHopperStorage graph;
4766

67+
4868
@BeforeEach
4969
public void setUp() {
5070
graph = new GraphHopperStorage(new RAMDirectory(), encodingManager, false);
@@ -198,6 +218,76 @@ public void testEdgeBasedWithForbiddenUTurns() {
198218
);
199219
}
200220

221+
@Test
222+
public void testEdgeBasedWithFinitePositiveUTurnCost() {
223+
TimeBasedUTurnCost turnCost = new TimeBasedUTurnCost(80000);
224+
FastestWeighting fastestWeighting = new FastestWeighting(carEncoder, new PMap(), turnCost);
225+
List<ShortestPathTree.IsoLabel> result = new ArrayList<>();
226+
ShortestPathTree instance = new ShortestPathTree(graph, fastestWeighting, false, TraversalMode.EDGE_BASED);
227+
instance.setTimeLimit(Double.MAX_VALUE);
228+
instance.search(0, result::add);
229+
// Just like with forbidden U-turns, but last thing is I can get out of the dead-end
230+
assertEquals(countDirectedEdges(graph) + 1, result.size());
231+
assertAll(
232+
() -> assertEquals(0, result.get(0).time),
233+
() -> assertEquals(9000, result.get(1).time),
234+
() -> assertEquals(18000, result.get(2).time),
235+
() -> assertEquals(25200, result.get(3).time),
236+
() -> assertEquals(27000, result.get(4).time),
237+
() -> assertEquals(34200, result.get(5).time),
238+
() -> assertEquals(36000, result.get(6).time),
239+
() -> assertEquals(50400, result.get(7).time),
240+
() -> assertEquals(50400, result.get(8).time),
241+
() -> assertEquals(54000, result.get(9).time),
242+
() -> assertEquals(55800, result.get(10).time),
243+
() -> assertEquals(60300, result.get(11).time),
244+
() -> assertEquals(61200, result.get(12).time),
245+
() -> assertEquals(61200, result.get(13).time),
246+
() -> assertEquals(61200, result.get(14).time),
247+
() -> assertEquals(72000, result.get(15).time),
248+
() -> assertEquals(81000, result.get(16).time),
249+
() -> assertEquals(90000, result.get(17).time),
250+
() -> assertEquals(97200, result.get(18).time),
251+
() -> assertEquals(126000, result.get(19).time),
252+
() -> assertEquals(144800, result.get(20).time)
253+
);
254+
}
255+
256+
@Test
257+
public void testEdgeBasedWithSmallerUTurnCost() {
258+
TimeBasedUTurnCost turnCost = new TimeBasedUTurnCost(20000);
259+
FastestWeighting fastestWeighting = new FastestWeighting(carEncoder, new PMap(), turnCost);
260+
List<ShortestPathTree.IsoLabel> result = new ArrayList<>();
261+
ShortestPathTree instance = new ShortestPathTree(graph, fastestWeighting, false, TraversalMode.EDGE_BASED);
262+
instance.setTimeLimit(Double.MAX_VALUE);
263+
instance.search(0, result::add);
264+
// Something in between
265+
assertEquals(countDirectedEdges(graph) + 1, result.size());
266+
assertAll(
267+
() -> assertEquals(0, result.get(0).time),
268+
() -> assertEquals(9000, result.get(1).time),
269+
() -> assertEquals(18000, result.get(2).time),
270+
() -> assertEquals(25200, result.get(3).time),
271+
() -> assertEquals(27000, result.get(4).time),
272+
() -> assertEquals(34200, result.get(5).time),
273+
() -> assertEquals(36000, result.get(6).time),
274+
() -> assertEquals(50400, result.get(7).time),
275+
() -> assertEquals(50400, result.get(8).time),
276+
() -> assertEquals(54000, result.get(9).time),
277+
() -> assertEquals(55800, result.get(10).time),
278+
() -> assertEquals(56000, result.get(11).time),
279+
() -> assertEquals(60300, result.get(12).time),
280+
() -> assertEquals(61200, result.get(13).time),
281+
() -> assertEquals(61200, result.get(14).time),
282+
() -> assertEquals(61200, result.get(15).time),
283+
() -> assertEquals(72000, result.get(16).time),
284+
() -> assertEquals(81000, result.get(17).time),
285+
() -> assertEquals(84800, result.get(18).time),
286+
() -> assertEquals(97200, result.get(19).time),
287+
() -> assertEquals(126000, result.get(20).time)
288+
);
289+
}
290+
201291
@Test
202292
public void testSearchByDistance() {
203293
List<ShortestPathTree.IsoLabel> result = new ArrayList<>();

0 commit comments

Comments
 (0)