From c6be1a9c10a0ca9f75d12dd23fc0825d3167d496 Mon Sep 17 00:00:00 2001 From: MadWasp79 Date: Fri, 4 Oct 2019 16:22:48 +0300 Subject: [PATCH 1/3] fix for slow excluded id check --- .../net/osmand/router/RoutingContext.java | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java b/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java index 3ede1df697..376ca8bcf1 100644 --- a/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java +++ b/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java @@ -6,10 +6,13 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.Map.Entry; import org.apache.commons.logging.Log; @@ -73,6 +76,7 @@ public class RoutingContext { // 2. Routing memory cache (big objects) TLongObjectHashMap> indexedSubregions = new TLongObjectHashMap>(); + TLongObjectHashMap> excludedIdsByTile = new TLongObjectHashMap>(); // Needs to be a sorted array list . Another option to use hashmap but it will be more memory expensive List subregionTiles = new ArrayList(); @@ -107,8 +111,7 @@ public class RoutingContext { // old planner public FinalRouteSegment finalRouteSegment; - - + RoutingContext(RoutingContext cp) { this.config = cp.config; this.map.putAll(cp.map); @@ -230,6 +233,7 @@ public class RoutingContext { } subregionTiles.clear(); indexedSubregions.clear(); + excludedIdsByTile.clear(); } private int searchSubregionTile(RouteSubregion subregion){ @@ -272,16 +276,19 @@ public class RoutingContext { public RouteSegment loadRouteSegment(int x31, int y31, int memoryLimit) { long tileId = getRoutingTile(x31, y31, memoryLimit, OPTION_SMART_LOAD); TLongObjectHashMap excludeDuplications = new TLongObjectHashMap(); - TLongSet excludeIds = new TLongHashSet(); RouteSegment original = null; List subregions = indexedSubregions.get(tileId); if (subregions != null) { - for (RoutingSubregionTile rs : subregions) { - // TODO - original = rs.loadRouteSegment(x31, y31, this, excludeDuplications, excludeIds, - original); - if(rs.excludedIds != null) { - excludeIds.addAll(rs.excludedIds); + for (int j = 0; j < subregions.size(); j++) { + original = subregions.get(j).loadRouteSegment(x31, y31, this, excludeDuplications, + excludedIdsByTile.get(tileId), original, j); + if(j < subregions.size()- 1) { + if (excludedIdsByTile.get(tileId) == null) { + excludedIdsByTile.put(tileId, new ArrayList<>()); + } + if (excludedIdsByTile.get(tileId).size() < j) { + excludedIdsByTile.get(tileId).add(subregions.get(j).excludedIds); + } } } } @@ -654,7 +661,8 @@ public class RoutingContext { } private RouteSegment loadRouteSegment(int x31, int y31, RoutingContext ctx, - TLongObjectHashMap excludeDuplications, TLongSet excludeIds, RouteSegment original) { + TLongObjectHashMap excludeDuplications, List excludedIdsInTile, + RouteSegment original, int subIndex) { access++; if (routes != null) { long l = (((long) x31) << 31) + (long) y31; @@ -662,8 +670,8 @@ public class RoutingContext { while (segment != null) { RouteDataObject ro = segment.road; RouteDataObject toCmp = excludeDuplications.get(calcRouteId(ro, segment.getSegmentStart())); - if ((toCmp == null || toCmp.getPointsLength() < ro.getPointsLength()) - && !excludeIds.contains(ro.id)) { + if ((toCmp == null || toCmp.getPointsLength() < ro.getPointsLength()) + && !isExcluded(ro.id, excludedIdsInTile, subIndex)) { excludeDuplications.put(calcRouteId(ro, segment.getSegmentStart()), ro); RouteSegment s = new RouteSegment(ro, segment.getSegmentStart()); s.next = original; @@ -676,6 +684,18 @@ public class RoutingContext { } return original; } + + private boolean isExcluded(long id, List excludedIdsBySub, int subIndex) { + if (subIndex == 0) { + return false; + } + for (int i = 0; i < subIndex-1; i++) { + if (excludedIdsBySub.get(i) != null && excludedIdsBySub.get(i).contains(id)) { + return true; + } + } + return false; + } public boolean isLoaded() { return isLoaded > 0; From 2ee126419fd671f9ed33d77d7023b4b44150d631 Mon Sep 17 00:00:00 2001 From: MadWasp79 Date: Mon, 7 Oct 2019 13:23:30 +0300 Subject: [PATCH 2/3] code clean-up --- .../net/osmand/router/RoutingContext.java | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java b/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java index 376ca8bcf1..89bfca8967 100644 --- a/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java +++ b/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java @@ -36,6 +36,7 @@ import net.osmand.router.BinaryRoutePlanner.FinalRouteSegment; import net.osmand.router.BinaryRoutePlanner.RouteSegment; import net.osmand.router.BinaryRoutePlanner.RouteSegmentVisitor; import net.osmand.router.RoutePlannerFrontEnd.RouteCalculationMode; +import net.osmand.util.Algorithms; public class RoutingContext { @@ -76,7 +77,7 @@ public class RoutingContext { // 2. Routing memory cache (big objects) TLongObjectHashMap> indexedSubregions = new TLongObjectHashMap>(); - TLongObjectHashMap> excludedIdsByTile = new TLongObjectHashMap>(); + TLongObjectHashMap excludedIdsByTile = new TLongObjectHashMap(); // Needs to be a sorted array list . Another option to use hashmap but it will be more memory expensive List subregionTiles = new ArrayList(); @@ -282,14 +283,6 @@ public class RoutingContext { for (int j = 0; j < subregions.size(); j++) { original = subregions.get(j).loadRouteSegment(x31, y31, this, excludeDuplications, excludedIdsByTile.get(tileId), original, j); - if(j < subregions.size()- 1) { - if (excludedIdsByTile.get(tileId) == null) { - excludedIdsByTile.put(tileId, new ArrayList<>()); - } - if (excludedIdsByTile.get(tileId).size() < j) { - excludedIdsByTile.get(tileId).add(subregions.get(j).excludedIds); - } - } } } return original; @@ -502,6 +495,7 @@ public class RoutingContext { List subregions = indexedSubregions.get(tileId); if (subregions != null) { boolean load = false; + TLongHashSet excludedIdsForTile = new TLongHashSet(); for (RoutingSubregionTile ts : subregions) { if (!ts.isLoaded()) { load = true; @@ -517,7 +511,11 @@ public class RoutingContext { excludeIds.addAll(ts.excludedIds); } } + if (excludeIds != null) { + excludedIdsForTile.addAll(excludeIds); + } } + excludedIdsByTile.put(tileId, excludedIdsForTile); } } } @@ -624,6 +622,7 @@ public class RoutingContext { private int isLoaded = 0; private TLongObjectMap routes = null; private TLongHashSet excludedIds = null; + private TLongHashSet excludedIdsForTile = null; public RoutingSubregionTile(RouteSubregion subregion) { this.subregion = subregion; @@ -661,8 +660,8 @@ public class RoutingContext { } private RouteSegment loadRouteSegment(int x31, int y31, RoutingContext ctx, - TLongObjectHashMap excludeDuplications, List excludedIdsInTile, - RouteSegment original, int subIndex) { + TLongObjectHashMap excludeDuplications, + TLongHashSet excludedIdsForTile, RouteSegment original, int subIndex) { access++; if (routes != null) { long l = (((long) x31) << 31) + (long) y31; @@ -670,8 +669,8 @@ public class RoutingContext { while (segment != null) { RouteDataObject ro = segment.road; RouteDataObject toCmp = excludeDuplications.get(calcRouteId(ro, segment.getSegmentStart())); - if ((toCmp == null || toCmp.getPointsLength() < ro.getPointsLength()) - && !isExcluded(ro.id, excludedIdsInTile, subIndex)) { + if (!isExcluded(ro.id, excludedIdsForTile) + && (toCmp == null || toCmp.getPointsLength() < ro.getPointsLength())) { excludeDuplications.put(calcRouteId(ro, segment.getSegmentStart()), ro); RouteSegment s = new RouteSegment(ro, segment.getSegmentStart()); s.next = original; @@ -685,14 +684,9 @@ public class RoutingContext { return original; } - private boolean isExcluded(long id, List excludedIdsBySub, int subIndex) { - if (subIndex == 0) { - return false; - } - for (int i = 0; i < subIndex-1; i++) { - if (excludedIdsBySub.get(i) != null && excludedIdsBySub.get(i).contains(id)) { - return true; - } + private boolean isExcluded(long id, TLongHashSet excludedIdsForTile) { + if (excludedIdsForTile != null && excludedIdsForTile.contains(id)) { + return true; } return false; } From 534f03d558c42d613fd26ee4eff49f642e1a99fb Mon Sep 17 00:00:00 2001 From: MadWasp79 Date: Mon, 7 Oct 2019 17:55:40 +0300 Subject: [PATCH 3/3] clean up --- .../net/osmand/router/RoutingContext.java | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java b/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java index 89bfca8967..5f548408e6 100644 --- a/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java +++ b/OsmAnd-java/src/main/java/net/osmand/router/RoutingContext.java @@ -77,7 +77,6 @@ public class RoutingContext { // 2. Routing memory cache (big objects) TLongObjectHashMap> indexedSubregions = new TLongObjectHashMap>(); - TLongObjectHashMap excludedIdsByTile = new TLongObjectHashMap(); // Needs to be a sorted array list . Another option to use hashmap but it will be more memory expensive List subregionTiles = new ArrayList(); @@ -234,7 +233,6 @@ public class RoutingContext { } subregionTiles.clear(); indexedSubregions.clear(); - excludedIdsByTile.clear(); } private int searchSubregionTile(RouteSubregion subregion){ @@ -282,7 +280,7 @@ public class RoutingContext { if (subregions != null) { for (int j = 0; j < subregions.size(); j++) { original = subregions.get(j).loadRouteSegment(x31, y31, this, excludeDuplications, - excludedIdsByTile.get(tileId), original, j); + original, subregions, j); } } return original; @@ -495,7 +493,6 @@ public class RoutingContext { List subregions = indexedSubregions.get(tileId); if (subregions != null) { boolean load = false; - TLongHashSet excludedIdsForTile = new TLongHashSet(); for (RoutingSubregionTile ts : subregions) { if (!ts.isLoaded()) { load = true; @@ -511,11 +508,7 @@ public class RoutingContext { excludeIds.addAll(ts.excludedIds); } } - if (excludeIds != null) { - excludedIdsForTile.addAll(excludeIds); - } } - excludedIdsByTile.put(tileId, excludedIdsForTile); } } } @@ -622,7 +615,6 @@ public class RoutingContext { private int isLoaded = 0; private TLongObjectMap routes = null; private TLongHashSet excludedIds = null; - private TLongHashSet excludedIdsForTile = null; public RoutingSubregionTile(RouteSubregion subregion) { this.subregion = subregion; @@ -660,8 +652,7 @@ public class RoutingContext { } private RouteSegment loadRouteSegment(int x31, int y31, RoutingContext ctx, - TLongObjectHashMap excludeDuplications, - TLongHashSet excludedIdsForTile, RouteSegment original, int subIndex) { + TLongObjectHashMap excludeDuplications, RouteSegment original, List subregions, int subregionIndex) { access++; if (routes != null) { long l = (((long) x31) << 31) + (long) y31; @@ -669,7 +660,7 @@ public class RoutingContext { while (segment != null) { RouteDataObject ro = segment.road; RouteDataObject toCmp = excludeDuplications.get(calcRouteId(ro, segment.getSegmentStart())); - if (!isExcluded(ro.id, excludedIdsForTile) + if (!isExcluded(ro.id, subregions, subregionIndex) && (toCmp == null || toCmp.getPointsLength() < ro.getPointsLength())) { excludeDuplications.put(calcRouteId(ro, segment.getSegmentStart()), ro); RouteSegment s = new RouteSegment(ro, segment.getSegmentStart()); @@ -684,9 +675,11 @@ public class RoutingContext { return original; } - private boolean isExcluded(long id, TLongHashSet excludedIdsForTile) { - if (excludedIdsForTile != null && excludedIdsForTile.contains(id)) { - return true; + private static boolean isExcluded(long id, List subregions, int subregionIndex) { + for (int i = 0; i < subregionIndex; i++ ) { + if (subregions.get(i).excludedIds != null && subregions.get(i).excludedIds.contains(id)) { + return true; + } } return false; }