From 0b6a49ad8f72d4cfd0f31ffb9fd02b6b67a459df Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Wed, 8 Jul 2015 10:58:13 +0300 Subject: [PATCH] Mwm locking routines for OSRM mapping. --- integration_tests/routing_test_tools.cpp | 13 ++-- map/feature_vec_model.hpp | 1 + map/framework.cpp | 2 +- .../pedestrian_routing_benchmarks.cpp | 7 ++- routing/cross_mwm_road_graph.cpp | 3 +- routing/online_absent_fetcher.hpp | 2 + routing/osrm_router.cpp | 15 +++-- routing/osrm_router.hpp | 3 +- routing/road_graph_router.cpp | 13 ++-- routing/road_graph_router.hpp | 8 +-- routing/routing_mapping.cpp | 60 +++++-------------- routing/routing_mapping.h | 25 ++++---- .../routing_tests/routing_mapping_test.cpp | 0 13 files changed, 62 insertions(+), 90 deletions(-) create mode 100644 routing/routing_tests/routing_mapping_test.cpp diff --git a/integration_tests/routing_test_tools.cpp b/integration_tests/routing_test_tools.cpp index 6df5720f5a..292f5fe323 100644 --- a/integration_tests/routing_test_tools.cpp +++ b/integration_tests/routing_test_tools.cpp @@ -85,21 +85,18 @@ namespace integration } } - shared_ptr CreateOsrmRouter(Index const & index, search::Engine & searchEngine) + shared_ptr CreateOsrmRouter(Index & index, search::Engine & searchEngine) { shared_ptr osrmRouter(new OsrmRouter( &index, [&searchEngine](m2::PointD const & pt) { return searchEngine.GetCountryFile(pt); - }, - [](string const & countryFileName) - { - return make_shared(LocalCountryFile::MakeForTesting(countryFileName)); - })); + } + )); return osrmRouter; } - shared_ptr CreatePedestrianRouter(Index const & index, search::Engine & searchEngine) + shared_ptr CreatePedestrianRouter(Index & index, search::Engine & searchEngine) { auto const countryFileFn = [&searchEngine](m2::PointD const & pt) { @@ -169,6 +166,8 @@ namespace integration vector localFiles; platform::FindAllLocalMaps(localFiles); + for (auto & file : localFiles) + file.SyncWithDisk(); ASSERT(!localFiles.empty(), ()); return shared_ptr(new TRouterComponents(localFiles)); } diff --git a/map/feature_vec_model.hpp b/map/feature_vec_model.hpp index 9eef962fff..f7e21a575a 100644 --- a/map/feature_vec_model.hpp +++ b/map/feature_vec_model.hpp @@ -94,6 +94,7 @@ class FeaturesFetcher : public Index::Observer //@} Index const & GetIndex() const { return m_multiIndex; } + Index & GetIndex() { return m_multiIndex; } m2::RectD GetWorldRect() const; }; } diff --git a/map/framework.cpp b/map/framework.cpp index 05d8749483..d625ea510b 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -2177,7 +2177,7 @@ void Framework::SetRouter(RouterType type) } else { - router.reset(new OsrmRouter(&m_model.GetIndex(), countryFileGetter, localFileGetter, + router.reset(new OsrmRouter(&m_model.GetIndex(), countryFileGetter, routingVisualizerFn)); fetcher.reset(new OnlineAbsentCountriesFetcher(countryFileGetter, localFileGetter)); } diff --git a/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp b/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp index bddba3e8ba..98d5730fec 100644 --- a/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp +++ b/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp @@ -58,7 +58,8 @@ private: shared_ptr const m_model; }; -unique_ptr CreatePedestrianAStarTestRouter(Index const & index, routing::TMwmFileByPointFn const & countryFileFn) +unique_ptr CreatePedestrianAStarTestRouter(Index & index, routing::TMwmFileByPointFn const & countryFileFn) + { unique_ptr vehicleModelFactory(new SimplifiedPedestrianModelFactory()); unique_ptr algorithm(new routing::AStarRoutingAlgorithm(nullptr)); @@ -66,7 +67,7 @@ unique_ptr CreatePedestrianAStarTestRouter(Index const & index return router; } -unique_ptr CreatePedestrianAStarBidirectionalTestRouter(Index const & index, routing::TMwmFileByPointFn const & countryFileFn) +unique_ptr CreatePedestrianAStarBidirectionalTestRouter(Index & index, routing::TMwmFileByPointFn const & countryFileFn) { unique_ptr vehicleModelFactory(new SimplifiedPedestrianModelFactory()); unique_ptr algorithm(new routing::AStarBidirectionalRoutingAlgorithm(nullptr)); @@ -122,7 +123,7 @@ void TestRouter(routing::IRouter & router, m2::PointD const & startPos, m2::Poin foundRoute.Swap(route); } -void TestRouters(Index const & index, m2::PointD const & startPos, m2::PointD const & finalPos) +void TestRouters(Index & index, m2::PointD const & startPos, m2::PointD const & finalPos) { auto const countryFileFn = [](m2::PointD const & /* point */) { return MAP_NAME; }; diff --git a/routing/cross_mwm_road_graph.cpp b/routing/cross_mwm_road_graph.cpp index 4ee4c44263..f28ce8a1c4 100644 --- a/routing/cross_mwm_road_graph.cpp +++ b/routing/cross_mwm_road_graph.cpp @@ -136,7 +136,8 @@ BorderCross CrossMwmGraph::FindNextMwmNode(OutgoingCrossNode const & startNode, if (ms::DistanceOnEarth(startPoint.y, startPoint.x, targetPoint.y, targetPoint.x) < kMwmCrossingNodeEqualityRadiusMeters) { - BorderCross const cross(CrossNode(startNode.m_nodeId, currentMapping->GetName(), + BorderCross const cross(CrossNode(startNode.m_nodeId, + currentMapping->GetCountryFile().GetNameWithoutExt(), MercatorBounds::FromLatLon(targetPoint.y, targetPoint.x)), CrossNode(i->m_nodeId, nextMwm, MercatorBounds::FromLatLon(targetPoint.y, targetPoint.x))); diff --git a/routing/online_absent_fetcher.hpp b/routing/online_absent_fetcher.hpp index 79feb98e90..2626dfecac 100644 --- a/routing/online_absent_fetcher.hpp +++ b/routing/online_absent_fetcher.hpp @@ -13,6 +13,8 @@ namespace routing { +using TCountryLocalFileFn = function(string const &)>; + /*! * \brief The OnlineAbsentCountriesFetcher class incapsulates async fetching the map * names from online OSRM server routines. diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp index 4eb18ee7ed..b1814540cd 100644 --- a/routing/osrm_router.cpp +++ b/routing/osrm_router.cpp @@ -327,11 +327,10 @@ private: }; } // namespace -OsrmRouter::OsrmRouter(Index const * index, TCountryFileFn const & countryFileFn, - TCountryLocalFileFn const & countryLocalFileFn, +OsrmRouter::OsrmRouter(Index * index, TCountryFileFn const & countryFileFn, TRoutingVisualizerFn routingVisualization) : m_pIndex(index), - m_indexManager(countryFileFn, countryLocalFileFn, index), + m_indexManager(countryFileFn, index), m_routingVisualization(routingVisualization) { } @@ -529,7 +528,7 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, if (!startMapping->IsValid()) { - route.AddAbsentCountry(startMapping->GetName()); + route.AddAbsentCountry(startMapping->GetCountryFile().GetNameWithoutExt()); return startMapping->GetError(); } if (!targetMapping->IsValid()) @@ -538,9 +537,9 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, startMapping->LoadCrossContext(); auto out_iterators = startMapping->m_crossContext.GetOutgoingIterators(); for (auto i = out_iterators.first; i != out_iterators.second; ++i) - if (startMapping->m_crossContext.GetOutgoingMwmName(*i) == targetMapping->GetName()) + if (startMapping->m_crossContext.GetOutgoingMwmName(*i) == targetMapping->GetCountryFile().GetNameWithoutExt()) { - route.AddAbsentCountry(targetMapping->GetName()); + route.AddAbsentCountry(targetMapping->GetCountryFile().GetNameWithoutExt()); return targetMapping->GetError(); } return targetMapping->GetError(); @@ -582,7 +581,7 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, RawRoutingResult routingResult; // 4.1 Single mwm case - if (startMapping->GetName() == targetMapping->GetName()) + if (startMapping->GetMwmId() == targetMapping->GetMwmId()) { LOG(LINFO, ("Single mwm routing case")); m_indexManager.ForEachMapping([](pair const & indexPair) @@ -654,7 +653,7 @@ IRouter::ResultCode OsrmRouter::FindPhantomNodes(m2::PointD const & point, if (!getter.HasCandidates()) return RouteNotFound; - getter.MakeResult(res, maxCount, mapping->GetName()); + getter.MakeResult(res, maxCount, mapping->GetCountryFile().GetNameWithoutExt()); return NoError; } diff --git a/routing/osrm_router.hpp b/routing/osrm_router.hpp index e2386fc994..7b9327b216 100644 --- a/routing/osrm_router.hpp +++ b/routing/osrm_router.hpp @@ -35,8 +35,7 @@ class OsrmRouter : public IRouter public: typedef vector GeomTurnCandidateT; - OsrmRouter(Index const * index, TCountryFileFn const & countryFileFn, - TCountryLocalFileFn const & countryLocalFileFn, + OsrmRouter(Index * index, TCountryFileFn const & countryFileFn, TRoutingVisualizerFn routingVisualization = nullptr); virtual string GetName() const; diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index 23ff71071b..8b4ee987a6 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -56,7 +56,7 @@ string GetCountryForMwmFile(string const & mwmName) RoadGraphRouter::~RoadGraphRouter() {} RoadGraphRouter::RoadGraphRouter(string const & name, - Index const & index, + Index & index, unique_ptr && vehicleModelFactory, unique_ptr && algorithm, TMwmFileByPointFn const & countryFileFn) @@ -104,12 +104,11 @@ IRouter::ResultCode RoadGraphRouter::CalculateRoute(m2::PointD const & startPoin return PointsInDifferentMWM; platform::CountryFile countryFile(mwmName); - MwmSet::MwmHandle const handle = - const_cast(m_index).GetMwmHandleByCountryFile(countryFile); - if (!handle.IsAlive()) + MwmSet::MwmHandle const mwmHandle = m_index.GetMwmHandleByCountryFile(countryFile); + if (!mwmHandle.IsAlive()) return RouteFileNotExist; - MwmSet::MwmId const mwmID = handle.GetId(); + MwmSet::MwmId const mwmID = mwmHandle.GetId(); if (!IsMyMWM(mwmID)) { m_vehicleModel = m_vehicleModelFactory->GetVehicleModelForCountry(GetCountryForMwmFile(mwmName)); @@ -150,7 +149,7 @@ IRouter::ResultCode RoadGraphRouter::CalculateRoute(m2::PointD const & startPoin return Convert(resultCode); } -unique_ptr CreatePedestrianAStarRouter(Index const & index, +unique_ptr CreatePedestrianAStarRouter(Index & index, TMwmFileByPointFn const & countryFileFn, TRoutingVisualizerFn const & visualizerFn) { @@ -160,7 +159,7 @@ unique_ptr CreatePedestrianAStarRouter(Index const & index, return router; } -unique_ptr CreatePedestrianAStarBidirectionalRouter(Index const & index, +unique_ptr CreatePedestrianAStarBidirectionalRouter(Index & index, TMwmFileByPointFn const & countryFileFn, TRoutingVisualizerFn const & visualizerFn) { diff --git a/routing/road_graph_router.hpp b/routing/road_graph_router.hpp index 1e5bebdc1a..6ad8678925 100644 --- a/routing/road_graph_router.hpp +++ b/routing/road_graph_router.hpp @@ -25,7 +25,7 @@ class RoadGraphRouter : public IRouter { public: RoadGraphRouter(string const & name, - Index const & index, + Index & index, unique_ptr && vehicleModelFactory, unique_ptr && algorithm, TMwmFileByPointFn const & countryFileFn); @@ -49,7 +49,7 @@ private: bool IsMyMWM(MwmSet::MwmId const & mwmID) const; string const m_name; - Index const & m_index; + Index & m_index; unique_ptr const m_vehicleModelFactory; unique_ptr const m_algorithm; TMwmFileByPointFn const m_countryFileFn; @@ -58,10 +58,10 @@ private: shared_ptr m_vehicleModel; }; -unique_ptr CreatePedestrianAStarRouter(Index const & index, +unique_ptr CreatePedestrianAStarRouter(Index & index, TMwmFileByPointFn const & countryFileFn, TRoutingVisualizerFn const & visualizerFn); -unique_ptr CreatePedestrianAStarBidirectionalRouter(Index const & index, +unique_ptr CreatePedestrianAStarBidirectionalRouter(Index & index, TMwmFileByPointFn const & countryFileFn, TRoutingVisualizerFn const & visualizerFn); diff --git a/routing/routing_mapping.cpp b/routing/routing_mapping.cpp index 3d73e94726..adf90fb52a 100644 --- a/routing/routing_mapping.cpp +++ b/routing/routing_mapping.cpp @@ -23,54 +23,28 @@ RoutingMapping::RoutingMapping(CountryFile const & countryFile) : m_mapCounter(0), m_facadeCounter(0), m_crossContextLoaded(0), - m_countryFileName(countryFile.GetNameWithoutExt()), - m_isValid(false), - m_error(IRouter::ResultCode::RouteFileNotExist) + m_mentionedCountryFile(countryFile), + m_error(IRouter::ResultCode::RouteFileNotExist), + m_handle() { } -RoutingMapping::RoutingMapping(LocalCountryFile const & localFile, Index const * pIndex) +RoutingMapping::RoutingMapping(CountryFile const & countryFile, Index * pIndex) : m_mapCounter(0), m_facadeCounter(0), m_crossContextLoaded(0), - m_countryFileName(localFile.GetCountryName()), - m_isValid(true), - m_error(IRouter::ResultCode::NoError) + m_mentionedCountryFile(countryFile), + m_error(IRouter::ResultCode::RouteFileNotExist), + m_handle(pIndex->GetMwmHandleByCountryFile(countryFile)) { - Platform & pl = GetPlatform(); - if (!HasOptions(localFile.GetFiles(), TMapOptions::EMapWithCarRouting)) - { - m_isValid = false; - m_error = IRouter::ResultCode::RouteFileNotExist; + if (!m_handle.IsAlive()) + return; + LocalCountryFile const & localFile = m_handle.GetInfo()->GetLocalFile(); + if (!HasOptions(localFile.GetFiles(), TMapOptions::EMapWithCarRouting)) return; - } - string const routingFilePath = localFile.GetPath(TMapOptions::ECarRouting); - // Open new container and check that mwm and routing have equal timestamp. - LOG(LDEBUG, ("Load routing index for file:", routingFilePath)); - m_container.Open(routingFilePath); - { - FileReader r1 = m_container.GetReader(VERSION_FILE_TAG); - ReaderSrc src1(r1); - ModelReaderPtr r2 = FilesContainerR(pl.GetCountryReader(localFile, TMapOptions::EMap)) - .GetReader(VERSION_FILE_TAG); - ReaderSrc src2(r2.GetPtr()); - version::MwmVersion version1; - version::ReadVersion(src1, version1); - - version::MwmVersion version2; - version::ReadVersion(src2, version2); - - if (version1.timestamp != version2.timestamp) - { - m_container.Close(); - m_isValid = false; - m_error = IRouter::ResultCode::InconsistentMWMandRoute; - return; - } - } - - m_mwmId = pIndex->GetMwmIdByCountryFile(localFile.GetCountryFile()); + m_error = IRouter::ResultCode::NoError; + m_container.Open(localFile.GetPath(TMapOptions::ECarRouting)); } RoutingMapping::~RoutingMapping() @@ -141,18 +115,14 @@ TRoutingMappingPtr RoutingIndexManager::GetMappingByPoint(m2::PointD const & poi TRoutingMappingPtr RoutingIndexManager::GetMappingByName(string const & mapName) { - shared_ptr localFile = m_countryLocalFileFn(mapName); - // Return invalid mapping when file does not exist. - if (!localFile) - return RoutingMapping::MakeInvalid(platform::CountryFile(mapName)); - // Check if we have already loaded this file. auto mapIter = m_mapping.find(mapName); if (mapIter != m_mapping.end()) return mapIter->second; // Or load and check file. - TRoutingMappingPtr newMapping = make_shared(*localFile, m_index); + TRoutingMappingPtr newMapping = make_shared(platform::CountryFile(mapName), + m_index); m_mapping.insert(make_pair(mapName, newMapping)); return newMapping; } diff --git a/routing/routing_mapping.h b/routing/routing_mapping.h index 41d0b0da50..404d4a30f7 100644 --- a/routing/routing_mapping.h +++ b/routing/routing_mapping.h @@ -21,7 +21,6 @@ namespace routing { using TDataFacade = OsrmDataFacade; using TCountryFileFn = function; -using TCountryLocalFileFn = function(string const &)>; /// Datamapping and facade for single MWM and MWM.routing file struct RoutingMapping @@ -31,7 +30,7 @@ struct RoutingMapping CrossRoutingContextReader m_crossContext; ///@param fName: mwm file path - RoutingMapping(platform::LocalCountryFile const & localFile, Index const * pIndex); + RoutingMapping(platform::CountryFile const & localFile, Index * pIndex); ~RoutingMapping(); @@ -47,13 +46,17 @@ struct RoutingMapping void FreeCrossContext(); - bool IsValid() const {return m_isValid;} + bool IsValid() const {return m_handle.IsAlive() && m_error == IRouter::ResultCode::NoError;} IRouter::ResultCode GetError() const {return m_error;} - string const & GetName() const { return m_countryFileName; } + /*! + * Returns mentioned country file. Works even the LocalCountryFile does not exist and + * the lock was not taken. + */ + platform::CountryFile const & GetCountryFile() const { return m_mentionedCountryFile; } - Index::MwmId const & GetMwmId() const { return m_mwmId; } + Index::MwmId const & GetMwmId() const { return m_handle.GetId(); } // static static shared_ptr MakeInvalid(platform::CountryFile const & countryFile); @@ -65,11 +68,10 @@ private: size_t m_mapCounter; size_t m_facadeCounter; bool m_crossContextLoaded; - string m_countryFileName; + platform::CountryFile m_mentionedCountryFile; FilesMappingContainer m_container; - Index::MwmId m_mwmId; - bool m_isValid; IRouter::ResultCode m_error; + MwmSet::MwmHandle m_handle; }; typedef shared_ptr TRoutingMappingPtr; @@ -100,8 +102,8 @@ class RoutingIndexManager { public: RoutingIndexManager(TCountryFileFn const & countryFileFn, - TCountryLocalFileFn const & countryLocalFileFn, Index const * index) - : m_countryFileFn(countryFileFn), m_countryLocalFileFn(countryLocalFileFn), m_index(index) + Index * index) + : m_countryFileFn(countryFileFn), m_index(index) { ASSERT(index, ()); } @@ -120,9 +122,8 @@ public: private: TCountryFileFn m_countryFileFn; - TCountryLocalFileFn m_countryLocalFileFn; unordered_map m_mapping; - Index const * m_index; + Index * m_index; }; } // namespace routing diff --git a/routing/routing_tests/routing_mapping_test.cpp b/routing/routing_tests/routing_mapping_test.cpp new file mode 100644 index 0000000000..e69de29bb2