diff --git a/features/testbot/distance_matrix.feature b/features/testbot/distance_matrix.feature index 7befc82e100..6e0aee97e41 100644 --- a/features/testbot/distance_matrix.feature +++ b/features/testbot/distance_matrix.feature @@ -25,80 +25,6 @@ Feature: Basic Distance Matrix Given the query options | exclude | toll | - Scenario: Testbot - Travel time matrix of minimal network with excludes - Given the query options - | exclude | toll | - - Given the node map - """ - a b - c d - """ - - And the ways - | nodes | highway | toll | # | - | ab | motorway | | not drivable for exclude=motorway | - | cd | primary | | always drivable | - | ac | highway | yes | not drivable for exclude=motorway exclude=toll and exclude=motorway,toll | - | bd | highway | yes | not drivable for exclude=motorway exclude=toll | - - When I request a travel time matrix I should get - | | a | b | c | d | - | a | 0 | 15 | | | - | b | 15 | 0 | | | - | c | | | 0 | 10 | - | d | | | 10 | 0 | - - Scenario: Testbot - Travel time matrix of minimal network with different exclude - Given the query options - | exclude | motorway | - - Given the node map - """ - a b - c d - """ - - And the ways - | nodes | highway | toll | # | - | ab | motorway | | not drivable for exclude=motorway | - | cd | primary | | always drivable | - | ac | highway | yes | not drivable for exclude=motorway exclude=toll and exclude=motorway,toll | - | bd | highway | yes | not drivable for exclude=motorway exclude=toll | - - - When I request a travel time matrix I should get - | | a | b | c | d | - | a | 0 | 40 | 15 | 25 | - | b | 40 | 0 | 25 | 15 | - | c | 15 | 25 | 0 | 10 | - | d | 25 | 15 | 10 | 0 | - - Scenario: Testbot - Travel time matrix of minimal network with excludes combination - Given the query options - | exclude | motorway,toll | - - Given the node map - """ - a b - c d - """ - - And the ways - | nodes | highway | toll | # | - | ab | motorway | | not drivable for exclude=motorway | - | cd | primary | | always drivable | - | ac | highway | yes | not drivable for exclude=motorway exclude=toll and exclude=motorway,toll | - | bd | highway | yes | not drivable for exclude=motorway exclude=toll | - - When I request a travel time matrix I should get - | | a | b | c | d | - | a | 0 | 10 | 0 | 10 | - | b | 10 | 0 | 10 | 0 | - | c | 0 | 10 | 0 | 10 | - | d | 10 | 0 | 10 | 0 | - - Scenario: Testbot - Travel time matrix with different way speeds Given the node map """ a b @@ -404,10 +330,10 @@ Feature: Basic Distance Matrix When I request a travel distance matrix I should get | | f | g | h | i | - | f | 0 | 300+-2 | 0 | 300+-2 | - | g | 300+-2 | 0 | 300+-2 | 0 | - | h | 0 | 300+-2 | 0 | 300+-2 | - | i | 300+-2 | 0 | 300+-2 | 0 | + | f | 0 | 300+-3 | 0 | 300+-3 | + | g | 300+-3 | 0 | 300+-3 | 0 | + | h | 0 | 300+-3 | 0 | 300+-3 | + | i | 300+-3 | 0 | 300+-3 | 0 | Scenario: Testbot - Travel distance matrix with loops Given the node map @@ -426,9 +352,9 @@ Feature: Basic Distance Matrix When I request a travel distance matrix I should get | | 1 | 2 | 3 | 4 | | 1 | 0 | 100+-1 | 400+-1 | 500+-1 | - | 2 | 700+-1 | 0 | 300+-1 | 400+-1 | + | 2 | 700+-2 | 0 | 300+-2 | 400+-1 | | 3 | 400+-1 | 500+-1 | 0 | 100+-1 | - | 4 | 300+-1 | 400+-1 | 700+-1 | 0 | + | 4 | 300+-2 | 400+-1 | 700+-3 | 0 | Scenario: Testbot - Travel distance matrix based on segment durations @@ -570,4 +496,4 @@ Feature: Basic Distance Matrix When I request a travel distance matrix I should get | | a | b | c | d | - | a | 0 | 1000+-3 | 2000+-3 | 3000+-3 | + | a | 0 | 1000+-3 | 2000+-3 | 3000+-3 | \ No newline at end of file diff --git a/include/engine/routing_algorithms/routing_base.hpp b/include/engine/routing_algorithms/routing_base.hpp index f5e2ae6caa0..12e23ee1e5f 100644 --- a/include/engine/routing_algorithms/routing_base.hpp +++ b/include/engine/routing_algorithms/routing_base.hpp @@ -177,7 +177,6 @@ void annotatePath(const FacadeT &facade, const auto geometry_index = facade.GetGeometryIndex(node_id); get_segment_geometry(geometry_index); - BOOST_ASSERT(id_vector.size() > 0); BOOST_ASSERT(datasource_vector.size() > 0); BOOST_ASSERT(weight_vector.size() + 1 == id_vector.size()); @@ -407,7 +406,6 @@ InternalRouteResult extractRoute(const DataFacade &facade, return raw_route_data; } - template EdgeDistance computeEdgeDistance(const FacadeT &facade, NodeID node_id) { const auto geometry_index = facade.GetGeometryIndex(node_id); diff --git a/include/engine/routing_algorithms/routing_base_ch.hpp b/include/engine/routing_algorithms/routing_base_ch.hpp index b8522e05bbd..e65c998675f 100644 --- a/include/engine/routing_algorithms/routing_base_ch.hpp +++ b/include/engine/routing_algorithms/routing_base_ch.hpp @@ -288,11 +288,12 @@ void unpackPath(const DataFacade &facade, } } } + template -EdgeDistance calculateEBGNodeDuration(const DataFacade &facade, - BidirectionalIterator packed_path_begin, - BidirectionalIterator packed_path_end, - UnpackingCache &unpacking_cache) +EdgeDuration calculateEBGNodeDuration(const DataFacade &facade, + BidirectionalIterator packed_path_begin, + BidirectionalIterator packed_path_end, + UnpackingCache &unpacking_cache) { // Make sure we have at least something to unpack if (packed_path_begin == packed_path_end || @@ -324,7 +325,7 @@ EdgeDistance calculateEBGNodeDuration(const DataFacade &facade, if (unpacking_cache.IsEdgeInCache(std::make_tuple( std::get<0>(edge), std::get<1>(edge), facade.GetExcludeIndex()))) { - EdgeDuration duration = unpacking_cache.GetDuration(std::make_tuple( + EdgeDuration duration = unpacking_cache.GetAnnotation(std::make_tuple( std::get<0>(edge), std::get<1>(edge), facade.GetExcludeIndex())); duration_stack.emplace(duration); } @@ -404,12 +405,11 @@ EdgeDistance calculateEBGNodeDuration(const DataFacade &facade, return total_duration; } - template EdgeDistance calculateEBGNodeDistance(const DataFacade &facade, - BidirectionalIterator packed_path_begin, - BidirectionalIterator packed_path_end, - UnpackingCache &unpacking_cache) + BidirectionalIterator packed_path_begin, + BidirectionalIterator packed_path_end, + UnpackingCache &unpacking_cache) { // Make sure we have at least something to unpack if (packed_path_begin == packed_path_end || @@ -417,6 +417,7 @@ EdgeDistance calculateEBGNodeDistance(const DataFacade &facade, return 0; std::stack> recursion_stack; + std::stack distance_stack; // We have to push the path in reverse order onto the stack because it's LIFO. for (auto current = std::prev(packed_path_end); current > packed_path_begin; @@ -442,7 +443,7 @@ EdgeDistance calculateEBGNodeDistance(const DataFacade &facade, if (unpacking_cache.IsEdgeInCache(std::make_tuple( std::get<0>(edge), std::get<1>(edge), facade.GetExcludeIndex()))) { - EdgeDuration distance = unpacking_cache.GetDistance(std::make_tuple( + EdgeDuration distance = unpacking_cache.GetAnnotation(std::make_tuple( std::get<0>(edge), std::get<1>(edge), facade.GetExcludeIndex())); distance_stack.emplace(distance); } diff --git a/include/engine/search_engine_data.hpp b/include/engine/search_engine_data.hpp index e9ab35bba8f..f1359d07fd7 100644 --- a/include/engine/search_engine_data.hpp +++ b/include/engine/search_engine_data.hpp @@ -47,7 +47,8 @@ template <> struct SearchEngineData using SearchEngineHeapPtr = boost::thread_specific_ptr; using ManyToManyHeapPtr = boost::thread_specific_ptr; - using UnpackingCachePtr = boost::thread_specific_ptr; + using DistanceCachePtr = boost::thread_specific_ptr>; + using DurationCachePtr = boost::thread_specific_ptr>; static SearchEngineHeapPtr forward_heap_1; static SearchEngineHeapPtr reverse_heap_1; @@ -56,7 +57,8 @@ template <> struct SearchEngineData static SearchEngineHeapPtr forward_heap_3; static SearchEngineHeapPtr reverse_heap_3; static ManyToManyHeapPtr many_to_many_heap; - static UnpackingCachePtr unpacking_cache; + static DistanceCachePtr distance_cache; + static DurationCachePtr duration_cache; void InitializeOrClearFirstThreadLocalStorage(unsigned number_of_nodes); @@ -66,7 +68,9 @@ template <> struct SearchEngineData void InitializeOrClearManyToManyThreadLocalStorage(unsigned number_of_nodes); - void InitializeOrClearUnpackingCacheThreadLocalStorage(unsigned timestamp); + void InitializeOrClearDistanceCacheThreadLocalStorage(unsigned timestamp); + + void InitializeOrClearDurationCacheThreadLocalStorage(unsigned timestamp); }; struct MultiLayerDijkstraHeapData diff --git a/include/engine/unpacking_cache.hpp b/include/engine/unpacking_cache.hpp index 0f7e61403f5..576e612280b 100644 --- a/include/engine/unpacking_cache.hpp +++ b/include/engine/unpacking_cache.hpp @@ -7,11 +7,6 @@ #include "../../third_party/compute_detail/lru_cache.hpp" #include "util/typedefs.hpp" - -// sizeof size_t: 8 -// sizeof unsigned: 4 -// sizeof unchar: 1 -// sizeof uint32: 4 namespace osrm { namespace engine @@ -20,63 +15,15 @@ typedef unsigned char ExcludeIndex; typedef unsigned Timestamp; typedef std::tuple Key; -class UnpackingCache +template class UnpackingCache { private: - boost::compute::detail::lru_cache, EdgeDuration> m_cache; + boost::compute::detail::lru_cache, AnnotationType> + m_cache; unsigned m_current_data_timestamp = 0; + const AnnotationType INVALID_EDGE_ANNOTATION = std::numeric_limits::max(); public: - // TO FIGURE OUT HOW MANY LINES TO INITIALIZE CACHE TO: - // Assume max cache size is 500mb (see bottom of OP here: - // https://github.com/Project-OSRM/osrm-backend/issues/4798#issue-288608332) - - // LRU CACHE IMPLEMENTATION HAS THESE TWO STORAGE CONTAINERS - // Key is of size: std::uint32_t * 2 + (unsigned char) * 1 - // = 4 * 2 + 1 * 1 = 9 - // map: n * Key + n * EdgeDuration - // = n * 9 bytes + n * std::int32_t - // = n * 9 bytes + n * 4 bytes - // = n * 13 bytes - // list: n * Key - // = n * 9 bytes - // Total = n * (13 + 9) = n * 22 bytes - // Total cache size: 500 mb = 500 * 1024 *1024 bytes = 524288000 bytes - - // THREAD LOCAL STORAGE (500 mb) - // Number of lines we need = 524288000 / 22 / number of threads = 23831272 / number of threads - // 16 threads: 23831272 / 16 = 1489454 - // 8 threads: 23831272 / 8 = 2978909 - // 4 threads: 23831272 / 4 = 5957818 - // 2 threads: 23831272 / 2 = 11915636 - - // THREAD LOCAL STORAGE (1024 mb) - // Number of lines we need = 1073741824 / 22 / number of threads = 48806446 / number of threads - // 16 threads: 48806446 / 16 = 3050402 - // 8 threads: 48806446 / 8 = 6100805 - // 4 threads: 48806446 / 4 = 12201611 - // 2 threads: 48806446 / 2 = 24403223 - - // LRU CACHE IMPLEMENTATION HAS THESE TWO STORAGE CONTAINERS - // Key is of size: std::uint32_t * 2 + (unsigned char) * 1 + unsigned * 1 - // = 4 * 2 + 1 * 1 + 4 * 1 = 13 - // map: n * Key + n * EdgeDuration - // = n * 13 bytes + n * std::int32_t - // = n * 13 bytes + n * 4 bytes - // = n * 17 bytes - // list: n * Key - // = n * 13 bytes - // Total = n * (17 + 13) = n * 30 bytes - // Total cache size: 500 mb = 500 * 1024 *1024 bytes = 524288000 bytes - // Total cache size: 1024 mb = 1024 * 1024 *1024 bytes = 1073741824 bytes - // Total cache size: 250 mb = 250 * 1024 *1024 bytes = 262144000 bytes - - // SHARED STORAGE CACHE - // Number of lines for shared storage cache 1024 mb = 524288000 / 30 = 17476266 - // Number of lines for shared storage cache 500 mb = 1073741824 / 30 = 35791394 - // Number of lines for shared storage cache 250 mb = 262144000 / 30 = 8738133 - - UnpackingCache(unsigned timestamp) : m_cache(8738133), m_current_data_timestamp(timestamp){}; UnpackingCache(std::size_t cache_size, unsigned timestamp) @@ -91,20 +38,20 @@ class UnpackingCache } } - bool IsEdgeInCache(std::tuple edge) + bool IsEdgeInCache(std::tuple edge) { return m_cache.contains(edge); } - void AddEdge(std::tuple edge, EdgeDuration duration) + void AddEdge(std::tuple edge, AnnotationType annotation) { - m_cache.insert(edge, duration); + m_cache.insert(edge, annotation); } - EdgeDuration GetDuration(std::tuple edge) + AnnotationType GetAnnotation(std::tuple edge) { - boost::optional duration = m_cache.get(edge); - return duration ? *duration : MAXIMAL_EDGE_DURATION; + boost::optional annotation = m_cache.get(edge); + return annotation ? *annotation : INVALID_EDGE_ANNOTATION; } }; } // engine diff --git a/src/engine/routing_algorithms/many_to_many_ch.cpp b/src/engine/routing_algorithms/many_to_many_ch.cpp index 6353c2fab0d..6b976d7e343 100644 --- a/src/engine/routing_algorithms/many_to_many_ch.cpp +++ b/src/engine/routing_algorithms/many_to_many_ch.cpp @@ -198,7 +198,8 @@ void calculateDistances(typename SearchEngineData::ManyToManyQuer const std::size_t number_of_targets, const std::vector &search_space_with_buckets, std::vector &distances_table, - const std::vector &middle_nodes_table) + const std::vector &middle_nodes_table, + const SearchEngineData &engine_working_data) { std::vector packed_leg; @@ -240,8 +241,10 @@ void calculateDistances(typename SearchEngineData::ManyToManyQuer } if (!packed_leg.empty()) { - auto annotation = - ch::calculateEBGNodeAnnotations(facade, packed_leg.begin(), packed_leg.end()); + auto annotation = ch::calculateEBGNodeDistance(facade, + packed_leg.begin(), + packed_leg.end(), + *(engine_working_data.distance_cache)); distances_table[row_index * number_of_targets + column_index] = annotation; @@ -337,9 +340,6 @@ manyToManySearch(SearchEngineData &engine_working_data, std::vector search_space_with_buckets; std::vector packed_leg; - engine_working_data.InitializeOrClearUnpackingCacheThreadLocalStorage( - facade.GetTimestamp()); // always pass in the timestamp and clear if it's different - // Populate buckets with paths from all accessible nodes to destinations via backward searches for (std::uint32_t column_index = 0; column_index < target_indices.size(); ++column_index) { @@ -390,6 +390,9 @@ manyToManySearch(SearchEngineData &engine_working_data, if (calculate_distance) { + engine_working_data.InitializeOrClearDistanceCacheThreadLocalStorage( + facade.GetTimestamp()); // always pass in the timestamp and clear if it's different + distances_table.resize(number_of_entries, INVALID_EDGE_DISTANCE); calculateDistances(query_heap, facade, @@ -401,7 +404,8 @@ manyToManySearch(SearchEngineData &engine_working_data, number_of_targets, search_space_with_buckets, distances_table, - middle_nodes_table); + middle_nodes_table, + engine_working_data); } } diff --git a/src/engine/search_engine_data.cpp b/src/engine/search_engine_data.cpp index 70231b6ee71..438735568b1 100644 --- a/src/engine/search_engine_data.cpp +++ b/src/engine/search_engine_data.cpp @@ -14,7 +14,8 @@ SearchEngineData::SearchEngineHeapPtr SearchEngineData::reverse_heap_2; SearchEngineData::SearchEngineHeapPtr SearchEngineData::forward_heap_3; SearchEngineData::SearchEngineHeapPtr SearchEngineData::reverse_heap_3; SearchEngineData::ManyToManyHeapPtr SearchEngineData::many_to_many_heap; -SearchEngineData::UnpackingCachePtr SearchEngineData::unpacking_cache; +SearchEngineData::DistanceCachePtr SearchEngineData::distance_cache; +SearchEngineData::DurationCachePtr SearchEngineData::duration_cache; void SearchEngineData::InitializeOrClearFirstThreadLocalStorage(unsigned number_of_nodes) { @@ -91,15 +92,27 @@ void SearchEngineData::InitializeOrClearManyToManyThreadLocalStorage(unsigne } } -void SearchEngineData::InitializeOrClearUnpackingCacheThreadLocalStorage(unsigned timestamp) +void SearchEngineData::InitializeOrClearDistanceCacheThreadLocalStorage(unsigned timestamp) { - if (unpacking_cache.get()) + if (distance_cache.get()) { - unpacking_cache->Clear(timestamp); + distance_cache->Clear(timestamp); } else { - unpacking_cache.reset(new UnpackingCache(timestamp)); + distance_cache.reset(new UnpackingCache(timestamp)); + } +} + +void SearchEngineData::InitializeOrClearDurationCacheThreadLocalStorage(unsigned timestamp) +{ + if (duration_cache.get()) + { + duration_cache->Clear(timestamp); + } + else + { + duration_cache.reset(new UnpackingCache(timestamp)); } } diff --git a/unit_tests/engine/unpacking_cache.cpp b/unit_tests/engine/unpacking_cache.cpp index d0a359a074d..b784ece087f 100644 --- a/unit_tests/engine/unpacking_cache.cpp +++ b/unit_tests/engine/unpacking_cache.cpp @@ -16,7 +16,7 @@ BOOST_AUTO_TEST_CASE(add_edge_and_check_existence) { // Arrange (Setup) unsigned timestamp = 1522782542; - UnpackingCache cache(1, timestamp); + UnpackingCache cache(1, timestamp); auto key = std::make_tuple(1, 1, 1); auto value = 1; @@ -28,7 +28,7 @@ BOOST_AUTO_TEST_CASE(add_edge_and_check_existence) BOOST_CHECK(cache.IsEdgeInCache(key) == true); BOOST_CHECK(cache.IsEdgeInCache(std::make_tuple(2, 2, 2)) == false); - auto result = cache.GetDuration(key); + auto result = cache.GetAnnotation(key); BOOST_CHECK_EQUAL(result, value); } @@ -36,7 +36,7 @@ BOOST_AUTO_TEST_CASE(cache_invalidation) { // Arrange (Setup) unsigned timestamp = 1522782542; - UnpackingCache cache(1, timestamp); + UnpackingCache cache(1, timestamp); auto key1 = std::make_tuple(1, 1, 1); auto value1 = 1; @@ -49,20 +49,54 @@ BOOST_AUTO_TEST_CASE(cache_invalidation) cache.AddEdge(key2, value2); // Assert - auto result = cache.GetDuration(key1); - BOOST_CHECK_EQUAL(result, MAXIMAL_EDGE_DURATION); + auto result = cache.GetAnnotation(key1); + BOOST_CHECK_EQUAL(result, INVALID_EDGE_DISTANCE); - result = cache.GetDuration(key2); + result = cache.GetAnnotation(key2); BOOST_CHECK_EQUAL(result, value2); } +BOOST_AUTO_TEST_CASE(type_check) +{ + // Arrange (Setup) + unsigned timestamp = 1522782542; + + UnpackingCache distance_cache(1, timestamp); + UnpackingCache duration_cache(1, timestamp); + + auto key1 = std::make_tuple(1, 1, 1); + auto value1 = 1; + + auto key2 = std::make_tuple(2, 2, 2); + auto value2 = 2; + + // Act + distance_cache.AddEdge(key1, value1); + distance_cache.AddEdge(key2, value2); + duration_cache.AddEdge(key1, value1); + duration_cache.AddEdge(key2, value2); + + // Assert + auto result_distance = distance_cache.GetAnnotation(key1); + BOOST_CHECK_EQUAL(result_distance, std::numeric_limits::max()); + + result_distance = distance_cache.GetAnnotation(key2); + BOOST_CHECK_EQUAL(result_distance, value2); + + auto result_duration = duration_cache.GetAnnotation(key1); + BOOST_CHECK_EQUAL(result_duration, std::numeric_limits::max()); + + result_duration = duration_cache.GetAnnotation(key2); + BOOST_CHECK_EQUAL(result_duration, value2); +} + BOOST_AUTO_TEST_CASE(new_data) { // Arrange (Setup) unsigned timestamp1 = 1522782542; unsigned timestamp2 = 1522782543; - UnpackingCache cache(1, timestamp1); + UnpackingCache cache(1, timestamp1); auto key1 = std::make_tuple(1, 2, 3); auto value1 = 1; @@ -79,7 +113,7 @@ BOOST_AUTO_TEST_CASE(new_data) BOOST_CHECK(cache.IsEdgeInCache(key2) == true); BOOST_CHECK(cache.IsEdgeInCache(std::make_tuple(2, 2, 2)) == false); - auto result = cache.GetDuration(key2); + auto result = cache.GetAnnotation(key2); BOOST_CHECK_EQUAL(result, value2); }