From c14461c27392c532fbf95bec54b3f27812e61c12 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sun, 28 Aug 2022 09:10:33 +0200 Subject: [PATCH 01/11] Optimize RestrictionParser performance --- include/extractor/restriction_parser.hpp | 7 +- src/extractor/restriction_parser.cpp | 142 +++++++++++------------ 2 files changed, 73 insertions(+), 76 deletions(-) diff --git a/include/extractor/restriction_parser.hpp b/include/extractor/restriction_parser.hpp index 58539685726..6749c90bcb8 100644 --- a/include/extractor/restriction_parser.hpp +++ b/include/extractor/restriction_parser.hpp @@ -6,7 +6,9 @@ #include #include +#include #include +#include namespace osmium { @@ -43,7 +45,7 @@ class RestrictionParser public: RestrictionParser(bool use_turn_restrictions, bool parse_conditionals, - std::vector &restrictions); + const std::vector &restrictions); std::vector TryParse(const osmium::Relation &relation) const; private: @@ -51,7 +53,8 @@ class RestrictionParser bool use_turn_restrictions; bool parse_conditionals; - std::vector restrictions; + std::set restrictions; + osmium::tags::KeyFilter filter; }; } // namespace extractor } // namespace osrm diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index 2e4328acfe9..90a2e0dc311 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -9,7 +9,6 @@ #include #include -#include #include @@ -20,9 +19,9 @@ namespace extractor RestrictionParser::RestrictionParser(bool use_turn_restrictions_, bool parse_conditionals_, - std::vector &restrictions_) + const std::vector &restrictions_) : use_turn_restrictions(use_turn_restrictions_), parse_conditionals(parse_conditionals_), - restrictions(restrictions_) + restrictions(restrictions_.begin(), restrictions_.end()), filter(false) { if (use_turn_restrictions) { @@ -40,11 +39,28 @@ RestrictionParser::RestrictionParser(bool use_turn_restrictions_, util::Log() << "Found no turn restriction tags"; } } + + filter.add(true, "restriction"); + if (parse_conditionals) + { + filter.add(true, "restriction:conditional"); + for (const auto &namespaced : restrictions_) + { + filter.add(true, "restriction:" + namespaced + ":conditional"); + } + } + + // Not only use restriction= but also e.g. restriction:motorcar= + // Include restriction:{mode}:conditional if flagged + for (const auto &namespaced : restrictions_) + { + filter.add(true, "restriction:" + namespaced); + } } /** * Tries to parse a relation as a turn restriction. This can fail for a number of - * reasons. The return type is a boost::optional. + * reasons. The return type is a std::vector. * * Some restrictions can also be ignored: See the ```get_restrictions``` function * in the corresponding profile. We use it for both namespacing restrictions, as in @@ -59,31 +75,13 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const return {}; } - osmium::tags::KeyFilter filter(false); - filter.add(true, "restriction"); - if (parse_conditionals) - { - filter.add(true, "restriction:conditional"); - for (const auto &namespaced : restrictions) - { - filter.add(true, "restriction:" + namespaced + ":conditional"); - } - } - - // Not only use restriction= but also e.g. restriction:motorcar= - // Include restriction:{mode}:conditional if flagged - for (const auto &namespaced : restrictions) - { - filter.add(true, "restriction:" + namespaced); - } - const osmium::TagList &tag_list = relation.tags(); osmium::tags::KeyFilter::iterator fi_begin(filter, tag_list.begin(), tag_list.end()); osmium::tags::KeyFilter::iterator fi_end(filter, tag_list.end(), tag_list.end()); // if it's not a restriction, continue; - if (std::distance(fi_begin, fi_end) == 0) + if (fi_begin == fi_end) { return {}; } @@ -99,18 +97,19 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const bool is_multi_from = false; bool is_multi_to = false; + std::vector condition; + for (; fi_begin != fi_end; ++fi_begin) { - const std::string key(fi_begin->key()); const std::string value(fi_begin->value()); // documented OSM restriction tags start either with only_* or no_*; // check and return on these values, and ignore no_*_on_red or unrecognized values - if (value.find("only_") == 0) + if (boost::algorithm::starts_with(value, "only_")) { is_only_restriction = true; } - else if (value.find("no_") == 0 && !boost::algorithm::ends_with(value, "_on_red")) + else if (boost::algorithm::starts_with(value, "no_") && !boost::algorithm::ends_with(value, "_on_red")) { is_only_restriction = false; if (boost::algorithm::starts_with(value, "no_exit")) @@ -126,6 +125,24 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const { return {}; } + + if (parse_conditionals) { + // Parse condition and add independent value/condition pairs + const auto &parsed = osrm::util::ParseConditionalRestrictions(value); + + if (parsed.empty()) + continue; + + for (const auto &p : parsed) + { + std::vector hours = util::ParseOpeningHours(p.condition); + // found unrecognized condition, continue + if (hours.empty()) + return {}; + + condition = std::move(hours); + } + } } constexpr auto INVALID_OSM_ID = std::numeric_limits::max(); @@ -138,7 +155,11 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const for (const auto &member : relation.members()) { const char *role = member.role(); - if (strcmp("from", role) != 0 && strcmp("to", role) != 0 && strcmp("via", role) != 0) + const bool is_from_role = strcmp("from", role) == 0; + const bool is_to_role = strcmp("to", role) == 0; + const bool is_via_role = strcmp("via", role) == 0; + + if (!is_from_role && !is_to_role && !is_via_role) { continue; } @@ -149,28 +170,27 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const { // Make sure nodes appear only in the role if a via node - if (0 == strcmp("from", role) || 0 == strcmp("to", role)) + if (is_from_role || is_to_role) { continue; } - BOOST_ASSERT(0 == strcmp("via", role)); + BOOST_ASSERT(is_via_role); via_node = static_cast(member.ref()); is_node_restriction = true; // set via node id break; } case osmium::item_type::way: - BOOST_ASSERT(0 == strcmp("from", role) || 0 == strcmp("to", role) || - 0 == strcmp("via", role)); - if (0 == strcmp("from", role)) + BOOST_ASSERT(is_from_role || is_to_role || is_via_role); + if (is_from_role) { from_ways.push_back({static_cast(member.ref())}); } - else if (0 == strcmp("to", role)) + else if (is_to_role) { to_ways.push_back({static_cast(member.ref())}); } - else if (0 == strcmp("via", role)) + else if (is_via_role) { via_ways.push_back({static_cast(member.ref())}); is_node_restriction = false; @@ -185,35 +205,6 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const } } - std::vector condition; - // parse conditional tags - if (parse_conditionals) - { - osmium::tags::KeyFilter::iterator fi_begin(filter, tag_list.begin(), tag_list.end()); - osmium::tags::KeyFilter::iterator fi_end(filter, tag_list.end(), tag_list.end()); - for (; fi_begin != fi_end; ++fi_begin) - { - const std::string key(fi_begin->key()); - const std::string value(fi_begin->value()); - - // Parse condition and add independent value/condition pairs - const auto &parsed = osrm::util::ParseConditionalRestrictions(value); - - if (parsed.empty()) - continue; - - for (const auto &p : parsed) - { - std::vector hours = util::ParseOpeningHours(p.condition); - // found unrecognized condition, continue - if (hours.empty()) - return {}; - - condition = std::move(hours); - } - } - } - std::vector restriction_containers; if (!from_ways.empty() && (via_node != INVALID_OSM_ID || !via_ways.empty()) && !to_ways.empty()) { @@ -270,17 +261,20 @@ bool RestrictionParser::ShouldIgnoreRestriction(const std::string &except_tag_st return false; } - // Be warned, this is quadratic work here, but we assume that - // only a few exceptions are actually defined. - const std::regex delimiter_re("[;][ ]*"); - std::sregex_token_iterator except_tags_begin( - except_tag_string.begin(), except_tag_string.end(), delimiter_re, -1); - std::sregex_token_iterator except_tags_end; - - return std::any_of(except_tags_begin, except_tags_end, [&](const std::string ¤t_string) { - return std::end(restrictions) != - std::find(std::begin(restrictions), std::end(restrictions), current_string); - }); + // split `except_tag_string` by semicolon and check if any of items is in `restrictions` + std::string current_string; + for (size_t index = 0; index < except_tag_string.size(); index++) { + const auto ch = except_tag_string[index]; + if (ch != ';') { + current_string += ch; + } else { + if (restrictions.find(current_string) != restrictions.end()) { + return true; + } + current_string.clear(); + } + } + return restrictions.find(current_string) != restrictions.end(); } } // namespace extractor } // namespace osrm From 01e2a2d3747688ce1a3b31c56c8cabc4b41cb9ca Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sun, 28 Aug 2022 09:11:50 +0200 Subject: [PATCH 02/11] Optimize RestrictionParser performance --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25c8aa33742..b04ad0df34e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - NodeJS: - FIXED: Support `skip_waypoints` in Node bindings [#6060](https://github.com/Project-OSRM/osrm-backend/pull/6060) - Misc: + - CHANGED: Optimize RestrictionParser performance. [#6344](https://github.com/Project-OSRM/osrm-backend/pull/6344) - CHANGED: Remove redundant nullptr check. [#6326](https://github.com/Project-OSRM/osrm-backend/pull/6326) - CHANGED: missing files list is included in exception message. [#5360](https://github.com/Project-OSRM/osrm-backend/pull/5360) - CHANGED: Do not use deprecated Callback::Call overload in Node bindings. [#6318](https://github.com/Project-OSRM/osrm-backend/pull/6318) From 794ae5996f4fe393f0852968a21213f4569548f6 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sun, 28 Aug 2022 09:12:49 +0200 Subject: [PATCH 03/11] Optimize RestrictionParser performance --- include/extractor/restriction_parser.hpp | 4 ++-- src/extractor/restriction_parser.cpp | 21 ++++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/include/extractor/restriction_parser.hpp b/include/extractor/restriction_parser.hpp index 6749c90bcb8..869ff9b8404 100644 --- a/include/extractor/restriction_parser.hpp +++ b/include/extractor/restriction_parser.hpp @@ -5,10 +5,10 @@ #include -#include +#include #include +#include #include -#include namespace osmium { diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index 90a2e0dc311..37ad2c212c9 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -109,7 +109,8 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const { is_only_restriction = true; } - else if (boost::algorithm::starts_with(value, "no_") && !boost::algorithm::ends_with(value, "_on_red")) + else if (boost::algorithm::starts_with(value, "no_") && + !boost::algorithm::ends_with(value, "_on_red")) { is_only_restriction = false; if (boost::algorithm::starts_with(value, "no_exit")) @@ -126,7 +127,8 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const return {}; } - if (parse_conditionals) { + if (parse_conditionals) + { // Parse condition and add independent value/condition pairs const auto &parsed = osrm::util::ParseConditionalRestrictions(value); @@ -158,7 +160,7 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const const bool is_from_role = strcmp("from", role) == 0; const bool is_to_role = strcmp("to", role) == 0; const bool is_via_role = strcmp("via", role) == 0; - + if (!is_from_role && !is_to_role && !is_via_role) { continue; @@ -263,12 +265,17 @@ bool RestrictionParser::ShouldIgnoreRestriction(const std::string &except_tag_st // split `except_tag_string` by semicolon and check if any of items is in `restrictions` std::string current_string; - for (size_t index = 0; index < except_tag_string.size(); index++) { + for (size_t index = 0; index < except_tag_string.size(); index++) + { const auto ch = except_tag_string[index]; - if (ch != ';') { + if (ch != ';') + { current_string += ch; - } else { - if (restrictions.find(current_string) != restrictions.end()) { + } + else + { + if (restrictions.find(current_string) != restrictions.end()) + { return true; } current_string.clear(); From cf223ef516223ce5d24a59301f986c1d0cd618ce Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sun, 28 Aug 2022 09:20:53 +0200 Subject: [PATCH 04/11] Optimize RestrictionParser performance --- src/extractor/restriction_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index 37ad2c212c9..94748bada5a 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -101,7 +101,7 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const for (; fi_begin != fi_end; ++fi_begin) { - const std::string value(fi_begin->value()); + auto value = fi_begin->value(); // documented OSM restriction tags start either with only_* or no_*; // check and return on these values, and ignore no_*_on_red or unrecognized values From 1d86602cee09d08a6e312e3a6f907f8acde47c8e Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 29 Aug 2022 17:59:13 +0200 Subject: [PATCH 05/11] Optimize RestrictionParser performance --- include/extractor/restriction_parser.hpp | 4 ++-- src/extractor/restriction_parser.cpp | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/extractor/restriction_parser.hpp b/include/extractor/restriction_parser.hpp index 869ff9b8404..c6f1ae46e88 100644 --- a/include/extractor/restriction_parser.hpp +++ b/include/extractor/restriction_parser.hpp @@ -5,7 +5,7 @@ #include -#include +#include #include #include #include @@ -54,7 +54,7 @@ class RestrictionParser bool use_turn_restrictions; bool parse_conditionals; std::set restrictions; - osmium::tags::KeyFilter filter; + osmium::TagsFilter filter; }; } // namespace extractor } // namespace osrm diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index 94748bada5a..4c5f343dbec 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -11,6 +11,7 @@ #include #include +#include namespace osrm { @@ -40,13 +41,13 @@ RestrictionParser::RestrictionParser(bool use_turn_restrictions_, } } - filter.add(true, "restriction"); + filter.add_rule(true, "restriction"); if (parse_conditionals) { - filter.add(true, "restriction:conditional"); + filter.add_rule(true, "restriction:conditional"); for (const auto &namespaced : restrictions_) { - filter.add(true, "restriction:" + namespaced + ":conditional"); + filter.add_rule(true, "restriction:" + namespaced + ":conditional"); } } @@ -54,7 +55,7 @@ RestrictionParser::RestrictionParser(bool use_turn_restrictions_, // Include restriction:{mode}:conditional if flagged for (const auto &namespaced : restrictions_) { - filter.add(true, "restriction:" + namespaced); + filter.add_rule(true, "restriction:" + namespaced); } } @@ -77,8 +78,8 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const const osmium::TagList &tag_list = relation.tags(); - osmium::tags::KeyFilter::iterator fi_begin(filter, tag_list.begin(), tag_list.end()); - osmium::tags::KeyFilter::iterator fi_end(filter, tag_list.end(), tag_list.end()); + osmium::TagsFilter::iterator fi_begin(filter, tag_list.begin(), tag_list.end()); + osmium::TagsFilter::iterator fi_end(filter, tag_list.end(), tag_list.end()); // if it's not a restriction, continue; if (fi_begin == fi_end) From ecbee992692e00b25458d602d0301a7653ab5a8b Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 29 Aug 2022 21:58:25 +0200 Subject: [PATCH 06/11] Update src/extractor/restriction_parser.cpp Co-authored-by: Michael Bell --- src/extractor/restriction_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index 4c5f343dbec..fc5f68c397c 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -266,7 +266,7 @@ bool RestrictionParser::ShouldIgnoreRestriction(const std::string &except_tag_st // split `except_tag_string` by semicolon and check if any of items is in `restrictions` std::string current_string; - for (size_t index = 0; index < except_tag_string.size(); index++) + for (auto index : util::irange(0, except_tag_string.size()) { const auto ch = except_tag_string[index]; if (ch != ';') From ff704db720340ea1179ecb06bf608da30271a291 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 29 Aug 2022 22:57:19 +0200 Subject: [PATCH 07/11] Update restriction_parser.cpp --- src/extractor/restriction_parser.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index fc5f68c397c..dd2c3ff4b43 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -2,6 +2,7 @@ #include "extractor/profile_properties.hpp" #include "util/conditional_restrictions.hpp" +#include "util/integer_range.hpp" #include "util/log.hpp" #include From 70a507f37164bfaa3f8d59202b4d0aba2d8915b1 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 29 Aug 2022 23:05:34 +0200 Subject: [PATCH 08/11] Update restriction_parser.cpp --- src/extractor/restriction_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index dd2c3ff4b43..8caf3adcdfd 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -267,7 +267,7 @@ bool RestrictionParser::ShouldIgnoreRestriction(const std::string &except_tag_st // split `except_tag_string` by semicolon and check if any of items is in `restrictions` std::string current_string; - for (auto index : util::irange(0, except_tag_string.size()) + for (auto index : util::irange(0, except_tag_string.size())) { const auto ch = except_tag_string[index]; if (ch != ';') From 47f7ad7f353066b6198cb49fc1e66912d96cb008 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 29 Aug 2022 23:25:28 +0200 Subject: [PATCH 09/11] Optimize RestrictionParser performance --- src/extractor/restriction_parser.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index 8caf3adcdfd..9c7f917d6c3 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -42,10 +42,12 @@ RestrictionParser::RestrictionParser(bool use_turn_restrictions_, } } - filter.add_rule(true, "restriction"); + using namespace std::string_literals; + + filter.add_rule(true, "restriction"s); if (parse_conditionals) { - filter.add_rule(true, "restriction:conditional"); + filter.add_rule(true, "restriction:conditional"s); for (const auto &namespaced : restrictions_) { filter.add_rule(true, "restriction:" + namespaced + ":conditional"); From 69a3843e51a94d0f73a5a8245d68cade1cd40ff4 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Tue, 30 Aug 2022 17:15:02 +0200 Subject: [PATCH 10/11] Optimize RestrictionParser performance --- src/extractor/restriction_parser.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index 07043c1be35..42a5a24103f 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -42,12 +42,10 @@ RestrictionParser::RestrictionParser(bool use_turn_restrictions_, } } - using namespace std::string_literals; - - filter.add(true, "restriction"s); + filter.add(true, "restriction"); if (parse_conditionals) { - filter.add(true, "restriction:conditional"s); + filter.add(true, "restriction:conditional"); for (const auto &namespaced : restrictions_) { filter.add(true, "restriction:" + namespaced + ":conditional"); From 7dfe4660218f7d792cd026863146f6558aff4eb8 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Tue, 30 Aug 2022 17:27:05 +0200 Subject: [PATCH 11/11] Optimize RestrictionParser performance --- src/extractor/restriction_parser.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index 42a5a24103f..5daab9e7440 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -12,7 +12,6 @@ #include #include -#include namespace osrm {