Skip to content

Commit

Permalink
Remove QueryGraph cache, fix #1768
Browse files Browse the repository at this point in the history
  • Loading branch information
@easbar
easbar committed Nov 20, 2019
1 parent 302882b commit 6da0e02
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 160 deletions.
6 changes: 2 additions & 4 deletions core/src/main/java/com/graphhopper/routing/AbstractBidirectionEdgeCHNoSOD.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ public AbstractBidirectionEdgeCHNoSOD(Graph graph, TurnWeighting weighting) {
// the inner explorers will run on the base- (or query/base-)graph edges only
Graph baseGraph = graph.getBaseGraph();
// we need extra edge explorers, because they get called inside a loop that already iterates over edges
// important: we have to use different filter ids for the edge explorers here than we use for the edge
// explorers in the superclass, otherwise this will not work with QueryGraph's edge explorer cache, see #1623.
innerInExplorer = baseGraph.createEdgeExplorer(DefaultEdgeFilter.inEdges(flagEncoder).setFilterId(1));
innerOutExplorer = baseGraph.createEdgeExplorer(DefaultEdgeFilter.outEdges(flagEncoder).setFilterId(1));
innerInExplorer = baseGraph.createEdgeExplorer(DefaultEdgeFilter.inEdges(flagEncoder));
innerOutExplorer = baseGraph.createEdgeExplorer(DefaultEdgeFilter.outEdges(flagEncoder));
}

