From 5f48bfa4664dd73f0901e1ce22cfdaede6debe70 Mon Sep 17 00:00:00 2001 From: Stepan Kizim <10885920+kinkard@users.noreply.github.com> Date: Mon, 23 Sep 2024 16:32:50 +0200 Subject: [PATCH] perf: Iterate over only `kLandmark` tagged values in `AddLandmarks` (#4873) --- CHANGELOG.md | 1 + src/thor/triplegbuilder.cc | 70 ++++++++++++++++++-------------------- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff7cd424ae..6f02cadfc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * **Removed** * **Bug Fix** * FIXED: All logging in `valhalla_export_edges` now goes to stderr [#4892](https://github.com/valhalla/valhalla/pull/4892) + * FIXED: Iterate over only `kLandmark` tagged values in `AddLandmarks()` [#4873](https://github.com/valhalla/valhalla/pull/4873) * **Enhancement** * CHANGED: voice instructions for OSRM serializer to work better in real-world environment [#4756](https://github.com/valhalla/valhalla/pull/4756) * ADDED: Add option `edge.forward` to trace attributes [#4876](https://github.com/valhalla/valhalla/pull/4876) diff --git a/src/thor/triplegbuilder.cc b/src/thor/triplegbuilder.cc index a29f241158..8a796252e3 100644 --- a/src/thor/triplegbuilder.cc +++ b/src/thor/triplegbuilder.cc @@ -507,42 +507,40 @@ void AddLandmarks(const EdgeInfo& edgeinfo, return; } - for (const auto& tag : edgeinfo.GetTags()) { - // get landmarks from tagged values in the edge info - if (tag.first == baldr::TaggedValue::kLandmark) { - Landmark lan(tag.second); - PointLL landmark_point = {lan.lng, lan.lat}; - - // find the closed point on edge to the landmark - auto closest = landmark_point.ClosestPoint(shape, begin_index); - // TODO: in the future maybe we could allow a request option to have a tighter threshold on - // how far landmarks should be away from an edge - - // add the landmark to trip leg - auto* landmark = trip_edge->mutable_landmarks()->Add(); - landmark->set_name(lan.name); - landmark->set_type(static_cast(lan.type)); - landmark->mutable_lat_lng()->set_lng(lan.lng); - landmark->mutable_lat_lng()->set_lat(lan.lat); - - // calculate the landmark's distance along the edge - // that is to accumulate distance from the begin point to the closest point to it on the edge - int closest_idx = std::get<2>(closest); - double distance_along_edge = 0; - for (int idx = begin_index + 1; idx <= closest_idx; ++idx) { - distance_along_edge += shape[idx].Distance(shape[idx - 1]); - } - distance_along_edge += shape[closest_idx].Distance(std::get<0>(closest)); - // the overall distance shouldn't be larger than edge length - distance_along_edge = std::min(distance_along_edge, static_cast(edge->length())); - landmark->set_distance(distance_along_edge); - // check which side of the edge the landmark is on - // quirks of the ClosestPoint function - bool is_right = closest_idx == (int)shape.size() - 1 - ? landmark_point.IsLeft(shape[closest_idx - 1], shape[closest_idx]) < 0 - : landmark_point.IsLeft(shape[closest_idx], shape[closest_idx + 1]) < 0; - landmark->set_right(is_right); - } + const auto landmark_range = edgeinfo.GetTags().equal_range(baldr::TaggedValue::kLandmark); + for (auto it = landmark_range.first; it != landmark_range.second; ++it) { + Landmark lan(it->second); + PointLL landmark_point = {lan.lng, lan.lat}; + + // find the closed point on edge to the landmark + auto closest = landmark_point.ClosestPoint(shape, begin_index); + // TODO: in the future maybe we could allow a request option to have a tighter threshold on + // how far landmarks should be away from an edge + + // add the landmark to trip leg + auto* landmark = trip_edge->mutable_landmarks()->Add(); + landmark->set_name(lan.name); + landmark->set_type(static_cast(lan.type)); + landmark->mutable_lat_lng()->set_lng(lan.lng); + landmark->mutable_lat_lng()->set_lat(lan.lat); + + // calculate the landmark's distance along the edge + // that is to accumulate distance from the begin point to the closest point to it on the edge + int closest_idx = std::get<2>(closest); + double distance_along_edge = 0; + for (int idx = begin_index + 1; idx <= closest_idx; ++idx) { + distance_along_edge += shape[idx].Distance(shape[idx - 1]); + } + distance_along_edge += shape[closest_idx].Distance(std::get<0>(closest)); + // the overall distance shouldn't be larger than edge length + distance_along_edge = std::min(distance_along_edge, static_cast(edge->length())); + landmark->set_distance(distance_along_edge); + // check which side of the edge the landmark is on + // quirks of the ClosestPoint function + bool is_right = closest_idx == (int)shape.size() - 1 + ? landmark_point.IsLeft(shape[closest_idx - 1], shape[closest_idx]) < 0 + : landmark_point.IsLeft(shape[closest_idx], shape[closest_idx + 1]) < 0; + landmark->set_right(is_right); } }