Merge pull request #177 from gardster/handle_free

[routing] Reduce file handles usage on crossmwm routing.

indexer/mwm_set.hpp

@@ -8,6 +8,7 @@
 
 #include "base/macros.hpp"
 
+#include "std/atomic.hpp"
 #include "std/deque.hpp"
 #include "std/map.hpp"
 #include "std/mutex.hpp"
@@ -71,7 +72,7 @@ private:
   inline void SetStatus(Status status) { m_status = status; }
 
   platform::LocalCountryFile m_file;  ///< Path to the mwm file.
-  Status m_status;                    ///< Current country status.
+  atomic<Status> m_status;            ///< Current country status.
   uint8_t m_numRefs;                  ///< Number of active handles.
 };
 

routing/cross_mwm_road_graph.cpp

@@ -166,6 +166,7 @@ void CrossMwmGraph::GetOutgoingEdgesList(BorderCross const & v,
   TRoutingMappingPtr currentMapping = m_indexManager.GetMappingByName(v.toNode.mwmName);
   ASSERT(currentMapping->IsValid(), ());
   currentMapping->LoadCrossContext();
+  currentMapping->FreeFileIfPossible();
 
   CrossRoutingContextReader const & currentContext = currentMapping->m_crossContext;
   auto inRange = currentContext.GetIngoingIterators();

routing/osrm_router.cpp

@@ -370,13 +370,13 @@ private:
 bool OsrmRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD const & finalPoint,
                                      TCountryFileFn const & countryFileFn, Index * index)
 {
-  RoutingIndexManager manager(countryFileFn, index);
+  RoutingIndexManager manager(countryFileFn, *index);
   return manager.GetMappingByPoint(startPoint)->IsValid() &&
          manager.GetMappingByPoint(finalPoint)->IsValid();
 }
 
 OsrmRouter::OsrmRouter(Index * index, TCountryFileFn const & countryFileFn)
-    : m_pIndex(index), m_indexManager(countryFileFn, index)
+    : m_pIndex(index), m_indexManager(countryFileFn, *index)
 {
 }
 

routing/routing_mapping.cpp

@@ -47,14 +47,15 @@ bool CheckMwmConsistency(LocalCountryFile const & localFile)
 namespace routing
 {
 
-RoutingMapping::RoutingMapping(string const & countryFile, MwmSet * pIndex)
+RoutingMapping::RoutingMapping(string const & countryFile, MwmSet & index)
     : m_mapCounter(0),
       m_facadeCounter(0),
       m_crossContextLoaded(0),
       m_countryFile(countryFile),
-      m_error(IRouter::ResultCode::RouteFileNotExist)
+      m_error(IRouter::ResultCode::RouteFileNotExist),
+      m_pIndex(&index)
 {
-  m_handle = pIndex->GetMwmHandleByCountryFile(CountryFile(countryFile));
+  m_handle = index.GetMwmHandleByCountryFile(CountryFile(countryFile));
   if (!m_handle.IsAlive())
     return;
 
@@ -75,9 +76,29 @@ RoutingMapping::RoutingMapping(string const & countryFile, MwmSet * pIndex)
     return;
   }
 
+  m_mwmId = m_handle.GetId();
   m_error = IRouter::ResultCode::NoError;
 }
 
