Review fixes.

search/v2/features_layer_matcher.cpp

@@ -16,73 +16,77 @@ double const FeaturesLayerMatcher::kBuildingRadiusMeters = 50;
 FeaturesLayerMatcher::FeaturesLayerMatcher(Index & index, my::Cancellable const & cancellable)
   : m_context(nullptr)
   , m_reverseGeocoder(index)
-  , m_nearbyStreetsCache("F2NS")
-  , m_matchingStreetsCache("B2S")
+  , m_nearbyStreetsCache("FeatureToNearbyStreets")
+  , m_matchingStreetsCache("BuildingToStreet")
   , m_loader(scales::GetUpperScale(), ReverseGeocoder::kLookupRadiusM)
   , m_cancellable(cancellable)
 {
 }
 
-void FeaturesLayerMatcher::InitContext(MwmContext * context)
+void FeaturesLayerMatcher::SetContext(MwmContext * context)
 {
+  ASSERT(context, ());
+  if (m_context == context)
+    return;
+
   m_context = context;
   m_houseToStreetTable = HouseToStreetTable::Load(m_context->m_value);
   ASSERT(m_houseToStreetTable, ());
-  m_loader.InitContext(context);
+  m_loader.SetContext(context);
 }
 
-void FeaturesLayerMatcher::FinishQuery()
+void FeaturesLayerMatcher::OnQueryFinished()
 {
-  m_nearbyStreetsCache.FinishQuery();
-  m_matchingStreetsCache.FinishQuery();
-  m_loader.FinishQuery();
+  m_nearbyStreetsCache.ClearIfNeeded();
+  m_matchingStreetsCache.ClearIfNeeded();
+  m_loader.OnQueryFinished();
 }
 
 uint32_t FeaturesLayerMatcher::GetMatchingStreet(uint32_t houseId)
 {
-  auto r = m_matchingStreetsCache.Get(houseId);
-  if (!r.second)
-    return r.first;
+  auto entry = m_matchingStreetsCache.Get(houseId);
+  if (!entry.second)
+    return entry.first;
 
   FeatureType houseFeature;
   GetByIndex(houseId, houseFeature);
 
-  r.first = GetMatchingStreetImpl(houseId, houseFeature);
-  return r.first;
+  entry.first = GetMatchingStreetImpl(houseId, houseFeature);
+  return entry.first;
 }
 
 uint32_t FeaturesLayerMatcher::GetMatchingStreet(uint32_t houseId, FeatureType & houseFeature)
 {
-  auto r = m_matchingStreetsCache.Get(houseId);
-  if (!r.second)
-    return r.first;
+  auto entry = m_matchingStreetsCache.Get(houseId);
+  if (!entry.second)
+    return entry.first;
 
-  r.first = GetMatchingStreetImpl(houseId, houseFeature);
-  return r.first;
+  entry.first = GetMatchingStreetImpl(houseId, houseFeature);
+  return entry.first;
 }
 
 vector<ReverseGeocoder::Street> const & FeaturesLayerMatcher::GetNearbyStreets(uint32_t featureId)
 {
-  auto r = m_nearbyStreetsCache.Get(featureId);
-  if (!r.second)
-    return r.first;
+  auto entry = m_nearbyStreetsCache.Get(featureId);
+  if (!entry.second)
+    return entry.first;
 
   FeatureType feature;
   GetByIndex(featureId, feature);
 
-  m_reverseGeocoder.GetNearbyStreets(feature::GetCenter(feature), r.first);
-  return r.first;
+  m_reverseGeocoder.GetNearbyStreets(feature::GetCenter(feature), entry.first);
+  return entry.first;
 }
 
 vector<ReverseGeocoder::Street> const & FeaturesLayerMatcher::GetNearbyStreets(
     uint32_t featureId, FeatureType & feature)
 {
-  auto r = m_nearbyStreetsCache.Get(featureId);
-  if (!r.second)
-    return r.first;
+  auto entry = m_nearbyStreetsCache.Get(featureId);
+  if (!entry.second)
+    return entry.first;
 
-  m_reverseGeocoder.GetNearbyStreets(feature::GetCenter(feature), r.first);
-  return r.first;
+  m_reverseGeocoder.GetNearbyStreets(feature::GetCenter(feature), entry.first);
+  return entry.first;
 }
 
 uint32_t FeaturesLayerMatcher::GetMatchingStreetImpl(uint32_t houseId, FeatureType & houseFeature)

search/v2/features_layer_matcher.hpp

@@ -59,7 +59,7 @@ public:
   static double const kBuildingRadiusMeters;
 
   FeaturesLayerMatcher(Index & index, my::Cancellable const & cancellable);
