Optimise R-tree queries in the case of map matching #6881

SiarheiFedartsou · 2024-05-12T09:56:28Z

Issue

Since in map matching we don't need to find the N nearest projections, but simply all projections within a certain radius, instead of using a priority queue-based algorithm, we can simply find all edges intersecting the bounding box defined by this radius and then post-process the results to remove edges that are not within the required radius. According to our current simple map matching benchmark with a default search radius of 5m (which is effectively 15m in practice), this makes map matching up to 50% faster! However, with an increase in radius, this optimization becomes less noticeable since a larger radius increases the number of candidates, impacting the number of routes we need to compute in the map matching algorithm. But I have some ideas on how to optimize this as well in my future PRs.

Actual improvement can be seen below in the match_ch and match_mld benchmarks(the result in master corresponds to default/5m in this PR).

Tasklist

CHANGELOG.md entry (How to write a changelog entry)
update relevant Wiki pages
add tests (see testing documentation)
review
adjust for comments
cherry pick to release branch

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

Benchmark Results

Benchmark	Base	PR
alias	aliased u32: 1101.99 plain u32: 1093.37 aliased double: 954.491 plain double: 966.534	aliased u32: 1098.31 plain u32: 1101.72 aliased double: 966.005 plain double: 964.487
json-render	String: 8.62683ms Stringstream: 11.6428ms Vector: 7.58827ms	String: 8.60096ms Stringstream: 11.7067ms Vector: 7.58147ms
match_ch	Default radius: 8.23085ms/req at 82 coordinate 0.100376ms/coordinate Radius 5m: 8.19639ms/req at 82 coordinate 0.099956ms/coordinate Radius 10m: 19.3981ms/req at 82 coordinate 0.236562ms/coordinate Radius 15m: 41.5492ms/req at 82 coordinate 0.506697ms/coordinate Radius 30m: 320.866ms/req at 82 coordinate 3.913ms/coordinate	Default radius: 4.50758ms/req at 82 coordinate 0.0549705ms/coordinate Radius 5m: 4.48594ms/req at 82 coordinate 0.0547065ms/coordinate Radius 10m: 15.3305ms/req at 82 coordinate 0.186957ms/coordinate Radius 15m: 37.3413ms/req at 82 coordinate 0.455381ms/coordinate Radius 30m: 318.02ms/req at 82 coordinate 3.87829ms/coordinate
match_mld	Default radius: 7.43256ms/req at 82 coordinate 0.090641ms/coordinate Radius 5m: 7.12935ms/req at 82 coordinate 0.0869433ms/coordinate Radius 10m: 16.5298ms/req at 82 coordinate 0.201583ms/coordinate Radius 15m: 36.6781ms/req at 82 coordinate 0.447294ms/coordinate Radius 30m: 361.997ms/req at 82 coordinate 4.4146ms/coordinate	Default radius: 3.39512ms/req at 82 coordinate 0.0414039ms/coordinate Radius 5m: 3.37432ms/req at 82 coordinate 0.0411503ms/coordinate Radius 10m: 12.3558ms/req at 82 coordinate 0.15068ms/coordinate Radius 15m: 32.0192ms/req at 82 coordinate 0.390477ms/coordinate Radius 30m: 351.998ms/req at 82 coordinate 4.29266ms/coordinate
packedvector	random write: std::vector 9792.03 ms util::packed_vector 82197.7 ms slowdown: 8.39435 random read: std::vector 8512.14 ms util::packed_vector 33173.5 ms slowdown: 3.8972	random write: std::vector 9835.57 ms util::packed_vector 74173.3 ms slowdown: 7.54133 random read: std::vector 8557.96 ms util::packed_vector 29830.8 ms slowdown: 3.48573
rtree	1 result: 208.575ms -> 0.0208575 ms/query 10 results: 243.318ms -> 0.0243318 ms/query	1 result: 220.757ms -> 0.0220757 ms/query 10 results: 242.402ms -> 0.0242402 ms/query

…mum number of results

SiarheiFedartsou · 2024-05-14T18:13:24Z

include/util/static_rtree.hpp

@@ -631,6 +611,73 @@ class StaticRTree
 }

 private:
+ template <typename Callback>


I just added callback parameter instead of returning vector and made this implementation private(for public we have another SearchInBox now which calls this method)

SiarheiFedartsou · 2024-05-14T18:22:31Z

include/util/rectangle.hpp

@@ -168,6 +167,18 @@ struct RectangleInt2D
 min_lat != FixedLatitude{std::numeric_limits<std::int32_t>::max()} &&
 max_lat != FixedLatitude{std::numeric_limits<std::int32_t>::min()};
 }
+
+ static RectangleInt2D ExpandMeters(const Coordinate &coordinate, const double meters)


It is basically re-implemented function from Valhalla: https://github.com/valhalla/valhalla/blob/c14d3ac8e4745a17e5e99a50dac32c838fc3475b/valhalla/midgard/util.h#L107

danpat · 2024-05-14T18:36:57Z

include/engine/geospatial_query.hpp

+ auto phantom_nodes = MakePhantomNodes(input_coordinate, results);
+ // there is no guarantee that `SearchInRange` will return results sorted by distance,
+ // but we may rely on it somewhere
+ std::sort(phantom_nodes.begin(),


I'd be tempted to leave this out, and just see what happens. We already have some "construction order dependent" results (when there are two paths to a destination of equal weight, the one picked depends on the order the graph was built, not some other stable heuristic), so stability is already not guaranteed.

That'd be an interesting insight, indeed.

Well, actually after I added it I realised that this method is only used in map matching. For map matching order shouldn't be important I believe. So I think it will be safe to remove it 👍

mjjbell · 2024-05-14T19:11:24Z

include/engine/geospatial_query.hpp

@@ -47,6 +47,42 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
 return rtree.SearchInBox(bbox);
 }