+void RoutingMapping::LoadFileIfNeeded()
+{
+  ASSERT(m_pIndex != nullptr, ());
+  if (!m_handle.IsAlive() && m_mwmId.IsAlive())
+  {
+    m_handle = m_pIndex->GetMwmHandleById(m_mwmId);
+    m_container.Open(m_mwmId.GetInfo()->GetLocalFile().GetPath(MapOptions::CarRouting));
+  }
+}
+
+void RoutingMapping::FreeFileIfPossible()
+{
+  if (m_mapCounter == 0 && m_facadeCounter == 0 && m_handle.IsAlive())
+  {
+    m_handle = MwmSet::MwmHandle();
+    m_container.Close();
+  }
+}
+
 RoutingMapping::~RoutingMapping()
 {
   // Clear data while m_container is valid.
@@ -88,6 +109,9 @@ RoutingMapping::~RoutingMapping()
 
 void RoutingMapping::Map()
 {
+  LoadFileIfNeeded();
+  if (!m_handle.IsAlive())
+    return;
   ++m_mapCounter;
   if (!m_segMapping.IsMapped())
   {
@@ -101,12 +125,16 @@ void RoutingMapping::Unmap()
   --m_mapCounter;
   if (m_mapCounter < 1 && m_segMapping.IsMapped())
     m_segMapping.Unmap();
+  FreeFileIfPossible();
 }
 
 void RoutingMapping::LoadFacade()
 {
   if (!m_facadeCounter)
   {
+    LoadFileIfNeeded();
+    if (!m_handle.IsAlive())
+      return;
     m_dataFacade.Load(m_container);
   }
   ++m_facadeCounter;
@@ -116,7 +144,10 @@ void RoutingMapping::FreeFacade()
 {
   --m_facadeCounter;
   if (!m_facadeCounter)
+  {
+    FreeFileIfPossible();
     m_dataFacade.Clear();
+  }
 }
 
 void RoutingMapping::LoadCrossContext()
@@ -124,6 +155,11 @@ void RoutingMapping::LoadCrossContext()
   if (m_crossContextLoaded)
     return;
 
+  LoadFileIfNeeded();
+
+  if (!m_handle.IsAlive())
+    return;
+
   if (m_container.IsExist(ROUTING_CROSS_CONTEXT_TAG))
   {
     m_crossContext.Load(m_container.GetReader(ROUTING_CROSS_CONTEXT_TAG));
@@ -135,6 +171,7 @@ void RoutingMapping::FreeCrossContext()
 {
   m_crossContextLoaded = false;
   m_crossContext = CrossRoutingContextReader();
+  FreeFileIfPossible();
 }
 
 TRoutingMappingPtr RoutingIndexManager::GetMappingByPoint(m2::PointD const & point)

routing/routing_mapping.hpp

@@ -25,9 +25,9 @@ struct RoutingMapping
 
   /// Default constructor to create invalid instance for existing client code.
   /// @postcondition IsValid() == false.
-  RoutingMapping() = default;
+  RoutingMapping() : m_pIndex(nullptr) {}
   /// @param countryFile Country file name without extension.
-  RoutingMapping(string const & countryFile, MwmSet * pIndex);
+  RoutingMapping(string const & countryFile, MwmSet & index);
   ~RoutingMapping();
 
   void Map();
@@ -39,7 +39,7 @@ struct RoutingMapping
   void LoadCrossContext();
   void FreeCrossContext();
 
-  bool IsValid() const { return m_handle.IsAlive() && m_error == IRouter::ResultCode::NoError; }
+  bool IsValid() const { return m_error == IRouter::ResultCode::NoError && m_mwmId.IsAlive(); }
 
   IRouter::ResultCode GetError() const { return m_error; }
 
@@ -49,9 +49,15 @@ struct RoutingMapping
    */
   string const & GetCountryName() const { return m_countryFile; }
 
-  Index::MwmId const & GetMwmId() const { return m_handle.GetId(); }
+  Index::MwmId const & GetMwmId() const { return m_mwmId; }
+
+  // Free file handles if it is possible. Works only if there is no mapped sections. Cross section
+  // will be valid after free.
+  void FreeFileIfPossible();
 
 private:
+  void LoadFileIfNeeded();
+
   size_t m_mapCounter;
   size_t m_facadeCounter;
   bool m_crossContextLoaded;
@@ -59,6 +65,9 @@ private:
   FilesMappingContainer m_container;
   IRouter::ResultCode m_error;
   MwmSet::MwmHandle m_handle;
+  // We save a mwmId for possibility to unlock a mwm file by rewriting m_handle.
+  Index::MwmId m_mwmId;
+  MwmSet * m_pIndex;
 };
 
 typedef shared_ptr<RoutingMapping> TRoutingMappingPtr;
@@ -88,10 +97,9 @@ public:
 class RoutingIndexManager
 {
 public:
-  RoutingIndexManager(TCountryFileFn const & countryFileFn, MwmSet * index)
+  RoutingIndexManager(TCountryFileFn const & countryFileFn, MwmSet & index)
       : m_countryFileFn(countryFileFn), m_index(index)
   {
-    ASSERT(index, ());
   }
 
   TRoutingMappingPtr GetMappingByPoint(m2::PointD const & point);
@@ -109,7 +117,7 @@ public:
 private:
   TCountryFileFn m_countryFileFn;
   unordered_map<string, TRoutingMappingPtr> m_mapping;
-  MwmSet * m_index;
+  MwmSet & m_index;
 };
 
 }  // namespace routing

routing/routing_tests/routing_mapping_test.cpp

@@ -70,7 +70,7 @@ UNIT_TEST(RoutingMappingCountryFileLockTest)
 {
   LocalFileGenerator generator("1TestCountry");
   {
-    RoutingMapping testMapping(generator.GetCountryName(), (&generator.GetMwmSet()));
+    RoutingMapping testMapping(generator.GetCountryName(), (generator.GetMwmSet()));
     TEST(testMapping.IsValid(), ());
     TEST_EQUAL(generator.GetNumRefs(), 1, ());
   }
@@ -83,7 +83,7 @@ UNIT_TEST(IndexManagerLockManagementTest)
   string const fileName("1TestCountry");
   LocalFileGenerator generator(fileName);
   RoutingIndexManager manager([&fileName](m2::PointD const & q) { return fileName; },
-                              &generator.GetMwmSet());
+                              generator.GetMwmSet());
   {
     auto testMapping = manager.GetMappingByName(fileName);
     TEST(testMapping->IsValid(), ());