-  void InitContext(MwmContext * context);
+  void SetContext(MwmContext * context);
 
   template <typename TFn>
   void Match(FeaturesLayer const & child, FeaturesLayer const & parent, TFn && fn)
@@ -91,7 +91,7 @@ public:
     }
   }
 
-  void FinishQuery();
+  void OnQueryFinished();
 
 private:
   template <typename TFn>
@@ -333,7 +333,7 @@ private:
 
   inline void GetByIndex(uint32_t id, FeatureType & ft) const
   {
-    /// @todo Add StatsCache for feature id -> (point, name / house number).
+    /// @todo Add Cache for feature id -> (point, name / house number).
     m_context->m_vector.GetByIndex(id, ft);
   }
 
@@ -343,12 +343,12 @@ private:
 
   // Cache of streets in a feature's vicinity. All lists in the cache
   // are ordered by distance from the corresponding feature.
-  StatsCache<uint32_t, vector<ReverseGeocoder::Street>> m_nearbyStreetsCache;
+  Cache<uint32_t, vector<ReverseGeocoder::Street>> m_nearbyStreetsCache;
 
   // Cache of correct streets for buildings. Current search algorithm
   // supports only one street for a building, whereas buildings can be
   // located on multiple streets.
-  StatsCache<uint32_t, uint32_t> m_matchingStreetsCache;
+  Cache<uint32_t, uint32_t> m_matchingStreetsCache;
 
   unique_ptr<HouseToStreetTable> m_houseToStreetTable;
 

search/v2/geocoder.cpp

@@ -294,6 +294,7 @@ Geocoder::Geocoder(Index & index, storage::CountryInfoGetter const & infoGetter)
   , m_numTokens(0)
   , m_model(SearchModel::Instance())
   , m_streets(nullptr)
