From 4f7254b17503a12480c543d0b6186c7c16068083 Mon Sep 17 00:00:00 2001 From: Maxim Pimenov Date: Mon, 9 Oct 2017 17:56:52 +0300 Subject: [PATCH] Review fixes. --- search/categories_cache.cpp | 50 ++++++------------------------------- search/categories_cache.hpp | 9 +++---- search/geocoder.cpp | 30 ++++++++++++++++------ search/geocoder.hpp | 2 +- search/processor.cpp | 1 + search/query_params.hpp | 2 +- search/retrieval.cpp | 4 +-- search/retrieval.hpp | 4 +-- 8 files changed, 39 insertions(+), 63 deletions(-) diff --git a/search/categories_cache.cpp b/search/categories_cache.cpp index 87095488d9..a840fde128 100644 --- a/search/categories_cache.cpp +++ b/search/categories_cache.cpp @@ -18,47 +18,28 @@ namespace search // CategoriesCache --------------------------------------------------------------------------------- CBV CategoriesCache::Get(MwmContext const & context) { - if (!context.m_handle.IsAlive() || !context.m_value.HasSearchIndex()) - return CBV(); + CHECK(context.m_handle.IsAlive(), ()); + ASSERT(context.m_value.HasSearchIndex(), ()); - auto id = context.m_handle.GetId(); + auto const id = context.m_handle.GetId(); auto const it = m_cache.find(id); if (it != m_cache.cend()) return it->second; - auto cbv = Load(context); + auto const cbv = Load(context); m_cache[id] = cbv; return cbv; } -CBV CategoriesCache::GetFuzzy(MwmContext const & context) -{ - if (!context.m_handle.IsAlive() || !context.m_value.HasSearchIndex()) - return CBV(); - - auto id = context.m_handle.GetId(); - auto const it = m_cacheFuzzy.find(id); - if (it != m_cacheFuzzy.cend()) - return it->second; - - auto cbv = LoadFuzzy(context); - m_cacheFuzzy[id] = cbv; - return cbv; -} - -void CategoriesCache::Clear() -{ - m_cacheFuzzy.clear(); - m_cache.clear(); -} - -CBV CategoriesCache::Load(MwmContext const & context) +CBV CategoriesCache::Load(MwmContext const & context) const { ASSERT(context.m_handle.IsAlive(), ()); ASSERT(context.m_value.HasSearchIndex(), ()); auto const & c = classif(); + // Any DFA will do, since we only use requests's m_categories, + // but the interface of Retrieval forces us to make a choice. SearchTrieRequest request; m_categories.ForEach([&request, &c](uint32_t const type) { @@ -69,23 +50,6 @@ CBV CategoriesCache::Load(MwmContext const & context) return CBV(retrieval.RetrieveAddressFeatures(request)); } -CBV CategoriesCache::LoadFuzzy(MwmContext const & context) -{ - ASSERT(context.m_handle.IsAlive(), ()); - ASSERT(context.m_value.HasSearchIndex(), ()); - - auto const & c = classif(); - - SearchTrieRequest request; - - m_categories.ForEach([&request, &c](uint32_t const type) { - request.m_categories.emplace_back(FeatureTypeToString(c.GetIndexForType(type))); - }); - - Retrieval retrieval(context, m_cancellable); - return CBV(retrieval.RetrieveAddressFeaturesFuzzy(request)); -} - // StreetsCache ------------------------------------------------------------------------------------ StreetsCache::StreetsCache(my::Cancellable const & cancellable) : CategoriesCache(ftypes::IsStreetChecker::Instance(), cancellable) diff --git a/search/categories_cache.hpp b/search/categories_cache.hpp index feafa56a56..3e14ef0699 100644 --- a/search/categories_cache.hpp +++ b/search/categories_cache.hpp @@ -24,7 +24,7 @@ public: source.ForEachType([this](uint32_t type) { m_categories.Add(type); }); } - CategoriesCache(set types, my::Cancellable const & cancellable) + CategoriesCache(set const & types, my::Cancellable const & cancellable) : m_cancellable(cancellable) { for (uint32_t type : types) @@ -34,18 +34,15 @@ public: virtual ~CategoriesCache() = default; CBV Get(MwmContext const & context); - CBV GetFuzzy(MwmContext const & context); - void Clear(); + inline void Clear() { m_cache.clear(); } private: - CBV Load(MwmContext const & context); - CBV LoadFuzzy(MwmContext const & context); + CBV Load(MwmContext const & context) const; CategoriesSet m_categories; my::Cancellable const & m_cancellable; map m_cache; - map m_cacheFuzzy; }; class StreetsCache : public CategoriesCache diff --git a/search/geocoder.cpp b/search/geocoder.cpp index 5d6c8de8c9..cd292f589f 100644 --- a/search/geocoder.cpp +++ b/search/geocoder.cpp @@ -547,7 +547,7 @@ void Geocoder::GoImpl(vector> & infos, bool inViewport) features = features.Intersect(viewportCBV); } - ctx.m_villages = m_villagesCache.GetFuzzy(*m_context); + ctx.m_villages = m_villagesCache.Get(*m_context); auto citiesFromWorld = m_cities; FillVillageLocalities(ctx); @@ -556,15 +556,16 @@ void Geocoder::GoImpl(vector> & infos, bool inViewport) m_cities = citiesFromWorld; }); + bool const intersectsPivot = index < numIntersectingMaps; if (m_params.IsCategorialRequest()) { - MatchCategories(ctx); + MatchCategories(ctx, intersectsPivot); } else { MatchRegions(ctx, Region::TYPE_COUNTRY); - if (index < numIntersectingMaps || m_preRanker.NumSentResults() == 0) + if (intersectsPivot || m_preRanker.NumSentResults() == 0) MatchAroundPivot(ctx); } @@ -599,9 +600,13 @@ void Geocoder::InitBaseContext(BaseContext & ctx) ctx.m_features[i] = cache.Get(*m_context); } else if (m_params.IsPrefixToken(i)) - ctx.m_features[i] = retrieval.RetrieveAddressFeaturesFuzzy(m_prefixTokenRequest); + { + ctx.m_features[i] = retrieval.RetrieveAddressFeatures(m_prefixTokenRequest); + } else - ctx.m_features[i] = retrieval.RetrieveAddressFeaturesFuzzy(m_tokenRequests[i]); + { + ctx.m_features[i] = retrieval.RetrieveAddressFeatures(m_tokenRequests[i]); + } if (m_params.m_cianMode) ctx.m_features[i] = DecimateCianResults(ctx.m_features[i]); @@ -776,8 +781,17 @@ void Geocoder::ForEachCountry(vector> const & infos, TFn && } } -void Geocoder::MatchCategories(BaseContext & ctx) +void Geocoder::MatchCategories(BaseContext & ctx, bool aroundPivot) { + auto features = ctx.m_features[0]; + + if (aroundPivot) + { + auto const pivotFeatures = RetrieveGeometryFeatures(*m_context, m_params.m_pivot, RECT_ID_PIVOT); + ViewportFilter filter(pivotFeatures, m_preRanker.Limit() /* threshold */); + features = filter.Filter(features); + } + auto emit = [&](uint64_t bit) { auto const featureId = base::asserted_cast(bit); Model::Type type; @@ -791,7 +805,7 @@ void Geocoder::MatchCategories(BaseContext & ctx) // Its features have been retrieved from the search index // using the exact (non-fuzzy) matching and intersected // with viewport, if needed. Every such feature is relevant. - ctx.m_features[0].ForEach(emit); + features.ForEach(emit); } void Geocoder::MatchRegions(BaseContext & ctx, Region::Type type) @@ -930,7 +944,7 @@ void Geocoder::LimitedSearch(BaseContext & ctx, FeaturesFilter const & filter) }); if (!ctx.m_streets) - ctx.m_streets = m_streetsCache.GetFuzzy(*m_context); + ctx.m_streets = m_streetsCache.Get(*m_context); MatchUnclassified(ctx, 0 /* curToken */); diff --git a/search/geocoder.hpp b/search/geocoder.hpp index ae291eae4f..f99fc6fcb5 100644 --- a/search/geocoder.hpp +++ b/search/geocoder.hpp @@ -154,7 +154,7 @@ private: inline void BailIfCancelled() { ::search::BailIfCancelled(m_cancellable); } // A fast-path branch for categorial requests. - void MatchCategories(BaseContext & ctx); + void MatchCategories(BaseContext & ctx, bool aroundPivot); // Tries to find all countries and states in a search query and then // performs matching of cities in found maps. diff --git a/search/processor.cpp b/search/processor.cpp index 464dfc6775..f2363e203b 100644 --- a/search/processor.cpp +++ b/search/processor.cpp @@ -390,6 +390,7 @@ void Processor::SetViewportByIndex(m2::RectD const & viewport, size_t idx, bool } void Processor::ClearCache(size_t ind) { m_viewport[ind].MakeEmpty(); } + Locales Processor::GetCategoryLocales() const { static int8_t const enLocaleCode = CategoriesHolder::MapLocaleToInteger("en"); diff --git a/search/query_params.hpp b/search/query_params.hpp index e59c72d1c6..3699777b9e 100644 --- a/search/query_params.hpp +++ b/search/query_params.hpp @@ -115,7 +115,7 @@ public: inline Langs const & GetLangs() const { return m_langs; } inline bool LangExists(int8_t lang) const { return m_langs.Contains(lang); } - inline void SetCategorialRequest(bool rhs) { m_isCategorialRequest = rhs; } + inline void SetCategorialRequest(bool isCategorial) { m_isCategorialRequest = isCategorial; } inline bool IsCategorialRequest() const { return m_isCategorialRequest; } inline int GetScale() const { return m_scale; } diff --git a/search/retrieval.cpp b/search/retrieval.cpp index 700fdf8da2..c412a4dc47 100644 --- a/search/retrieval.cpp +++ b/search/retrieval.cpp @@ -329,13 +329,13 @@ unique_ptr Retrieval::RetrieveAddressFeatures( return Retrieve(request); } -unique_ptr Retrieval::RetrieveAddressFeaturesFuzzy( +unique_ptr Retrieval::RetrieveAddressFeatures( SearchTrieRequest const & request) { return Retrieve(request); } -unique_ptr Retrieval::RetrieveAddressFeaturesFuzzy( +unique_ptr Retrieval::RetrieveAddressFeatures( SearchTrieRequest> const & request) { return Retrieve(request); diff --git a/search/retrieval.hpp b/search/retrieval.hpp index 0a99e2865f..35ea2ad267 100644 --- a/search/retrieval.hpp +++ b/search/retrieval.hpp @@ -43,10 +43,10 @@ public: unique_ptr RetrieveAddressFeatures( SearchTrieRequest> const & request); - unique_ptr RetrieveAddressFeaturesFuzzy( + unique_ptr RetrieveAddressFeatures( SearchTrieRequest const & request); - unique_ptr RetrieveAddressFeaturesFuzzy( + unique_ptr RetrieveAddressFeatures( SearchTrieRequest> const & request); // Retrieves from the search index corresponding to |value| all