perf: Iterate over only kLandmark tagged values in AddLandmarks (v…

…alhalla#4873)
Georepublic · Sep 23, 2024 · 5f48bfa · 5f48bfa
1 parent ac0cc3d
commit 5f48bfa
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 36 deletions.
diff --git 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
@@ -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<valhalla::RouteLandmark::Type>(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<double>(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<valhalla::RouteLandmark::Type>(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<double>(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);
   }
 }