From 600151d4c698f9c31917a56ec36451424c4f8c15 Mon Sep 17 00:00:00 2001 From: dnesbitt61 Date: Fri, 19 Nov 2021 14:34:58 -0500 Subject: [PATCH 1/6] Change admin polys and tz polys to be stored in std::multimap instead of std::unordered_multimap. This preserves the order so that polygons that are the result of state queries are tested first in GetMultiPolyId. With unordered_multipmap, the order is indeterminant and the country polygon can be checked first. This is sub-optimal since they are generally much larger, and often would then need to also check the state polygons as well. Another optimization is to avoid the country query if there is only 1 multipolygon resulting from the state query AND the bounding box is entirely contained within the state polygon. In this case it is safe to set the tile_within_one_admin flag to true which will avoid calling boost geometry covered_by for each node lat,lng within the tile. --- src/mjolnir/admin.cc | 59 ++++++++++++++++--------- src/mjolnir/graphbuilder.cc | 4 +- src/mjolnir/valhalla_convert_transit.cc | 4 +- src/mjolnir/valhalla_fetch_transit.cc | 4 +- valhalla/mjolnir/admin.h | 12 ++--- 5 files changed, 50 insertions(+), 33 deletions(-) diff --git a/src/mjolnir/admin.cc b/src/mjolnir/admin.cc index e717e79dfd..2dffc2b4ac 100644 --- a/src/mjolnir/admin.cc +++ b/src/mjolnir/admin.cc @@ -10,6 +10,8 @@ namespace valhalla { namespace mjolnir { +using polygon_type = boost::geometry::model::polygon; + // Get the dbhandle of a sqlite db. Used for timezones and admins DBs. sqlite3* GetDBHandle(const std::string& database) { @@ -37,7 +39,7 @@ sqlite3* GetDBHandle(const std::string& database) { // Get the polygon index. Used by tz and admin areas. Checks if the pointLL is covered_by the // poly. -uint32_t GetMultiPolyId(const std::unordered_multimap& polys, +uint32_t GetMultiPolyId(const std::multimap& polys, const PointLL& ll, GraphTileBuilder& graphtile) { uint32_t index = 0; @@ -45,9 +47,6 @@ uint32_t GetMultiPolyId(const std::unordered_multimap& polys, +uint32_t GetMultiPolyId(const std::multimap& polys, const PointLL& ll) { uint32_t index = 0; point_type p(ll.lng(), ll.lat()); @@ -68,9 +67,9 @@ uint32_t GetMultiPolyId(const std::unordered_multimap GetTimeZones(sqlite3* db_handle, - const AABB2& aabb) { - std::unordered_multimap polys; +std::multimap GetTimeZones(sqlite3* db_handle, + const AABB2& aabb) { + std::multimap polys; if (!db_handle) { return polys; } @@ -127,7 +126,7 @@ void GetData(sqlite3* db_handle, sqlite3_stmt* stmt, const std::string& sql, GraphTileBuilder& tilebuilder, - std::unordered_multimap& polys, + std::multimap& polys, std::unordered_map& drive_on_right, std::unordered_map& allow_intersection_names) { uint32_t result = 0; @@ -197,13 +196,13 @@ void GetData(sqlite3* db_handle, } // Get the admin polys that intersect with the tile bounding box. -std::unordered_multimap +std::multimap GetAdminInfo(sqlite3* db_handle, std::unordered_map& drive_on_right, std::unordered_map& allow_intersection_names, const AABB2& aabb, GraphTileBuilder& tilebuilder) { - std::unordered_multimap polys; + std::multimap polys; if (!db_handle) { return polys; } @@ -224,17 +223,35 @@ GetAdminInfo(sqlite3* db_handle, sql += std::to_string(aabb.maxy()) + "));"; GetData(db_handle, stmt, sql, tilebuilder, polys, drive_on_right, allow_intersection_names); + // If the state query yields a single result and the bounding box is entirely contained within + // the polygon we can return. TODO - this must have both a state and country field. + bool skip_country = false; + if (polys.size() == 1) { + // Turn aabb into a polygon and check if within poly + polygon_type polygon; + boost::geometry::append(polygon.outer(), point_type(aabb.minx(), aabb.miny())); + boost::geometry::append(polygon.outer(), point_type(aabb.maxx(), aabb.miny())); + boost::geometry::append(polygon.outer(), point_type(aabb.maxx(), aabb.maxy())); + boost::geometry::append(polygon.outer(), point_type(aabb.minx(), aabb.maxy())); + // TODO - does the polygon have to be explicitly closed + boost::geometry::append(polygon.outer(), point_type(aabb.minx(), aabb.miny())); + + skip_country = boost::geometry::within(polygon, polys.begin()->second); + } + // country query - sql = - "SELECT name, \"\", iso_code, \"\", drive_on_right, allow_intersection_names, st_astext(geom) from "; - sql += " admins where ST_Intersects(geom, BuildMBR(" + std::to_string(aabb.minx()) + ","; - sql += std::to_string(aabb.miny()) + ", " + std::to_string(aabb.maxx()) + ","; - sql += std::to_string(aabb.maxy()) + ")) and admin_level=2 "; - sql += "and rowid IN (SELECT rowid FROM SpatialIndex WHERE f_table_name = "; - sql += "'admins' AND search_frame = BuildMBR(" + std::to_string(aabb.minx()) + ","; - sql += std::to_string(aabb.miny()) + ", " + std::to_string(aabb.maxx()) + ","; - sql += std::to_string(aabb.maxy()) + "));"; - GetData(db_handle, stmt, sql, tilebuilder, polys, drive_on_right, allow_intersection_names); + if (!skip_country) { + sql = + "SELECT name, \"\", iso_code, \"\", drive_on_right, allow_intersection_names, st_astext(geom) from "; + sql += " admins where ST_Intersects(geom, BuildMBR(" + std::to_string(aabb.minx()) + ","; + sql += std::to_string(aabb.miny()) + ", " + std::to_string(aabb.maxx()) + ","; + sql += std::to_string(aabb.maxy()) + ")) and admin_level=2 "; + sql += "and rowid IN (SELECT rowid FROM SpatialIndex WHERE f_table_name = "; + sql += "'admins' AND search_frame = BuildMBR(" + std::to_string(aabb.minx()) + ","; + sql += std::to_string(aabb.miny()) + ", " + std::to_string(aabb.maxx()) + ","; + sql += std::to_string(aabb.maxy()) + "));"; + GetData(db_handle, stmt, sql, tilebuilder, polys, drive_on_right, allow_intersection_names); + } if (stmt) { // just in case something bad happened. sqlite3_finalize(stmt); diff --git a/src/mjolnir/graphbuilder.cc b/src/mjolnir/graphbuilder.cc index a2aeb48d77..47bb377435 100644 --- a/src/mjolnir/graphbuilder.cc +++ b/src/mjolnir/graphbuilder.cc @@ -470,7 +470,7 @@ void BuildTileSet(const std::string& ways_file, // Get the admin polygons. If only one exists for the tile check if the // tile is entirely inside the polygon bool tile_within_one_admin = false; - std::unordered_multimap admin_polys; + std::multimap admin_polys; std::unordered_map drive_on_right; std::unordered_map allow_intersection_names; @@ -484,7 +484,7 @@ void BuildTileSet(const std::string& ways_file, } bool tile_within_one_tz = false; - std::unordered_multimap tz_polys; + std::multimap tz_polys; if (tz_db_handle) { tz_polys = GetTimeZones(tz_db_handle, tiling.TileBounds(id)); if (tz_polys.size() == 1) { diff --git a/src/mjolnir/valhalla_convert_transit.cc b/src/mjolnir/valhalla_convert_transit.cc index 8c529799cc..a0e05480e1 100644 --- a/src/mjolnir/valhalla_convert_transit.cc +++ b/src/mjolnir/valhalla_convert_transit.cc @@ -490,7 +490,7 @@ void AddToGraph(GraphTileBuilder& tilebuilder_transit, const std::vector& distances, const std::vector& route_types, bool tile_within_one_tz, - const std::unordered_multimap& tz_polys, + const std::multimap& tz_polys, uint32_t& no_dir_edge_count) { auto t1 = std::chrono::high_resolution_clock::now(); @@ -1148,7 +1148,7 @@ void build_tiles(const boost::property_tree::ptree& pt, std::vector route_types = AddRoutes(transit, tilebuilder_transit); auto filter = tiles.TileBounds(tile_id.tileid()); bool tile_within_one_tz = false; - std::unordered_multimap tz_polys; + std::multimap tz_polys; if (tz_db_handle) { tz_polys = GetTimeZones(tz_db_handle, filter); if (tz_polys.size() == 1) { diff --git a/src/mjolnir/valhalla_fetch_transit.cc b/src/mjolnir/valhalla_fetch_transit.cc index 78663409b7..ddcdda931c 100644 --- a/src/mjolnir/valhalla_fetch_transit.cc +++ b/src/mjolnir/valhalla_fetch_transit.cc @@ -287,7 +287,7 @@ void get_stop_stations(Transit& tile, const ptree& response, const AABB2& filter/*, bool tile_within_one_tz, - const std::unordered_multimap& tz_polys*/) { + const std::multimap& tz_polys*/) { for (const auto& station_pt : response.get_child("stop_stations")) { @@ -774,7 +774,7 @@ void fetch_tiles(const ptree& pt, LOG_INFO("Fetching " + transit_tile.string()); bool tile_within_one_tz = false; - std::unordered_multimap tz_polys; + std::multimap tz_polys; if (tz_db_handle) { tz_polys = GetTimeZones(tz_db_handle, filter); if (tz_polys.size() == 1) { diff --git a/valhalla/mjolnir/admin.h b/valhalla/mjolnir/admin.h index 85d93437b9..82a2d27188 100644 --- a/valhalla/mjolnir/admin.h +++ b/valhalla/mjolnir/admin.h @@ -39,7 +39,7 @@ sqlite3* GetDBHandle(const std::string& database); * @param ll point that needs to be checked. * @param graphtile graphtilebuilder that is used to determine if we are a country poly or not. */ -uint32_t GetMultiPolyId(const std::unordered_multimap& polys, +uint32_t GetMultiPolyId(const std::multimap& polys, const PointLL& ll, GraphTileBuilder& graphtile); @@ -49,7 +49,7 @@ uint32_t GetMultiPolyId(const std::unordered_multimap& polys, +uint32_t GetMultiPolyId(const std::multimap& polys, const PointLL& ll); /** @@ -57,8 +57,8 @@ uint32_t GetMultiPolyId(const std::unordered_multimap GetTimeZones(sqlite3* db_handle, - const AABB2& aabb); +std::multimap GetTimeZones(sqlite3* db_handle, + const AABB2& aabb); /** * Get the admin data from the spatialite db given an SQL statement * @param db_handle sqlite3 db handle @@ -73,7 +73,7 @@ void GetData(sqlite3* db_handle, sqlite3_stmt* stmt, const std::string& sql, GraphTileBuilder& tilebuilder, - std::unordered_multimap& polys, + std::multimap& polys, std::unordered_map& drive_on_right); /** @@ -86,7 +86,7 @@ void GetData(sqlite3* db_handle, * @param aabb bb of the tile * @param tilebuilder Graph tile builder */ -std::unordered_multimap +std::multimap GetAdminInfo(sqlite3* db_handle, std::unordered_map& drive_on_right, std::unordered_map& allow_intersection_names, From e0bf19dd7c555c128df5c74275a19e3a7d1de5fb Mon Sep 17 00:00:00 2001 From: dnesbitt61 Date: Wed, 24 Nov 2021 12:30:36 -0500 Subject: [PATCH 2/6] Revert change that avoided country query if bounding box was entirely within the single state query result. This change led to tile size changes which may be ok but was not verified. The change without this still leads to substantial performance improvement. --- src/mjolnir/admin.cc | 41 +++++++++++++---------------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/src/mjolnir/admin.cc b/src/mjolnir/admin.cc index 2dffc2b4ac..f5b1328675 100644 --- a/src/mjolnir/admin.cc +++ b/src/mjolnir/admin.cc @@ -47,6 +47,9 @@ uint32_t GetMultiPolyId(const std::multimap& polys for (const auto& poly : polys) { if (boost::geometry::covered_by(p, poly.second)) { const auto& admin = graphtile.admins_builder(poly.first); + if (!admin.state_offset()) + index = poly.first; + else return poly.first; } } @@ -223,35 +226,17 @@ GetAdminInfo(sqlite3* db_handle, sql += std::to_string(aabb.maxy()) + "));"; GetData(db_handle, stmt, sql, tilebuilder, polys, drive_on_right, allow_intersection_names); - // If the state query yields a single result and the bounding box is entirely contained within - // the polygon we can return. TODO - this must have both a state and country field. - bool skip_country = false; - if (polys.size() == 1) { - // Turn aabb into a polygon and check if within poly - polygon_type polygon; - boost::geometry::append(polygon.outer(), point_type(aabb.minx(), aabb.miny())); - boost::geometry::append(polygon.outer(), point_type(aabb.maxx(), aabb.miny())); - boost::geometry::append(polygon.outer(), point_type(aabb.maxx(), aabb.maxy())); - boost::geometry::append(polygon.outer(), point_type(aabb.minx(), aabb.maxy())); - // TODO - does the polygon have to be explicitly closed - boost::geometry::append(polygon.outer(), point_type(aabb.minx(), aabb.miny())); - - skip_country = boost::geometry::within(polygon, polys.begin()->second); - } - // country query - if (!skip_country) { - sql = - "SELECT name, \"\", iso_code, \"\", drive_on_right, allow_intersection_names, st_astext(geom) from "; - sql += " admins where ST_Intersects(geom, BuildMBR(" + std::to_string(aabb.minx()) + ","; - sql += std::to_string(aabb.miny()) + ", " + std::to_string(aabb.maxx()) + ","; - sql += std::to_string(aabb.maxy()) + ")) and admin_level=2 "; - sql += "and rowid IN (SELECT rowid FROM SpatialIndex WHERE f_table_name = "; - sql += "'admins' AND search_frame = BuildMBR(" + std::to_string(aabb.minx()) + ","; - sql += std::to_string(aabb.miny()) + ", " + std::to_string(aabb.maxx()) + ","; - sql += std::to_string(aabb.maxy()) + "));"; - GetData(db_handle, stmt, sql, tilebuilder, polys, drive_on_right, allow_intersection_names); - } + sql = + "SELECT name, \"\", iso_code, \"\", drive_on_right, allow_intersection_names, st_astext(geom) from "; + sql += " admins where ST_Intersects(geom, BuildMBR(" + std::to_string(aabb.minx()) + ","; + sql += std::to_string(aabb.miny()) + ", " + std::to_string(aabb.maxx()) + ","; + sql += std::to_string(aabb.maxy()) + ")) and admin_level=2 "; + sql += "and rowid IN (SELECT rowid FROM SpatialIndex WHERE f_table_name = "; + sql += "'admins' AND search_frame = BuildMBR(" + std::to_string(aabb.minx()) + ","; + sql += std::to_string(aabb.miny()) + ", " + std::to_string(aabb.maxx()) + ","; + sql += std::to_string(aabb.maxy()) + "));"; + GetData(db_handle, stmt, sql, tilebuilder, polys, drive_on_right, allow_intersection_names); if (stmt) { // just in case something bad happened. sqlite3_finalize(stmt); From d96e6695618ac1f07123c750eda379aa19ee4d6a Mon Sep 17 00:00:00 2001 From: dnesbitt61 Date: Wed, 24 Nov 2021 12:59:46 -0500 Subject: [PATCH 3/6] Formatting. --- src/mjolnir/admin.cc | 3 +-- valhalla/mjolnir/admin.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/mjolnir/admin.cc b/src/mjolnir/admin.cc index f5b1328675..e72fbcc642 100644 --- a/src/mjolnir/admin.cc +++ b/src/mjolnir/admin.cc @@ -58,8 +58,7 @@ uint32_t GetMultiPolyId(const std::multimap& polys // Get the polygon index. Used by tz and admin areas. Checks if the pointLL is covered_by the // poly. -uint32_t GetMultiPolyId(const std::multimap& polys, - const PointLL& ll) { +uint32_t GetMultiPolyId(const std::multimap& polys, const PointLL& ll) { uint32_t index = 0; point_type p(ll.lng(), ll.lat()); for (const auto& poly : polys) { diff --git a/valhalla/mjolnir/admin.h b/valhalla/mjolnir/admin.h index 82a2d27188..a96eb1c5a8 100644 --- a/valhalla/mjolnir/admin.h +++ b/valhalla/mjolnir/admin.h @@ -49,8 +49,7 @@ uint32_t GetMultiPolyId(const std::multimap& polys * @param polys unordered map of polys. * @param ll point that needs to be checked. */ -uint32_t GetMultiPolyId(const std::multimap& polys, - const PointLL& ll); +uint32_t GetMultiPolyId(const std::multimap& polys, const PointLL& ll); /** * Get the timezone polys from the db From bafea7843554e5638339d607cca07ba994f92160 Mon Sep 17 00:00:00 2001 From: dnesbitt61 Date: Wed, 24 Nov 2021 13:01:05 -0500 Subject: [PATCH 4/6] Add admin poly change --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01a9d47720..5e2adb083e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ * ADDED: Add boolean parameter to clear memory for edge labels from thor. [#2789](https://github.com/valhalla/valhalla/pull/2789) * CHANGED: Do not create statsd client in workers if it is not configured [#3394](https://github.com/valhalla/valhalla/pull/3394) * ADDED: Import of Bike Share Stations information in BSS Connection edges [#3411](https://github.com/valhalla/valhalla/pull/3411) + * CHANGED: Use std::multimap for polygons returned for admin and timezone queries. Improves performance when building tiles. [#3427](https://github.com/valhalla/valhalla/pull/3427) ## Release Date: 2021-10-07 Valhalla 3.1.4 * **Removed** From a4d9b1b209232fbb1d4dc6ee7b9caaf02da86df8 Mon Sep 17 00:00:00 2001 From: Kevin Kreiser Date: Mon, 6 Dec 2021 10:49:55 -0500 Subject: [PATCH 5/6] remove unused type alias --- src/mjolnir/admin.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mjolnir/admin.cc b/src/mjolnir/admin.cc index e72fbcc642..4b8bf11820 100644 --- a/src/mjolnir/admin.cc +++ b/src/mjolnir/admin.cc @@ -10,8 +10,6 @@ namespace valhalla { namespace mjolnir { -using polygon_type = boost::geometry::model::polygon; - // Get the dbhandle of a sqlite db. Used for timezones and admins DBs. sqlite3* GetDBHandle(const std::string& database) { From 7d1a458b717ebd1c58c1a478efb063a341524262 Mon Sep 17 00:00:00 2001 From: dnesbitt61 Date: Mon, 6 Dec 2021 11:03:35 -0500 Subject: [PATCH 6/6] Remove unused code --- src/mjolnir/admin.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mjolnir/admin.cc b/src/mjolnir/admin.cc index e72fbcc642..4b8bf11820 100644 --- a/src/mjolnir/admin.cc +++ b/src/mjolnir/admin.cc @@ -10,8 +10,6 @@ namespace valhalla { namespace mjolnir { -using polygon_type = boost::geometry::model::polygon; - // Get the dbhandle of a sqlite db. Used for timezones and admins DBs. sqlite3* GetDBHandle(const std::string& database) {