From b15f973c4ac8d7105bce0f4990bcfb886eef7ba7 Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Thu, 25 Jun 2015 16:31:54 +0300 Subject: [PATCH] Async mode for OSRM online absent country checker. --- integration_tests/online_cross_tests.cpp | 8 ++ integration_tests/osrm_test_tools.cpp | 32 ++++++- integration_tests/osrm_test_tools.hpp | 5 +- map/framework.cpp | 5 +- map/routing_session.cpp | 6 +- map/routing_session.hpp | 2 +- routing/async_router.cpp | 102 +++++++++++++++-------- routing/async_router.hpp | 10 ++- routing/online_absent_fetcher.cpp | 32 +++++++ routing/online_absent_fetcher.hpp | 25 ++++++ routing/online_cross_fetcher.cpp | 15 ++-- routing/online_cross_fetcher.hpp | 18 ++-- routing/osrm_router.cpp | 37 -------- routing/router.hpp | 1 + routing/routing.pro | 2 + 15 files changed, 208 insertions(+), 92 deletions(-) create mode 100644 routing/online_absent_fetcher.cpp create mode 100644 routing/online_absent_fetcher.hpp diff --git a/integration_tests/online_cross_tests.cpp b/integration_tests/online_cross_tests.cpp index 248186054d..d7059693bf 100644 --- a/integration_tests/online_cross_tests.cpp +++ b/integration_tests/online_cross_tests.cpp @@ -4,6 +4,14 @@ namespace { +// Separately tests only fetcher. Other tests will test both fetcher and raw code. +UNIT_TEST(OnlineCrossFetcherSmokeTest) +{ + integration::OsrmRouterComponents & routerComponents = integration::GetAllMaps(); + TestOnlineFetcher({34.45, 61.76}, {38.94, 45.07}, + {"Russia_Central", "Russia_Southern", "Russia_Northwestern"}, routerComponents); +} + UNIT_TEST(OnlineRussiaNorthToSouthTest) { integration::OsrmRouterComponents & routerComponents = integration::GetAllMaps(); diff --git a/integration_tests/osrm_test_tools.cpp b/integration_tests/osrm_test_tools.cpp index 0550d1d0e7..1fdc298b9e 100644 --- a/integration_tests/osrm_test_tools.cpp +++ b/integration_tests/osrm_test_tools.cpp @@ -5,6 +5,7 @@ #include "map/feature_vec_model.hpp" #include "routing/online_cross_fetcher.hpp" +#include "routing/online_absent_fetcher.hpp" #include "routing/route.hpp" #include "search/search_engine.hpp" @@ -267,11 +268,39 @@ namespace integration return TestTurn(); } + void TestOnlineFetcher(m2::PointD const & startPoint, m2::PointD const & finalPoint, + vector const & expected, OsrmRouterComponents & routerComponents) + { + search::Engine * searchEngine(routerComponents.GetSearchEngine()); + routing::OnlineAbsentFetcher fetcher([&searchEngine](m2::PointD const & pt) + { + return searchEngine->GetCountryFile(pt); + }); + fetcher.GenerateRequest(MercatorBounds::FromLatLon(startPoint.y, startPoint.x), + MercatorBounds::FromLatLon(finalPoint.y, finalPoint.x)); + vector absent; + fetcher.GetAbsentCountries(absent); + if (expected.size() < 2) + { + // Single MWM case. Do not use online routing. + TEST(absent.empty(), ()); + return; + } + TEST_EQUAL(absent.size(), expected.size(), ()); + for (string const & name : expected) + { + TEST(find(absent.begin(), absent.end(), name) != absent.end(), ("Can't find ", name)); + } + } + void TestOnlineCrosses(m2::PointD const & startPoint, m2::PointD const & finalPoint, vector const & expected, OsrmRouterComponents & routerComponents) { - routing::OnlineCrossFetcher fetcher(OSRM_ONLINE_SERVER_URL, startPoint, finalPoint); + routing::OnlineCrossFetcher fetcher(OSRM_ONLINE_SERVER_URL, + MercatorBounds::FromLatLon(startPoint.y, startPoint.x), + MercatorBounds::FromLatLon(finalPoint.y, finalPoint.x)); + fetcher.Do(); vector const & points = fetcher.GetMwmPoints(); TEST_EQUAL(points.size(), expected.size(), ()); for (m2::PointD const & point : points) @@ -279,5 +308,6 @@ namespace integration string const mwmName = routerComponents.GetSearchEngine()->GetCountryFile(point); TEST(find(expected.begin(), expected.end(), mwmName) != expected.end(), ("Can't find ", mwmName)); } + TestOnlineFetcher(startPoint, finalPoint, expected, routerComponents); } } diff --git a/integration_tests/osrm_test_tools.hpp b/integration_tests/osrm_test_tools.hpp index b6d23146d7..31148491f6 100644 --- a/integration_tests/osrm_test_tools.hpp +++ b/integration_tests/osrm_test_tools.hpp @@ -44,8 +44,9 @@ namespace integration class OsrmRouterComponents; void TestOnlineCrosses(m2::PointD const & startPoint, m2::PointD const & finalPoint, - vector const & expected, - OsrmRouterComponents & routerComponents); + vector const & expected, OsrmRouterComponents & routerComponents); + void TestOnlineFetcher(m2::PointD const & startPoint, m2::PointD const & finalPoint, + vector const & expected, OsrmRouterComponents & routerComponents); OsrmRouterComponents & GetAllMaps(); shared_ptr LoadMaps(vector const & localFiles); diff --git a/map/framework.cpp b/map/framework.cpp index c197e24c6b..7c118938d9 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -18,6 +18,7 @@ #include "defines.hpp" +#include "routing/online_absent_fetcher.hpp" #include "routing/osrm_router.hpp" #include "routing/road_graph_router.hpp" #include "routing/route.hpp" @@ -2157,6 +2158,7 @@ void Framework::SetRouter(RouterType type) }; unique_ptr router; + unique_ptr fetcher; if (type == RouterType::Pedestrian) { router = CreatePedestrianAStarBidirectionalRouter(m_model.GetIndex(), countryFileGetter, @@ -2166,9 +2168,10 @@ void Framework::SetRouter(RouterType type) { router.reset(new OsrmRouter(&m_model.GetIndex(), countryFileGetter, localFileGetter, routingVisualizerFn)); + fetcher.reset(new OnlineAbsentFetcher(countryFileFn)); } - m_routingSession.SetRouter(move(router), routingStatisticsFn); + m_routingSession.SetRouter(move(router), move(fetcher), routingStatisticsFn); } void Framework::RemoveRoute() diff --git a/map/routing_session.cpp b/map/routing_session.cpp index 431f13ee00..fbf7571949 100644 --- a/map/routing_session.cpp +++ b/map/routing_session.cpp @@ -191,9 +191,11 @@ void RoutingSession::AssignRoute(Route & route) m_route.Swap(route); } -void RoutingSession::SetRouter(unique_ptr && router, TRoutingStatisticsCallback const & routingStatisticsFn) +void RoutingSession::SetRouter(unique_ptr && router, + unique_ptr && fetcher, + TRoutingStatisticsCallback const & routingStatisticsFn) { - m_router.reset(new AsyncRouter(move(router), routingStatisticsFn)); + m_router.reset(new AsyncRouter(move(router), move(fetcher), routingStatisticsFn)); } void RoutingSession::MatchLocationToRoute(location::GpsInfo & location, diff --git a/map/routing_session.hpp b/map/routing_session.hpp index 98a7606ae3..b4b605fdb3 100644 --- a/map/routing_session.hpp +++ b/map/routing_session.hpp @@ -52,7 +52,7 @@ public: RoutingSession(); - void SetRouter(unique_ptr && router, + void SetRouter(unique_ptr && router, unique_ptr && fetcher, TRoutingStatisticsCallback const & routingStatisticsFn); /// @param[in] startPoint and endPoint in mercator diff --git a/routing/async_router.cpp b/routing/async_router.cpp index dcb3cdb34b..686315a312 100644 --- a/routing/async_router.cpp +++ b/routing/async_router.cpp @@ -29,6 +29,7 @@ string ToString(IRouter::ResultCode code) case IRouter::PointsInDifferentMWM: return "PointsInDifferentMWM"; case IRouter::RouteNotFound: return "RouteNotFound"; case IRouter::InternalError: return "InternalError"; + case IRouter::NeedMoreMaps: return "NeedMoreMaps"; default: { ASSERT(false, ("Unknown IRouter::ResultCode value ", code)); @@ -58,10 +59,11 @@ map PrepareStatisticsData(string const & routerName, } // namespace -AsyncRouter::AsyncRouter(unique_ptr && router, +AsyncRouter::AsyncRouter(unique_ptr && router, unique_ptr && fetcher, TRoutingStatisticsCallback const & routingStatisticsFn) - : m_router(move(router)) - , m_routingStatisticsFn(routingStatisticsFn) + : m_absentFetcher(move(fetcher)), + m_router(move(router)), + m_routingStatisticsFn(routingStatisticsFn) { m_isReadyThread.clear(); } @@ -95,6 +97,41 @@ void AsyncRouter::ClearState() m_router->ClearState(); } +void AsyncRouter::LogCode(IRouter::ResultCode code, double const elapsedSec) +{ + switch (code) + { + case IRouter::StartPointNotFound: + LOG(LWARNING, ("Can't find start or end node")); + break; + case IRouter::EndPointNotFound: + LOG(LWARNING, ("Can't find end point node")); + break; + case IRouter::PointsInDifferentMWM: + LOG(LWARNING, ("Points are in different MWMs")); + break; + case IRouter::RouteNotFound: + LOG(LWARNING, ("Route not found")); + break; + case IRouter::RouteFileNotExist: + LOG(LWARNING, ("There is no routing file")); + break; + case IRouter::NeedMoreMaps: + LOG(LINFO, + ("Routing can find a better way with additional maps, elapsed seconds:", elapsedSec)); + break; + case IRouter::Cancelled: + LOG(LINFO, ("Route calculation cancelled, elapsed seconds:", elapsedSec)); + break; + case IRouter::NoError: + LOG(LINFO, ("Route found, elapsed seconds:", elapsedSec)); + break; + + default: + break; + } +} + void AsyncRouter::CalculateRouteImpl(TReadyCallback const & callback) { if (m_isReadyThread.test_and_set()) @@ -118,44 +155,20 @@ void AsyncRouter::CalculateRouteImpl(TReadyCallback const & callback) m_router->Reset(); } + my::Timer timer; + timer.Reset(); try { LOG(LDEBUG, ("Calculating the route from", startPoint, "to", finalPoint, "startDirection", startDirection)); - my::Timer timer; - timer.Reset(); + m_absentFetcher->GenerateRequest(startPoint, finalPoint); + // Run basic request code = m_router->CalculateRoute(startPoint, startDirection, finalPoint, route); double const elapsedSec = timer.ElapsedSeconds(); - switch (code) - { - case IRouter::StartPointNotFound: - LOG(LWARNING, ("Can't find start or end node")); - break; - case IRouter::EndPointNotFound: - LOG(LWARNING, ("Can't find end point node")); - break; - case IRouter::PointsInDifferentMWM: - LOG(LWARNING, ("Points are in different MWMs")); - break; - case IRouter::RouteNotFound: - LOG(LWARNING, ("Route not found")); - break; - case IRouter::RouteFileNotExist: - LOG(LWARNING, ("There is no routing file")); - break; - case IRouter::Cancelled: - LOG(LINFO, ("Route calculation cancelled, elapsed seconds:", elapsedSec)); - break; - case IRouter::NoError: - LOG(LINFO, ("Route found, elapsed seconds:", elapsedSec)); - break; - - default: - break; - } + LogCode(code, elapsedSec); SendStatistics(startPoint, startDirection, finalPoint, code, elapsedSec); } @@ -167,7 +180,30 @@ void AsyncRouter::CalculateRouteImpl(TReadyCallback const & callback) SendStatistics(startPoint, startDirection, finalPoint, e.Msg()); } - GetPlatform().RunOnGuiThread(bind(callback, route, code)); + if (code == IRouter::NoError) + GetPlatform().RunOnGuiThread(bind(callback, route, code)); + + // Check online response if we have. + vector absent; + m_absentFetcher->GetAbsentCountries(absent); + if (absent.empty() && code != IRouter::NoError) + { + GetPlatform().RunOnGuiThread(bind(callback, route, code)); + return; + } + for (string const & country : absent) + route.AddAbsentCountry(country); + if (code != IRouter::NoError) + { + GetPlatform().RunOnGuiThread(bind(callback, route, code)); + } + else + { + double const elapsedSec = timer.ElapsedSeconds(); + LogCode(IRouter::NeedMoreMaps, elapsedSec); + SendStatistics(startPoint, startDirection, finalPoint, IRouter::NeedMoreMaps, elapsedSec); + GetPlatform().RunOnGuiThread(bind(callback, route, IRouter::NeedMoreMaps)); + } } void AsyncRouter::SendStatistics(m2::PointD const & startPoint, m2::PointD const & startDirection, diff --git a/routing/async_router.hpp b/routing/async_router.hpp index dc934ffb33..41ebb52c67 100644 --- a/routing/async_router.hpp +++ b/routing/async_router.hpp @@ -1,7 +1,8 @@ #pragma once -#include "routing/route.hpp" -#include "routing/router.hpp" +#include "online_absent_fetcher.hpp" +#include "route.hpp" +#include "router.hpp" #include "std/atomic.hpp" #include "std/map.hpp" @@ -24,7 +25,7 @@ public: /// AsyncRouter is a wrapper class to run routing routines in the different thread /// @param router pointer to the router implementation. AsyncRouter will take ownership over /// router. - AsyncRouter(unique_ptr && router, + AsyncRouter(unique_ptr && router, unique_ptr && fetcher, TRoutingStatisticsCallback const & routingStatisticsFn); virtual ~AsyncRouter(); @@ -55,6 +56,8 @@ private: m2::PointD const & finalPoint, string const & exceptionMessage); + void LogCode(IRouter::ResultCode code, double const elapsedSec); + mutex m_paramsMutex; mutex m_routeMutex; atomic_flag m_isReadyThread; @@ -63,6 +66,7 @@ private: m2::PointD m_finalPoint; m2::PointD m_startDirection; + unique_ptr const m_absentFetcher; unique_ptr const m_router; TRoutingStatisticsCallback const m_routingStatisticsFn; }; diff --git a/routing/online_absent_fetcher.cpp b/routing/online_absent_fetcher.cpp new file mode 100644 index 0000000000..0f73f97377 --- /dev/null +++ b/routing/online_absent_fetcher.cpp @@ -0,0 +1,32 @@ +#include "online_absent_fetcher.hpp" + +#include "defines.hpp" +#include "online_cross_fetcher.hpp" + +namespace routing +{ +void OnlineAbsentFetcher::GenerateRequest(const m2::PointD & startPoint, + const m2::PointD & finalPoint) +{ + // single mwm case + if (m_countryFunction(startPoint) == m_countryFunction(finalPoint)) + return; + unique_ptr fetcher = + make_unique(OSRM_ONLINE_SERVER_URL, startPoint, finalPoint); + m_fetcherThread.Create(move(fetcher)); +} + +void OnlineAbsentFetcher::GetAbsentCountries(vector & countries) +{ + // Have task check + if (!m_fetcherThread.GetRoutine()) + return; + m_fetcherThread.Join(); + for (auto point : static_cast(m_fetcherThread.GetRoutine())->GetMwmPoints()) + { + string name = m_countryFunction(point); + LOG(LINFO, ("Online recomends to download: ", name)); + countries.emplace_back(move(name)); + } +} +} // namespace routing diff --git a/routing/online_absent_fetcher.hpp b/routing/online_absent_fetcher.hpp new file mode 100644 index 0000000000..538f9ef926 --- /dev/null +++ b/routing/online_absent_fetcher.hpp @@ -0,0 +1,25 @@ +#pragma once + +#include "route.hpp" +#include "router.hpp" +#include "routing_mapping.h" + +#include "base/thread.hpp" + +#include "std/string.hpp" +#include "std/unique_ptr.hpp" + +namespace routing +{ +class OnlineAbsentFetcher +{ +public: + OnlineAbsentFetcher(TCountryFileFn const & fn) : m_countryFunction(fn) {} + void GenerateRequest(m2::PointD const & startPoint, m2::PointD const & finalPoint); + void GetAbsentCountries(vector & countries); + +private: + TCountryFileFn const m_countryFunction; + threads::Thread m_fetcherThread; +}; +} // namespace routing diff --git a/routing/online_cross_fetcher.cpp b/routing/online_cross_fetcher.cpp index 3412c19d7d..dc311f015f 100644 --- a/routing/online_cross_fetcher.cpp +++ b/routing/online_cross_fetcher.cpp @@ -7,6 +7,8 @@ #include "3party/jansson/myjansson.hpp" +#include "indexer/mercator.hpp" + #include "std/bind.hpp" namespace routing @@ -45,16 +47,19 @@ string GenerateOnlineRequest(string const & serverURL, m2::PointD const & startP OnlineCrossFetcher::OnlineCrossFetcher(string const & serverURL, m2::PointD const & startPoint, m2::PointD const & finalPoint) - : m_request(GenerateOnlineRequest(serverURL, startPoint, finalPoint)) + : m_request(GenerateOnlineRequest( + serverURL, {MercatorBounds::XToLon(startPoint.x), MercatorBounds::YToLat(startPoint.y)}, + {MercatorBounds::XToLon(finalPoint.x), MercatorBounds::YToLat(finalPoint.y)})) { LOG(LINFO, ("Check mwms by URL: ", GenerateOnlineRequest(serverURL, startPoint, finalPoint))); } -vector const & OnlineCrossFetcher::GetMwmPoints() +void OnlineCrossFetcher::Do() { m_mwmPoints.clear(); - if (m_request.RunHTTPRequest()) + if (m_request.RunHTTPRequest() && m_request.error_code() == 200 && !m_request.was_redirected()) ParseResponse(m_request.server_response(), m_mwmPoints); - return m_mwmPoints; -} + else + LOG(LWARNING, ("Can't get OSRM server response. Code: ", m_request.error_code())); } +} // namespace routing diff --git a/routing/online_cross_fetcher.hpp b/routing/online_cross_fetcher.hpp index 9e93e30875..6a713f65f7 100644 --- a/routing/online_cross_fetcher.hpp +++ b/routing/online_cross_fetcher.hpp @@ -4,6 +4,8 @@ #include "geometry/point2d.hpp" +#include "base/thread.hpp" + #include "std/string.hpp" #include "std/vector.hpp" @@ -25,21 +27,23 @@ string GenerateOnlineRequest(string const & serverURL, m2::PointD const & startP /// \return true if there are some maps in a server's response. bool ParseResponse(string const & serverResponse, vector & outPoints); -class OnlineCrossFetcher +class OnlineCrossFetcher : public threads::IRoutine { public: /// \brief OnlineCrossFetcher helper class to make request to online OSRM server /// and get mwm name list /// \param serverURL Server URL - /// \param startPoint Start point coordinate - /// \param finalPoint Finish point coordinate + /// \param startPoint Start point coordinates in mercator + /// \param finalPoint Finish point coordinates in mercator OnlineCrossFetcher(string const & serverURL, m2::PointD const & startPoint, m2::PointD const & finalPoint); - /// \brief getMwmPoints Waits for a server response, and returns mwm representation points list. - /// \return Mwm names to build route from startPt to finishPt. Empty list if there were errors. - /// \warning Can take a long time while waits a server response. - vector const & GetMwmPoints(); + /// Do synchronous (blocking) call of online OSRM server. Designing for call from another thread. + void Do() override; + + /// \brief GetMwmPoints returns mwm representation points list. + /// \return Mwm points to build route from startPt to finishPt. Empty list if there were errors. + vector const & GetMwmPoints() { return m_mwmPoints; } private: alohalytics::HTTPClientPlatformWrapper m_request; diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp index e0d3bd7531..3e9ac34cdf 100644 --- a/routing/osrm_router.cpp +++ b/routing/osrm_router.cpp @@ -49,38 +49,6 @@ using RawRouteData = InternalRouteResult; namespace { -class AbsentCountryChecker -{ -public: - AbsentCountryChecker(m2::PointD const & startPoint, m2::PointD const & finalPoint, - RoutingIndexManager & indexManager, Route & route) - : m_route(route), - m_indexManager(indexManager), - m_fetcher(OSRM_ONLINE_SERVER_URL, - {MercatorBounds::XToLon(startPoint.x), MercatorBounds::YToLat(startPoint.y)}, - {MercatorBounds::XToLon(finalPoint.x), MercatorBounds::YToLat(finalPoint.y)}) - { - } - - ~AbsentCountryChecker() - { - vector const & points = m_fetcher.GetMwmPoints(); - for (m2::PointD const & point : points) - { - TRoutingMappingPtr mapping = m_indexManager.GetMappingByPoint(point); - if (!mapping->IsValid()) - { - LOG(LINFO, ("Online recomends to download: ", mapping->GetName())); - m_route.AddAbsentCountry(mapping->GetName()); - } - } - } - -private: - Route & m_route; - RoutingIndexManager & m_indexManager; - OnlineCrossFetcher m_fetcher; -}; class Point2PhantomNode { @@ -553,11 +521,6 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, m2::PointD const & startDirection, m2::PointD const & finalPoint, Route & route) { -// Experimental feature -#if defined(DEBUG) - AbsentCountryChecker checker(startPoint, finalPoint, m_indexManager, route); - UNUSED_VALUE(checker); -#endif my::HighResTimer timer(true); TRoutingMappingPtr startMapping = m_indexManager.GetMappingByPoint(startPoint); TRoutingMappingPtr targetMapping = m_indexManager.GetMappingByPoint(finalPoint); diff --git a/routing/router.hpp b/routing/router.hpp index df3303f1be..26a9a8df4e 100644 --- a/routing/router.hpp +++ b/routing/router.hpp @@ -33,6 +33,7 @@ public: EndPointNotFound, PointsInDifferentMWM, RouteNotFound, + NeedMoreMaps, InternalError }; diff --git a/routing/routing.pro b/routing/routing.pro index fb9b4a1038..c60acd1632 100644 --- a/routing/routing.pro +++ b/routing/routing.pro @@ -19,6 +19,7 @@ SOURCES += \ cross_routing_context.cpp \ features_road_graph.cpp \ nearest_edge_finder.cpp \ + online_absent_fetcher.cpp \ online_cross_fetcher.cpp \ osrm2feature_map.cpp \ osrm_engine.cpp \ @@ -41,6 +42,7 @@ HEADERS += \ cross_routing_context.hpp \ features_road_graph.hpp \ nearest_edge_finder.hpp \ + online_absent_fetcher.hpp \ online_cross_fetcher.hpp \ osrm2feature_map.hpp \ osrm_data_facade.hpp \