@Override
Expand Down
43 changes: 5 additions & 38 deletions core/src/main/java/com/graphhopper/routing/querygraph/QueryGraph.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
import com.graphhopper.coll.GHIntObjectHashMap;
import com.graphhopper.routing.util.AllEdgesIterator;
import com.graphhopper.routing.util.EdgeFilter;
import com.graphhopper.storage.*;
import com.graphhopper.storage.ExtendedNodeAccess;
import com.graphhopper.storage.Graph;
import com.graphhopper.storage.NodeAccess;
import com.graphhopper.storage.TurnCostExtension;
import com.graphhopper.storage.index.QueryResult;
import com.graphhopper.util.*;
import com.graphhopper.util.shapes.BBox;
Expand Down Expand Up @@ -51,13 +54,11 @@ public class QueryGraph implements Graph {
// todo: why do we need this and do we still need it when we stop wrapping CHGraph with QueryGraph ?
private final QueryGraph baseGraph;
private final TurnCostExtension wrappedExtension;
private final Map<EdgeFilter, EdgeExplorer> cacheMap = new HashMap<>(4);
private final NodeAccess nodeAccess;
private final GraphModification graphModification;

// Use LinkedHashSet for predictable iteration order.
private final Set<VirtualEdgeIteratorState> unfavoredEdges = new LinkedHashSet<>(5);
private boolean useEdgeExplorerCache = false;
private final IntObjectMap<List<EdgeIteratorState>> virtualEdgesAtRealNodes;
private final List<List<EdgeIteratorState>> virtualEdgesAtVirtualNodes;

Expand Down Expand Up @@ -93,14 +94,7 @@ private QueryGraph(Graph graph, List<QueryResult> queryResults) {
virtualEdgesAtVirtualNodes = buildVirtualEdgesAtVirtualNodes();

// create very lightweight QueryGraph which uses variables from this QueryGraph (same virtual edges)
baseGraph = new QueryGraph(graph.getBaseGraph(), this) {
// override method to avoid stackoverflow
@Override
public QueryGraph setUseEdgeExplorerCache(boolean useEECache) {
baseGraph.useEdgeExplorerCache = useEECache;
return baseGraph;
}
};
baseGraph = new QueryGraph(graph.getBaseGraph(), this);
}

/**
Expand Down Expand Up @@ -138,20 +132,6 @@ public boolean isVirtualNode(int nodeId) {
return nodeId >= mainNodes;
}

/**
* This method is an experimental feature to reduce memory and CPU resources if there are many
* locations ("hundreds") for one QueryGraph. EdgeExplorer instances are cached based on the {@link EdgeFilter}
* passed into {@link #createEdgeExplorer(EdgeFilter)}. For equal (in the java sense) {@link EdgeFilter}s always
* the same {@link EdgeExplorer} will be returned when caching is enabled. Care has to be taken for example for
* custom or threaded algorithms, when using custom {@link EdgeFilter}s, or when the same edge explorer is used
* with different vehicles/encoders.
*/
public QueryGraph setUseEdgeExplorerCache(boolean useEECache) {
this.useEdgeExplorerCache = useEECache;
this.baseGraph.setUseEdgeExplorerCache(useEECache);
return this;
}

/**
* Set those edges at the virtual node (nodeId) to 'unfavored' that require at least a turn of
* 100° from favoredHeading.
Expand Down Expand Up @@ -307,19 +287,6 @@ private int getPosOfReverseEdge(int edgeId) {

@Override
public EdgeExplorer createEdgeExplorer(final EdgeFilter edgeFilter) {
if (useEdgeExplorerCache) {
EdgeExplorer cached = cacheMap.get(edgeFilter);
if (cached == null) {
cached = createUncachedEdgeExplorer(edgeFilter);
cacheMap.put(edgeFilter, cached);
}
return cached;
} else {
return createUncachedEdgeExplorer(edgeFilter);
}
}

private EdgeExplorer createUncachedEdgeExplorer(final EdgeFilter edgeFilter) {
// re-use these objects between setBaseNode calls to prevent GC
final EdgeExplorer mainExplorer = mainGraph.createEdgeExplorer(edgeFilter);
final VirtualEdgeIterator virtualEdgeIterator = new VirtualEdgeIterator(edgeFilter, null);
Expand Down
21 changes: 4 additions & 17 deletions core/src/main/java/com/graphhopper/routing/util/DefaultEdgeFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,22 @@
* @author Peter Karich
*/
public class DefaultEdgeFilter implements EdgeFilter {
private static int DEFAULT_FILTER_ID = 0;
private final boolean bwd;
private final boolean fwd;
private final BooleanEncodedValue accessEnc;
/**
* Used to be able to create non-equal filter instances with equal access encoder and fwd/bwd flags.
*/
private int filterId;

private DefaultEdgeFilter(BooleanEncodedValue accessEnc, boolean fwd, boolean bwd, int filterId) {
private DefaultEdgeFilter(BooleanEncodedValue accessEnc, boolean fwd, boolean bwd) {
this.accessEnc = accessEnc;
this.fwd = fwd;
this.bwd = bwd;
this.filterId = filterId;
}

public static DefaultEdgeFilter outEdges(BooleanEncodedValue accessEnc) {
return new DefaultEdgeFilter(accessEnc, true, false, DEFAULT_FILTER_ID);
return new DefaultEdgeFilter(accessEnc, true, false);
}

public static DefaultEdgeFilter inEdges(BooleanEncodedValue accessEnc) {
return new DefaultEdgeFilter(accessEnc, false, true, DEFAULT_FILTER_ID);
return new DefaultEdgeFilter(accessEnc, false, true);
}

/**
Expand All @@ -54,7 +48,7 @@ public static DefaultEdgeFilter inEdges(BooleanEncodedValue accessEnc) {
* regardless of their encoding use {@link EdgeFilter#ALL_EDGES} instead.
*/
public static DefaultEdgeFilter allEdges(BooleanEncodedValue accessEnc) {
return new DefaultEdgeFilter(accessEnc, true, true, DEFAULT_FILTER_ID);
return new DefaultEdgeFilter(accessEnc, true, true);
}

public static DefaultEdgeFilter outEdges(FlagEncoder flagEncoder) {
Expand All @@ -69,11 +63,6 @@ public static DefaultEdgeFilter allEdges(FlagEncoder flagEncoder) {
return DefaultEdgeFilter.allEdges(flagEncoder.getAccessEnc());
}

public DefaultEdgeFilter setFilterId(int filterId) {
this.filterId = filterId;
return this;
}

public BooleanEncodedValue getAccessEnc() {
return accessEnc;
}
Expand Down Expand Up @@ -104,7 +93,6 @@ public boolean equals(Object o) {

if (bwd != that.bwd) return false;
if (fwd != that.fwd) return false;
if (filterId != that.filterId) return false;
return accessEnc.equals(that.accessEnc);
}

Expand All @@ -113,7 +101,6 @@ public int hashCode() {
int result = (bwd ? 1 : 0);
result = 31 * result + (fwd ? 1 : 0);
result = 31 * result + accessEnc.hashCode();
result = 31 * result + filterId;
return result;
}
}
36 changes: 0 additions & 36 deletions core/src/test/java/com/graphhopper/routing/ch/CHTurnCostTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -894,42 +894,6 @@ public void test_issue_1593_simple() {
assertFalse("no path should be found, but found " + path.calcNodes(), path.isFound());
}

@Test
public void test_issue_1623_query_graph_cache() {
// 4-2->5-3
NodeAccess na = graph.getNodeAccess();
na.setNode(0, 49.408550, 9.701805);
na.setNode(1, 49.405988, 9.706111);
na.setNode(2, 49.400772, 9.706245);
na.setNode(3, 49.403167, 9.704774);
na.setNode(4, 49.405817, 9.704301);
na.setNode(5, 49.402488, 9.707799);
graph.edge(2, 5, 222.771000, false);
graph.edge(4, 2, 583.496000, true);
graph.edge(3, 5, 231.495000, true);
graph.freeze();

RoutingAlgorithmFactory pch = automaticPrepareCH();
LocationIndexTree index = new LocationIndexTree(graph, new RAMDirectory());
index.prepareIndex();
QueryResult qr1 = index.findClosest(49.400772, 9.706245, EdgeFilter.ALL_EDGES);
QueryResult qr2 = index.findClosest(49.403167, 9.704774, EdgeFilter.ALL_EDGES);
QueryGraph queryGraph = QueryGraph.lookup(chGraph, qr1, qr2);

// before fixing #1623 this test only worked for a disabled edge explorer cache
queryGraph.setUseEdgeExplorerCache(true);

assertEquals(2, qr1.getClosestNode());
assertEquals(3, qr2.getClosestNode());
RoutingAlgorithm chAlgo = pch.createAlgo(queryGraph, AlgorithmOptions.start()
.traversalMode(TraversalMode.EDGE_BASED)
.build());
Path path = chAlgo.calcPath(2, 3);
assertTrue("no path found", path.isFound());
assertEquals(IntArrayList.from(2, 5, 3), path.calcNodes());
assertEquals(454.266, path.getDistance(), 1.e-1);
}

@Test
public void testRouteViaVirtualNode() {
// 3
Expand Down
69 changes: 4 additions & 65 deletions core/src/test/java/com/graphhopper/routing/querygraph/QueryGraphTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@
import org.junit.Before;
import org.junit.Test;

import java.util.*;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;

import static com.graphhopper.storage.index.QueryResult.Position.*;
import static com.graphhopper.util.GHUtility.updateDistancesFor;
Expand Down Expand Up @@ -654,70 +657,6 @@ public void testInternalAPIOriginalEdgeKey() {
((VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(iter.getEdge(), 1)).getOriginalEdgeKey());
}

@Test
public void useEECache() {
initGraph(g);
EdgeExplorer explorer = g.createEdgeExplorer();
EdgeIterator iter = explorer.setBaseNode(1);
assertTrue(iter.next());
QueryResult res = createLocationResult(2, 1.5, iter, 1, PILLAR);

QueryGraph queryGraph = lookup(res);
queryGraph.setUseEdgeExplorerCache(true);

EdgeExplorer edgeExplorer = queryGraph.createEdgeExplorer();
// using cache means same reference
assertSame(edgeExplorer, queryGraph.createEdgeExplorer());
}

@Test
public void useEECache_nestedLoop() {
//
// 0->3
// |\
// 1 2->6
// |\
// 4 5
g.edge(0, 1, 10, false);
g.edge(0, 2, 10, false);
g.edge(0, 3, 10, false);
g.edge(2, 4, 10, false);
g.edge(2, 5, 10, false);
g.edge(2, 6, 10, false);

EdgeExplorer explorer = g.createEdgeExplorer();
EdgeIterator iter = explorer.setBaseNode(0);
assertTrue(iter.next());
QueryResult res = createLocationResult(0, 0, iter, 1, PILLAR);

QueryGraph queryGraph = lookup(Collections.singletonList(res));
queryGraph.setUseEdgeExplorerCache(true);

EdgeExplorer outerEdgeExplorer = queryGraph.createEdgeExplorer(DefaultEdgeFilter.outEdges(carEncoder));
EdgeExplorer innerEdgeExplorer = queryGraph.createEdgeExplorer(DefaultEdgeFilter.outEdges(carEncoder));

// without using filter id for DefaultEdgeFilter the filters are equal and using the explorers in a nested
// loop would fail
assertSame(outerEdgeExplorer, innerEdgeExplorer);

// using a different filter id for the second filter we get different explorers
outerEdgeExplorer = queryGraph.createEdgeExplorer(DefaultEdgeFilter.outEdges(carEncoder));
innerEdgeExplorer = queryGraph.createEdgeExplorer(DefaultEdgeFilter.outEdges(carEncoder).setFilterId(1));
assertNotSame(outerEdgeExplorer, innerEdgeExplorer);

// now we can safely use the two explorers in a nested loop
Set<String> edges = new HashSet<>();
EdgeIterator outerIter = outerEdgeExplorer.setBaseNode(0);
while (outerIter.next()) {
edges.add("o" + outerIter.getBaseNode() + "-" + outerIter.getAdjNode());
EdgeIterator innerIter = innerEdgeExplorer.setBaseNode(outerIter.getAdjNode());
while (innerIter.next()) {
edges.add("i" + innerIter.getBaseNode() + "-" + innerIter.getAdjNode());
}
}
assertEquals(new HashSet<>(Arrays.asList("o0-1", "o0-2", "o0-3", "i2-4", "i2-5", "i2-6")), edges);
}

@Test
public void testWayGeometry_edge() {
// drawn as horizontal linear graph for simplicity
Expand Down

0 comments on commit 6da0e02

Please sign in to comment.