[booking] review fixes - no mutexes

This commit is contained in:
Arsentiy Milchakov 2017-12-12 17:08:03 +03:00 committed by Ilya Grechuhin
parent b26d8b9470
commit 4ac645a02d
6 changed files with 112 additions and 65 deletions

View file

@ -144,6 +144,49 @@ void PrepareData(Index const & index, search::Results const & results,
p.m_hotelIds.push_back(std::move(hotelId));
}
}
void GetAvailableFeaturesFromCacheImpl(Index const & index, search::Results const & results,
availability::Cache & cache,
std::vector<FeatureID> & sortedResults)
{
std::vector<FeatureID> features;
for (auto const & r : results)
{
if (!r.m_metadata.m_isSponsoredHotel ||
r.GetResultType() != search::Result::ResultType::RESULT_FEATURE)
{
continue;
}
features.push_back(r.GetFeatureID());
}
std::sort(features.begin(), features.end());
MwmSet::MwmId mwmId;
std::unique_ptr<Index::FeaturesLoaderGuard> guard;
for (auto const & featureId : features)
{
if (mwmId != featureId.m_mwmId)
{
guard = my::make_unique<Index::FeaturesLoaderGuard>(index, featureId.m_mwmId);
mwmId = featureId.m_mwmId;
}
FeatureType ft;
if (!guard->GetFeatureByIndex(featureId.m_index, ft))
{
LOG(LERROR, ("Feature can't be loaded:", featureId));
continue;
}
auto const & hotelId = ft.GetMetadata().Get(feature::Metadata::FMD_SPONSORED_ID);
if (cache.Get(hotelId) == availability::Cache::HotelStatus::Available)
sortedResults.push_back(featureId);
}
}
} // namespace
namespace booking
@ -157,18 +200,12 @@ void Filter::FilterAvailability(search::Results const & results,
{
GetPlatform().RunTask(Platform::Thread::File, [this, results, params]()
{
std::lock_guard<std::mutex> lock(m_mutex);
auto const & cb = params.m_callback;
ASSERT(params.m_params.m_hotelIds.empty(), ());
m_currentParams.m_hotelIds.clear();
if (m_currentParams != params.m_params)
{
m_currentParams = std::move(params.m_params);
m_availabilityCache = std::make_shared<availability::Cache>();
}
UpdateAvailabilityParams(std::move(params.m_params));
HotelToResults hotelToResults;
PrepareData(m_index, results, hotelToResults, *m_availabilityCache, m_currentParams);
@ -199,22 +236,32 @@ void Filter::FilterAvailability(search::Results const & results,
});
}
availability::Cache::HotelStatus Filter::GetHotelAvailabilityStatus(std::string const & hotelId)
{
std::lock_guard<std::mutex> lock(m_mutex);
return m_availabilityCache->Get(hotelId);
}
void Filter::OnParamsUpdated(AvailabilityParams const & params)
{
std::lock_guard<std::mutex> lock(m_mutex);
if (m_currentParams != params)
GetPlatform().RunTask(Platform::Thread::File, [this, params]()
{
m_currentParams = params;
m_availabilityCache = std::make_shared<availability::Cache>();
}
UpdateAvailabilityParams(std::move(params));
});
}
void Filter::GetAvailableFeaturesFromCache(search::Results const & results,
FillSearchMarksCallback const & callback)
{
GetPlatform().RunTask(Platform::Thread::File, [this, results, callback]()
{
std::vector<FeatureID> resultSorted;
GetAvailableFeaturesFromCacheImpl(m_index, results, *m_availabilityCache, resultSorted);
callback(resultSorted);
});
}
void Filter::UpdateAvailabilityParams(AvailabilityParams params)
{
if (m_currentParams == params)
return;
m_currentParams = std::move(params);
m_availabilityCache = std::make_shared<availability::Cache>();
}
} // namespace filter
} // namespace booking

View file