+ std::vector<PhantomNodeWithDistance>
+ NearestPhantomNodes(const util::Coordinate input_coordinate,


I know it's an internal API, but might be worth changing NearestPhantomNodes with optional max_results parameter to be a required parameter, to clarify the distinction between the two functions.

Out of interest, did you evaluate this approach for when max_results is set also?

I know it's an internal API, but might be worth changing NearestPhantomNodes with optional max_results parameter to be a required parameter, to clarify the distinction between the two functions.

Great idea, updated 👍

Out of interest, did you evaluate this approach for when max_results is set also?

Nope, but I thought about it: we probably could make another optimisation when both max_results and max_distance are set. Or may be you see how it can be applied when only max_results is set?

No particular insights from me, was just interested 🙂

DennisOSRM · 2024-05-15T04:10:42Z

include/engine/geospatial_query.hpp

+ CheckSegmentDistance(input_coordinate, segment, max_distance);
+ if (invalidDistance)
+ {
+ return std::pair<bool, bool>{false, false};


Probably only a matter of style, but how about doing make_pair(false, false)?

DennisOSRM · 2024-05-15T04:11:26Z

include/engine/geospatial_query.hpp

+ auto phantom_nodes = MakePhantomNodes(input_coordinate, results);
+ // there is no guarantee that `SearchInRange` will return results sorted by distance,
+ // but we may rely on it somewhere
+ std::sort(phantom_nodes.begin(),


That'd be an interesting insight, indeed.

DennisOSRM

nice optimization!

SiarheiFedartsou added 9 commits May 14, 2024 17:15

Optimise R-tree queries in the case if there is no need to limit maxi…

1d17ff2

…mum number of results

Optimise R-tree queries in the case if there is no need to limit maxi…

717407e

…mum number of results

Optimise R-tree queries in the case if there is no need to limit maxi…

36c074c

…mum number of results

Optimise R-tree queries in the case if there is no need to limit maxi…

8ac3936

…mum number of results

Optimise R-tree queries in the case if there is no need to limit maxi…

99dceb1

…mum number of results

Improve benchmark

9f80b43

Optimise R-tree queries in the case if there is no need to limit maxi…

fc4fa10

…mum number of results

Optimise R-tree queries in the case if there is no need to limit maxi…

85d95af

…mum number of results

Add <optional>

1fdbb5d

SiarheiFedartsou force-pushed the sf-optimise-geospatial-query branch from 1c14ee7 to 1fdbb5d Compare May 14, 2024 15:15

remove #if 0

5443794

SiarheiFedartsou commented May 14, 2024

View reviewed changes

SiarheiFedartsou changed the title ~~Optimise R-tree queries in the case if there is no need to limit maximum number of results~~ Optimise R-tree queries in the case of map matching May 14, 2024

add changelog

5349858

SiarheiFedartsou commented May 14, 2024

View reviewed changes

SiarheiFedartsou requested review from mjjbell and DennisOSRM May 14, 2024 18:23

SiarheiFedartsou marked this pull request as ready for review May 14, 2024 18:23

danpat reviewed May 14, 2024

View reviewed changes

mjjbell reviewed May 14, 2024

View reviewed changes

DennisOSRM reviewed May 15, 2024

View reviewed changes

SiarheiFedartsou added 4 commits May 15, 2024 17:41

Remove sorting

0a0ebc1

Make max_results non-optional

2d04112

Use std::make_pair

7bdf921

Fix tests

5d10255

SiarheiFedartsou requested review from mjjbell, danpat and DennisOSRM May 15, 2024 18:52

SiarheiFedartsou mentioned this pull request May 19, 2024

Improve map matching benchmark #6885

Merged

6 tasks

DennisOSRM approved these changes May 19, 2024

View reviewed changes

Merge branch 'master' into sf-optimise-geospatial-query

38e95b4

SiarheiFedartsou merged commit d259848 into master May 20, 2024
21 checks passed

SiarheiFedartsou deleted the sf-optimise-geospatial-query branch May 20, 2024 10:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimise R-tree queries in the case of map matching #6881

Optimise R-tree queries in the case of map matching #6881

SiarheiFedartsou commented May 12, 2024 •

edited by github-actions bot

SiarheiFedartsou May 14, 2024 •

edited

SiarheiFedartsou May 14, 2024

danpat May 14, 2024

DennisOSRM May 15, 2024

SiarheiFedartsou May 15, 2024

mjjbell May 14, 2024

mjjbell May 14, 2024

SiarheiFedartsou May 15, 2024

mjjbell May 24, 2024

DennisOSRM May 15, 2024

DennisOSRM May 15, 2024

DennisOSRM left a comment

Optimise R-tree queries in the case of map matching #6881

Optimise R-tree queries in the case of map matching #6881

Conversation

SiarheiFedartsou commented May 12, 2024 • edited by github-actions bot

Issue

Tasklist

Requirements / Relations

Benchmark Results

SiarheiFedartsou May 14, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DennisOSRM left a comment

Choose a reason for hiding this comment

SiarheiFedartsou commented May 12, 2024 •

edited by github-actions bot

SiarheiFedartsou May 14, 2024 •

edited