+  , m_matcher(nullptr)
   , m_finder(static_cast<my::Cancellable const &>(*this))
   , m_results(nullptr)
 {
@@ -426,34 +427,35 @@ void Geocoder::GoImpl(vector<shared_ptr<MwmInfo>> & infos, bool inViewport)
 
     // MatchViewportAndPosition() should always be matched in mwms
     // intersecting with position and viewport.
+    auto const & cancellable = static_cast<my::Cancellable const&>(*this);
     auto processCountry = [&](size_t index, unique_ptr<MwmContext> context)
     {
       ASSERT(context, ());
       m_context = move(context);
       MY_SCOPE_GUARD(cleanup, [&]()
                      {
-                       LOG(LDEBUG, ("Search results for", m_context->GetMwmName()));
-                       m_matcher->FinishQuery();
+                       LOG(LDEBUG, (m_context->GetMwmName(), "processing complete."));
+                       m_matcher->OnQueryFinished();
                        m_matcher = nullptr;
                        m_context.reset();
                        m_addressFeatures.clear();
                        m_streets = nullptr;
                      });
 
-      auto iMatcher = m_matchersCache.find(m_context->m_id);
-      if (iMatcher == m_matchersCache.end())
+      auto it = m_matchersCache.find(m_context->m_id);
+      if (it == m_matchersCache.end())
       {
-        iMatcher = m_matchersCache.insert(make_pair(m_context->m_id, make_unique<FeaturesLayerMatcher>(
-                                            m_index, *this /* cancellable */))).first;
+        it = m_matchersCache.insert(make_pair(m_context->m_id, make_unique<FeaturesLayerMatcher>(
+                    m_index, cancellable))).first;
       }
-      m_matcher = iMatcher->second.get();
-      m_matcher->InitContext(m_context.get());
+      m_matcher = it->second.get();
+      m_matcher->SetContext(m_context.get());
 
       unique_ptr<coding::CompressedBitVector> viewportCBV;
       if (inViewport)
       {
         viewportCBV = Retrieval::RetrieveGeometryFeatures(
-              m_context->m_value, static_cast<my::Cancellable const &>(*this),
+              m_context->m_value, cancellable,
               m_params.m_viewport, m_params.m_scale);
       }
 
@@ -464,7 +466,7 @@ void Geocoder::GoImpl(vector<shared_ptr<MwmInfo>> & infos, bool inViewport)
         PrepareRetrievalParams(i, i + 1);
 
         m_addressFeatures[i] = Retrieval::RetrieveAddressFeatures(
-            m_context->m_value, *this /* cancellable */, m_retrievalParams);
+              m_context->m_value, cancellable, m_retrievalParams);
         ASSERT(m_addressFeatures[i], ());
 
         if (viewportCBV)
@@ -726,7 +728,7 @@ void Geocoder::MatchRegions(RegionType type)
 
   auto const & regions = m_regions[type];
 
-  auto const & fileName = m_context->m_handle.GetId().GetInfo()->GetCountryName();
+  auto const & fileName = m_context->GetMwmName();
 
   // Try to match regions.
   for (auto const & p : regions)

search/v2/stats_cache.hpp

@@ -2,6 +2,7 @@
 #include "base/logging.hpp"
 
 #include "std/unordered_map.hpp"
+#include "std/utility.hpp"
 
 
 namespace search
@@ -10,22 +11,30 @@ namespace v2
 {
 
 template <class TKey, class TValue>
-class StatsCache
+class Cache
 {
   unordered_map<TKey, TValue> m_map;
-  size_t m_count, m_new, m_emptyCount;
-  char const * m_name;
+
+  /// query statistics
+  size_t m_accesses;
+  size_t m_misses;
+
+  size_t m_emptyQueriesCount; /// empty queries count at a row
+  string m_name;              /// cache name for print functions
 
 public:
-  explicit StatsCache(char const * name) : m_count(0), m_new(0), m_emptyCount(0), m_name(name) {}
+  explicit Cache(string const & name)
+    : m_accesses(0), m_misses(0), m_emptyQueriesCount(0), m_name(name)
+  {
+  }
 
   pair<TValue &, bool> Get(TKey const & key)
   {
     auto r = m_map.insert(make_pair(key, TValue()));
 
-    ++m_count;
+    ++m_accesses;
     if (r.second)
-      ++m_new;
+      ++m_misses;
 
     return pair<TValue &, bool>(r.first->second, r.second);
   }
@@ -33,20 +42,24 @@ public:
   void Clear()
   {
     m_map.clear();
-    m_count = m_new = 0;
+    m_accesses = m_misses = 0;
+    m_emptyQueriesCount = 0;
   }
 
-  void FinishQuery()
+  /// Called at the end of every search query.
+  void ClearIfNeeded()
   {
-    if (m_count != 0)
+    if (m_accesses != 0)
     {
-      LOG(LDEBUG, ("Cache", m_name, "Queries =", m_count, "From cache =", m_count - m_new, "Added =", m_new));
-      m_count = m_new = 0;
+      LOG(LDEBUG, ("Cache", m_name, "Queries =", m_accesses,
+                   "From cache =", m_accesses - m_misses, "Added =", m_misses));
+      m_accesses = m_misses = 0;
+      m_emptyQueriesCount = 0;
     }
-    else if (++m_emptyCount > 5)
+    else if (++m_emptyQueriesCount > 5)
     {
+      LOG(LDEBUG, ("Clearing cache", m_name));
       Clear();
-      m_emptyCount = 0;
     }
   }
 };

search/v2/street_vicinity_loader.cpp

@@ -20,16 +20,20 @@ StreetVicinityLoader::StreetVicinityLoader(int scale, double offsetMeters)
 {
 }
 
-void StreetVicinityLoader::InitContext(MwmContext * context)
+void StreetVicinityLoader::SetContext(MwmContext * context)
 {
+  ASSERT(context, ());
+  if (m_context == context)
+    return;
+
   m_context = context;
   auto const scaleRange = m_context->m_value.GetHeader().GetScaleRange();
   m_scale = my::clamp(m_scale, scaleRange.first, scaleRange.second);
 }
 
-void StreetVicinityLoader::FinishQuery()
+void StreetVicinityLoader::OnQueryFinished()
 {
-  m_cache.FinishQuery();
+  m_cache.ClearIfNeeded();
 }
 
 StreetVicinityLoader::Street const & StreetVicinityLoader::GetStreet(uint32_t featureId)

search/v2/street_vicinity_loader.hpp

@@ -41,7 +41,7 @@ public:
   };
 
   StreetVicinityLoader(int scale, double offsetMeters);
-  void InitContext(MwmContext * context);
+  void SetContext(MwmContext * context);
 
   // Calls |fn| on each index in |sortedIds| where sortedIds[index]
   // belongs to the street's vicinity.
@@ -69,7 +69,7 @@ public:
     }
   }
 
-  void FinishQuery();
+  void OnQueryFinished();
 
   Street const & GetStreet(uint32_t featureId);
 
@@ -80,7 +80,7 @@ private:
   int m_scale;
   double const m_offsetMeters;
 
-  StatsCache<uint32_t, Street> m_cache;
+  Cache<uint32_t, Street> m_cache;
 
   DISALLOW_COPY_AND_MOVE(StreetVicinityLoader);
 };