@ -5,8 +5,9 @@
#include "base/macros.hpp"
#include <functional>
#include <memory>
#include <mutex>
#include <vector>
class Index;
@ -21,7 +22,9 @@ class Api;
namespace filter
{
// NOTE: this class IS thread-safe.
using FillSearchMarksCallback = std::function<void(std::vector<FeatureID> availableHotelsSorted)>;
class Filter
{
public:
@ -32,9 +35,13 @@ public:
void OnParamsUpdated(AvailabilityParams const & params);
availability::Cache::HotelStatus GetHotelAvailabilityStatus(std::string const & hotelId);
void GetAvailableFeaturesFromCache(search::Results const & results,
FillSearchMarksCallback const & callback);
private:
void UpdateAvailabilityParams(AvailabilityParams params);
Index const & m_index;
Api const & m_api;
@ -43,7 +50,6 @@ private:
CachePtr m_availabilityCache = std::make_shared<availability::Cache>();
AvailabilityParams m_currentParams;
std::mutex m_mutex;
DISALLOW_COPY_AND_MOVE(Filter);
};

View file

@ -15,8 +15,6 @@ Cache::Cache(size_t maxCount, size_t expiryPeriodSeconds)
Cache::HotelStatus Cache::Get(std::string const & hotelId)
{
std::lock_guard<std::mutex> lock(m_mutex);
HotelStatus result = Get(m_notReadyHotels, hotelId);
if (result == HotelStatus::Absent)
@ -27,8 +25,6 @@ Cache::HotelStatus Cache::Get(std::string const & hotelId)
void Cache::Reserve(std::string const & hotelId)
{
std::lock_guard<std::mutex> lock(m_mutex);
ASSERT(m_hotelToStatus.find(hotelId) == m_hotelToStatus.end(), ());
m_notReadyHotels.emplace(hotelId, Item());
@ -36,8 +32,6 @@ void Cache::Reserve(std::string const & hotelId)
void Cache::Insert(std::string const & hotelId, HotelStatus const s)
{
std::lock_guard<std::mutex> lock(m_mutex);
ASSERT_NOT_EQUAL(s, HotelStatus::NotReady,
("Please, use Cache::Reserve method for HotelStatus::NotReady"));
@ -50,8 +44,6 @@ void Cache::Insert(std::string const & hotelId, HotelStatus const s)
void Cache::RemoveOutdated()
{
std::lock_guard<std::mutex> lock(m_mutex);
if (m_expiryPeriodSeconds == 0)
return;
@ -61,8 +53,6 @@ void Cache::RemoveOutdated()
void Cache::Clear()
{
std::lock_guard<std::mutex> lock(m_mutex);
m_hotelToStatus.clear();
m_notReadyHotels.clear();
}

View file

@ -4,7 +4,6 @@
#include <chrono>
#include <map>
#include <mutex>
namespace booking
{
@ -12,7 +11,6 @@ namespace filter
{
namespace availability
{
// NOTE: this class IS thread-safe.
class Cache
{
public:
@ -66,10 +64,6 @@ private:
size_t const m_maxCount = 1000;
// Do not use aging when |m_expiryPeriodSeconds| is equal to zero.
size_t const m_expiryPeriodSeconds = 60;
std::mutex m_mutex;
DISALLOW_COPY_AND_MOVE(Cache);
};
std::string DebugPrint(Cache::HotelStatus status);

View file

@ -1599,7 +1599,8 @@ void Framework::FillSearchResultsMarks(bool clear, search::Results const & resul
}
void Framework::FillSearchResultsMarks(bool clear, search::Results::ConstIter begin,
search::Results::ConstIter end)
search::Results::ConstIter end,
SearchMarkPostProcesing fn /* = nullptr */)
{
UserMarkNotificationGuard guard(GetBookmarkManager(), UserMark::Type::SEARCH);
@ -1638,8 +1639,8 @@ void Framework::FillSearchResultsMarks(bool clear, search::Results::ConstIter be
if (r.m_metadata.m_isSponsoredHotel)
mark->SetMarkType(SearchMarkType::Booking);
if (GetSearchAPI().GetSponsoredMode() == SearchAPI::SponsoredMode::Booking)
SetPreparingStateForBookingHotel(r.GetFeatureID(), mark);
if (fn)
fn(*mark);
}
}
@ -3185,7 +3186,34 @@ void Framework::SetSearchDisplacementModeEnabled(bool enabled)
void Framework::ShowViewportSearchResults(bool clear, search::Results::ConstIter begin,
search::Results::ConstIter end)
{
FillSearchResultsMarks(clear, begin, end);
if (GetSearchAPI().GetSponsoredMode() != SearchAPI::SponsoredMode::Booking)
{
FillSearchResultsMarks(clear, begin, end);
return;
}
search::Results results;
results.AddResultsNoChecks(begin, end);
auto const fillCallback = [this, clear, results] (std::vector<FeatureID> featuresSorted)
{
auto const postProcessing = [featuresSorted] (SearchMarkPoint & mark)
{
auto const & id = mark.GetFeatureID();
if (!id.IsValid())
return;
auto const isAvailable =
std::binary_search(featuresSorted.cbegin(), featuresSorted.cend(), id);
mark.SetPreparing(!isAvailable);
};
FillSearchResultsMarks(clear, results.begin(), results.end(), postProcessing);
};
m_bookingFilter.GetAvailableFeaturesFromCache(results, fillCallback);
}
void Framework::ClearViewportSearchResults()
@ -3456,22 +3484,3 @@ void Framework::OnBookingFilterParamsUpdate(booking::AvailabilityParams const &
{
m_bookingFilter.OnParamsUpdated(params);
}
void Framework::SetPreparingStateForBookingHotel(FeatureID const & id, SearchMarkPoint * mark)
{
using booking::filter::availability::Cache;
Index::FeaturesLoaderGuard guard(m_model.GetIndex(), id.m_mwmId);
FeatureType ft;
if (!guard.GetFeatureByIndex(id.m_index, ft))
{
LOG(LERROR, ("Feature can't be loaded:", id));
return;
}
auto const & hotelId = ft.GetMetadata().Get(feature::Metadata::FMD_SPONSORED_ID);
auto const isPreparing =
m_bookingFilter.GetHotelAvailabilityStatus(hotelId) != Cache::HotelStatus::Available;
mark->SetPreparing(isPreparing);
}

View file

@ -551,9 +551,11 @@ public:
size_t ShowSearchResults(search::Results const & results);
using SearchMarkPostProcesing = function<void(SearchMarkPoint & mark)>;
void FillSearchResultsMarks(bool clear, search::Results const & results);
void FillSearchResultsMarks(bool clear, search::Results::ConstIter begin,
search::Results::ConstIter end);
search::Results::ConstIter end, SearchMarkPostProcesing fn = nullptr);
void ClearSearchResultsMarks();
list<TSearchRequest> const & GetLastSearchQueries() const { return m_searchQuerySaver.Get(); }
@ -853,7 +855,6 @@ public:
void OnBookingFilterParamsUpdate(booking::AvailabilityParams const & params) override;
private:
void SetPreparingStateForBookingHotel(FeatureID const & id, SearchMarkPoint * mark);
// m_discoveryManager must be bellow m_searchApi, m_viatorApi, m_localsApi
unique_ptr<discovery::Manager> m_discoveryManager;
};