diff --git a/features/testbot/basic.feature b/features/testbot/basic.feature index 03b48929d..466fff2e1 100644 --- a/features/testbot/basic.feature +++ b/features/testbot/basic.feature @@ -233,7 +233,7 @@ Feature: Basic Routing | d | a | abcd,abcd | | a | m | aeim,aeim | | m | a | aeim,aeim | - + Scenario: Testbot - Triangle challenge Given the node map | | | | d | @@ -251,3 +251,39 @@ Feature: Basic Routing | from | to | route | | d | c | de,ce,ce | | e | d | de,de | + + Scenario: Ambiguous edge weights - Use minimal edge weight + Given the node map + | a | b | + + And the ways + | nodes | highway | name | + | ab | tertiary | | + | ab | primary | | + | ab | secondary | | + + When I route I should get + | from | to | route | time | + | a | b | , | 10s | + | b | a | , | 10s | + + Scenario: Ambiguous edge names - Use lexicographically smallest name + Given the node map + | a | b | c | + + And the ways + | nodes | highway | name | + | ab | primary | | + | ab | primary | Αβγ | + | ab | primary | | + | ab | primary | Abc | + | ab | primary | | + | ab | primary | Абв | + | bc | primary | Ηθι | + | bc | primary | Δεζ | + | bc | primary | Где | + + When I route I should get + | from | to | route | + | a | c | Abc,Δεζ,Δεζ | + | c | a | Δεζ,Abc,Abc | diff --git a/include/extractor/extraction_containers.hpp b/include/extractor/extraction_containers.hpp index 6f56dffb7..ccd710453 100644 --- a/include/extractor/extraction_containers.hpp +++ b/include/extractor/extraction_containers.hpp @@ -8,9 +8,9 @@ #include "extractor/restriction.hpp" #include "extractor/scripting_environment.hpp" +#include #include #include -#include namespace osrm { @@ -39,12 +39,11 @@ class ExtractionContainers void WriteNodes(std::ofstream &file_out_stream) const; void WriteRestrictions(const std::string &restrictions_file_name) const; void WriteEdges(std::ofstream &file_out_stream) const; - void WriteCharData(const std::string &file_name, - const stxxl::vector &offests, - const stxxl::vector &char_data) const; - void WriteTurnLaneMasks(const std::string &file_name, - const stxxl::vector &turn_lane_offsets, - const stxxl::vector &turn_lane_masks) const; + void WriteCharData(const std::string &file_name); + void + WriteTurnLaneMasks(const std::string &file_name, + const stxxl::vector &turn_lane_offsets, + const stxxl::vector &turn_lane_masks) const; public: using STXXLNodeIDVector = stxxl::vector; @@ -52,12 +51,14 @@ class ExtractionContainers using STXXLEdgeVector = stxxl::vector; using STXXLRestrictionsVector = stxxl::vector; using STXXLWayIDStartEndVector = stxxl::vector; + using STXXLNameCharData = stxxl::vector; + using STXXLNameOffsets = stxxl::vector; STXXLNodeIDVector used_node_id_list; STXXLNodeVector all_nodes_list; STXXLEdgeVector all_edges_list; - stxxl::vector name_char_data; - stxxl::vector name_lengths; + STXXLNameCharData name_char_data; + STXXLNameOffsets name_offsets; // an adjacency array containing all turn lane masks stxxl::vector turn_lane_offsets; stxxl::vector turn_lane_masks; diff --git a/include/extractor/internal_extractor_edge.hpp b/include/extractor/internal_extractor_edge.hpp index fea66ff8d..376e587c1 100644 --- a/include/extractor/internal_extractor_edge.hpp +++ b/include/extractor/internal_extractor_edge.hpp @@ -142,43 +142,6 @@ struct InternalExtractorEdge } }; -struct CmpEdgeByInternalStartThenInternalTargetID -{ - using value_type = InternalExtractorEdge; - bool operator()(const InternalExtractorEdge &lhs, const InternalExtractorEdge &rhs) const - { - return (lhs.result.source < rhs.result.source) || - ((lhs.result.source == rhs.result.source) && - (lhs.result.target < rhs.result.target)); - } - - value_type max_value() { return InternalExtractorEdge::max_internal_value(); } - value_type min_value() { return InternalExtractorEdge::min_internal_value(); } -}; - -struct CmpEdgeByOSMStartID -{ - using value_type = InternalExtractorEdge; - bool operator()(const InternalExtractorEdge &lhs, const InternalExtractorEdge &rhs) const - { - return lhs.result.osm_source_id < rhs.result.osm_source_id; - } - - value_type max_value() { return InternalExtractorEdge::max_osm_value(); } - value_type min_value() { return InternalExtractorEdge::min_osm_value(); } -}; - -struct CmpEdgeByOSMTargetID -{ - using value_type = InternalExtractorEdge; - bool operator()(const InternalExtractorEdge &lhs, const InternalExtractorEdge &rhs) const - { - return lhs.result.osm_target_id < rhs.result.osm_target_id; - } - - value_type max_value() { return InternalExtractorEdge::max_osm_value(); } - value_type min_value() { return InternalExtractorEdge::min_osm_value(); } -}; } } diff --git a/src/extractor/extraction_containers.cpp b/src/extractor/extraction_containers.cpp index d154304df..908083ff3 100644 --- a/src/extractor/extraction_containers.cpp +++ b/src/extractor/extraction_containers.cpp @@ -26,6 +26,8 @@ namespace { +namespace oe = osrm::extractor; + // Needed for STXXL comparison - STXXL requires max_value(), min_value(), so we can not use // std::less{}. Anonymous namespace to keep translation unit local. struct OSMNodeIDSTXXLLess @@ -35,6 +37,69 @@ struct OSMNodeIDSTXXLLess value_type max_value() { return MAX_OSM_NODEID; } value_type min_value() { return MIN_OSM_NODEID; } }; + +struct CmpEdgeByOSMStartID +{ + using value_type = oe::InternalExtractorEdge; + bool operator()(const value_type &lhs, const value_type &rhs) const + { + return lhs.result.osm_source_id < rhs.result.osm_source_id; + } + + value_type max_value() { return value_type::max_osm_value(); } + value_type min_value() { return value_type::min_osm_value(); } +}; + +struct CmpEdgeByOSMTargetID +{ + using value_type = oe::InternalExtractorEdge; + bool operator()(const value_type &lhs, const value_type &rhs) const + { + return lhs.result.osm_target_id < rhs.result.osm_target_id; + } + + value_type max_value() { return value_type::max_osm_value(); } + value_type min_value() { return value_type::min_osm_value(); } +}; + +struct CmpEdgeByInternalSourceTargetAndName +{ + using value_type = oe::InternalExtractorEdge; + bool operator()(const value_type &lhs, const value_type &rhs) const + { + if (lhs.result.source != rhs.result.source) + return lhs.result.source < rhs.result.source; + + if (lhs.result.target != rhs.result.target) + return lhs.result.target < rhs.result.target; + + if (lhs.result.source == SPECIAL_NODEID) + return false; + + if (lhs.result.name_id == rhs.result.name_id) + return false; + + if (lhs.result.name_id == EMPTY_NAMEID) + return false; + + if (rhs.result.name_id == EMPTY_NAMEID) + return true; + + BOOST_ASSERT(!name_offsets.empty() && name_offsets.back() == name_data.size()); + const oe::ExtractionContainers::STXXLNameCharData::const_iterator data = name_data.begin(); + return std::lexicographical_compare(data + name_offsets[lhs.result.name_id], + data + name_offsets[lhs.result.name_id + 1], + data + name_offsets[rhs.result.name_id], + data + name_offsets[rhs.result.name_id + 1]); + } + + value_type max_value() { return value_type::max_internal_value(); } + value_type min_value() { return value_type::min_internal_value(); } + + const oe::ExtractionContainers::STXXLNameCharData &name_data; + const oe::ExtractionContainers::STXXLNameOffsets &name_offsets; +}; + } namespace osrm @@ -48,10 +113,13 @@ ExtractionContainers::ExtractionContainers() { // Check if stxxl can be instantiated stxxl::vector dummy_vector; - // Insert three empty strings for name, destination and pronunciation - name_lengths.push_back(0); - name_lengths.push_back(0); - name_lengths.push_back(0); + + // Insert three empty strings offsets for name, destination and pronunciation + name_offsets.push_back(0); + name_offsets.push_back(0); + name_offsets.push_back(0); + // Insert the total length sentinel (corresponds to the next name string offset) + name_offsets.push_back(0); // the offsets have to be initialized with two values, since we have the empty turn string for // the first id @@ -90,7 +158,7 @@ void ExtractionContainers::PrepareData(const std::string &output_file_name, PrepareRestrictions(); WriteRestrictions(restrictions_file_name); - WriteCharData(name_file_name, name_lengths, name_char_data); + WriteCharData(name_file_name); WriteTurnLaneMasks(turn_lane_file_name, turn_lane_offsets, turn_lane_masks); } catch (const std::exception &e) @@ -125,23 +193,26 @@ void ExtractionContainers::WriteTurnLaneMasks( util::SimpleLogger().Write() << "done (" << TIMER_SEC(turn_lane_timer) << ")"; } -void ExtractionContainers::WriteCharData(const std::string &file_name, - const stxxl::vector &offsets, - const stxxl::vector &char_data) const +void ExtractionContainers::WriteCharData(const std::string &file_name) { std::cout << "[extractor] writing street name index ... " << std::flush; TIMER_START(write_index); boost::filesystem::ofstream file_stream(file_name, std::ios::binary); - unsigned total_length = 0; - - for (const auto length : offsets) + // transforms in-place name offsets to name lengths + BOOST_ASSERT(!name_offsets.empty()); + for (auto curr = name_offsets.begin(), next = name_offsets.begin() + 1; + next != name_offsets.end(); ++curr, ++next) { - total_length += length; + *curr = *next - *curr; } + // removes the total length sentinel + unsigned total_length = name_offsets.back(); + name_offsets.pop_back(); + // builds and writes the index - util::RangeTable<> index_range(offsets); + util::RangeTable<> index_range(name_offsets); file_stream << index_range; file_stream.write((char *)&total_length, sizeof(unsigned)); @@ -150,7 +221,7 @@ void ExtractionContainers::WriteCharData(const std::string &file_name, char write_buffer[WRITE_BLOCK_BUFFER_SIZE]; unsigned buffer_len = 0; - for (const auto c : char_data) + for (const auto c : name_char_data) { write_buffer[buffer_len++] = c; @@ -416,7 +487,7 @@ void ExtractionContainers::PrepareEdges(lua_State *segment_state) TIMER_START(sort_edges_by_renumbered_start); stxxl::sort(all_edges_list.begin(), all_edges_list.end(), - CmpEdgeByInternalStartThenInternalTargetID(), + CmpEdgeByInternalSourceTargetAndName{name_char_data, name_offsets}, stxxl_memory); TIMER_STOP(sort_edges_by_renumbered_start); std::cout << "ok, after " << TIMER_SEC(sort_edges_by_renumbered_start) << "s" << std::endl; @@ -453,11 +524,13 @@ void ExtractionContainers::PrepareEdges(lua_State *segment_state) all_edges_list[i].result.weight < min_forward_weight) { min_forward_idx = i; + min_forward_weight = all_edges_list[i].result.weight; } if (all_edges_list[i].result.backward && all_edges_list[i].result.weight < min_backward_weight) { min_backward_idx = i; + min_backward_weight = all_edges_list[i].result.weight; } // this also increments the outer loop counter! diff --git a/src/extractor/extractor_callbacks.cpp b/src/extractor/extractor_callbacks.cpp index fd03ca7e2..4287255be 100644 --- a/src/extractor/extractor_callbacks.cpp +++ b/src/extractor/extractor_callbacks.cpp @@ -1,8 +1,8 @@ +#include "extractor/extractor_callbacks.hpp" #include "extractor/external_memory_node.hpp" #include "extractor/extraction_containers.hpp" #include "extractor/extraction_node.hpp" #include "extractor/extraction_way.hpp" -#include "extractor/extractor_callbacks.hpp" #include "extractor/restriction.hpp" #include "util/for_each_pair.hpp" @@ -215,7 +215,7 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti // convert the lane description into an ID and, if necessary, remembr the description in the // description_map const auto requestId = [&](std::string lane_string) { - if( lane_string.empty() ) + if (lane_string.empty()) return INVALID_LANE_DESCRIPTIONID; TurnLaneDescription lane_description = laneStringToDescription(std::move(lane_string)); @@ -253,33 +253,35 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti // Get the unique identifier for the street name // Get the unique identifier for the street name and destination const auto name_iterator = string_map.find(MapKey(parsed_way.name, parsed_way.destinations)); - unsigned name_id = external_memory.name_lengths.size(); + unsigned name_id = EMPTY_NAMEID; if (string_map.end() == name_iterator) { - auto name_length = std::min(MAX_STRING_LENGTH, parsed_way.name.size()); - auto destinations_length = + const auto name_length = std::min(MAX_STRING_LENGTH, parsed_way.name.size()); + const auto destinations_length = std::min(MAX_STRING_LENGTH, parsed_way.destinations.size()); - auto pronunciation_length = + const auto pronunciation_length = std::min(MAX_STRING_LENGTH, parsed_way.pronunciation.size()); - external_memory.name_char_data.reserve(name_id + name_length + destinations_length + - pronunciation_length); + // name_offsets already has an offset of a new name, take the offset index as the name id + name_id = external_memory.name_offsets.size() - 1; + + external_memory.name_char_data.reserve(external_memory.name_char_data.size() + name_length + + destinations_length + pronunciation_length); std::copy(parsed_way.name.c_str(), parsed_way.name.c_str() + name_length, std::back_inserter(external_memory.name_char_data)); + external_memory.name_offsets.push_back(external_memory.name_char_data.size()); std::copy(parsed_way.destinations.c_str(), parsed_way.destinations.c_str() + destinations_length, std::back_inserter(external_memory.name_char_data)); + external_memory.name_offsets.push_back(external_memory.name_char_data.size()); std::copy(parsed_way.pronunciation.c_str(), parsed_way.pronunciation.c_str() + pronunciation_length, std::back_inserter(external_memory.name_char_data)); - - external_memory.name_lengths.push_back(name_length); - external_memory.name_lengths.push_back(destinations_length); - external_memory.name_lengths.push_back(pronunciation_length); + external_memory.name_offsets.push_back(external_memory.name_char_data.size()); auto k = MapKey{parsed_way.name, parsed_way.destinations}; auto v = MapVal{name_id};