From afa652c0616abb9b753e00bc7b89818aa7169c77 Mon Sep 17 00:00:00 2001 From: Maksim Andrianov Date: Thu, 8 Aug 2019 15:36:29 +0300 Subject: [PATCH] Review fixes. --- generator/filter_world.cpp | 9 +++---- generator/filter_world.hpp | 2 +- generator/raw_generator.cpp | 29 +++++++++++----------- generator/raw_generator.hpp | 6 ++--- generator/raw_generator_writer.cpp | 25 +++++++++++-------- generator/raw_generator_writer.hpp | 4 +-- generator/translators_pool.cpp | 40 ++++++++++++++---------------- generator/translators_pool.hpp | 5 ++-- 8 files changed, 59 insertions(+), 61 deletions(-) diff --git a/generator/filter_world.cpp b/generator/filter_world.cpp index 6351062df4..78c828382a 100644 --- a/generator/filter_world.cpp +++ b/generator/filter_world.cpp @@ -22,7 +22,7 @@ std::shared_ptr FilterWorld::Clone() const bool FilterWorld::IsAccepted(feature::FeatureBuilder const & fb) { - return IsGoogScale(fb) || + return IsGoodScale(fb) || IsPopularAttraction(fb, m_popularityFilename) || IsInternationalAirport(fb); } @@ -35,7 +35,7 @@ bool FilterWorld::IsInternationalAirport(feature::FeatureBuilder const & fb) } // static -bool FilterWorld::IsGoogScale(feature::FeatureBuilder const & fb) +bool FilterWorld::IsGoodScale(feature::FeatureBuilder const & fb) { // GetMinFeatureDrawScale also checks suitable size for AREA features return scales::GetUpperWorldScale() >= fb.GetMinFeatureDrawScale(); @@ -64,10 +64,7 @@ bool FilterWorld::IsPopularAttraction(feature::FeatureBuilder const & fb, std::s // todo(@t.yan): adjust uint8_t const kPopularityThreshold = 12; - if (it->second < kPopularityThreshold) - return false; - // todo(@t.yan): maybe check place has wikipedia link. - return true; + return it->second >= kPopularityThreshold; } } // namespace generator diff --git a/generator/filter_world.hpp b/generator/filter_world.hpp index 62833cfdc7..b2ec4e1333 100644 --- a/generator/filter_world.hpp +++ b/generator/filter_world.hpp @@ -20,7 +20,7 @@ public: bool IsAccepted(feature::FeatureBuilder const & feature) override; static bool IsInternationalAirport(feature::FeatureBuilder const & fb); - static bool IsGoogScale(feature::FeatureBuilder const & fb); + static bool IsGoodScale(feature::FeatureBuilder const & fb); static bool IsPopularAttraction(feature::FeatureBuilder const & fb, std::string const & popularityFilename); private: diff --git a/generator/raw_generator.cpp b/generator/raw_generator.cpp index 1b89245ffa..a7cac75cfe 100644 --- a/generator/raw_generator.cpp +++ b/generator/raw_generator.cpp @@ -12,10 +12,10 @@ namespace generator { -RawGenerator::RawGenerator(feature::GenerateInfo & genInfo, size_t threadsCount, size_t chankSize) +RawGenerator::RawGenerator(feature::GenerateInfo & genInfo, size_t threadsCount, size_t chunkSize) : m_genInfo(genInfo) , m_threadsCount(threadsCount) - , m_chankSize(chankSize) + , m_chunkSize(chunkSize) , m_cache(std::make_shared(genInfo)) , m_queue(std::make_shared()) , m_translators(std::make_shared()) @@ -94,24 +94,23 @@ bool RawGenerator::Execute() while (!m_finalProcessors.empty()) { base::thread_pool::computational::ThreadPool threadPool(m_threadsCount); - do + while (true) { - auto const & finalProcessor = m_finalProcessors.top(); + auto const finalProcessor = m_finalProcessors.top(); + m_finalProcessors.pop(); threadPool.SubmitWork([finalProcessor{finalProcessor}]() { finalProcessor->Process(); }); - m_finalProcessors.pop(); if (m_finalProcessors.empty() || *finalProcessor != *m_finalProcessors.top()) break; } - while (true); } LOG(LINFO, ("Final processing is finished.")); return true; } -std::vector RawGenerator::GetNames() const +std::vector const & RawGenerator::GetNames() const { return m_names; } @@ -160,29 +159,30 @@ bool RawGenerator::GenerateFilteredFeatures() SourceReader reader = m_genInfo.m_osmFileName.empty() ? SourceReader() : SourceReader(m_genInfo.m_osmFileName); - unique_ptr sourseProcessor; + std::unique_ptr sourseProcessor; switch (m_genInfo.m_osmFileType) { case feature::GenerateInfo::OsmSourceType::O5M: - sourseProcessor = make_unique(reader); + sourseProcessor = std::make_unique(reader); break; case feature::GenerateInfo::OsmSourceType::XML: - sourseProcessor = make_unique(reader); + sourseProcessor = std::make_unique(reader); break; } + CHECK(sourseProcessor, ()); - TranslatorsPool translators(m_translators, m_cache, m_threadsCount - 1 /* copyCount */); + TranslatorsPool translators(m_translators, m_cache, m_threadsCount); RawGeneratorWriter rawGeneratorWriter(m_queue, m_genInfo.m_tmpDir); rawGeneratorWriter.Run(); size_t element_pos = 0; - std::vector elements(m_chankSize); + std::vector elements(m_chunkSize); while(sourseProcessor->TryRead(elements[element_pos])) { - if (++element_pos != m_chankSize) + if (++element_pos != m_chunkSize) continue; translators.Emit(std::move(elements)); - elements = vector(m_chankSize); + elements = vector(m_chunkSize); element_pos = 0; } elements.resize(element_pos); @@ -192,6 +192,7 @@ bool RawGenerator::GenerateFilteredFeatures() if (!translators.Finish()) return false; + rawGeneratorWriter.ShutdownAndJoin(); m_names = rawGeneratorWriter.GetNames(); LOG(LINFO, ("Names:", m_names)); return true; diff --git a/generator/raw_generator.hpp b/generator/raw_generator.hpp index d98f210ac7..ceb2073bef 100644 --- a/generator/raw_generator.hpp +++ b/generator/raw_generator.hpp @@ -18,7 +18,7 @@ class RawGenerator { public: explicit RawGenerator(feature::GenerateInfo & genInfo, size_t threadsCount = 1, - size_t chankSize = 1024); + size_t chunkSize = 1024); void GenerateCountries(bool disableAds = true); void GenerateWorld(bool disableAds = true); @@ -30,7 +30,7 @@ public: void GenerateCustom(std::shared_ptr const & translator, std::shared_ptr const & finalProcessor); bool Execute(); - std::vector GetNames() const; + std::vector const & GetNames() const; std::shared_ptr GetQueue(); void ForceReloadCache(); @@ -52,7 +52,7 @@ private: feature::GenerateInfo & m_genInfo; size_t m_threadsCount; - size_t m_chankSize; + size_t m_chunkSize; std::shared_ptr m_cache; std::shared_ptr m_queue; std::shared_ptr m_translators; diff --git a/generator/raw_generator_writer.cpp b/generator/raw_generator_writer.cpp index aac73644a5..0e380b00d0 100644 --- a/generator/raw_generator_writer.cpp +++ b/generator/raw_generator_writer.cpp @@ -25,6 +25,8 @@ void RawGeneratorWriter::Run() { FeatureProcessorChank chank; m_queue->WaitAndPop(chank); + // As a sign of the end of tasks, we use an empty message. We have the right to do that, + // because there is only one reader. if (chank.IsEmpty()) return; @@ -35,10 +37,11 @@ void RawGeneratorWriter::Run() std::vector RawGeneratorWriter::GetNames() { - ShutdownAndJoin(); + CHECK(!m_thread.joinable(), ()); + std::vector names; - names.reserve(m_collectors.size()); - for (const auto & p : m_collectors) + names.reserve(m_writers.size()); + for (const auto & p : m_writers) names.emplace_back(p.first); return names; @@ -53,26 +56,28 @@ void RawGeneratorWriter::Write(std::vector const & vecChanks) if (affiliation.empty()) continue; - auto collectorIt = m_collectors.find(affiliation); - if (collectorIt == std::cend(m_collectors)) + auto writerIt = m_writers.find(affiliation); + if (writerIt == std::cend(m_writers)) { auto path = base::JoinPath(m_path, affiliation + DATA_FILE_EXTENSION_TMP); auto writer = std::make_unique(std::move(path)); - collectorIt = m_collectors.emplace(affiliation, std::move(writer)).first; + writerIt = m_writers.emplace(affiliation, std::move(writer)).first; } - auto & collector = collectorIt->second; + auto & writer = writerIt->second; auto const & buffer = chank.m_buffer; - WriteVarUint(*collector, static_cast(buffer.size())); - collector->Write(buffer.data(), buffer.size()); + WriteVarUint(*writer, static_cast(buffer.size())); + writer->Write(buffer.data(), buffer.size()); } } } void RawGeneratorWriter::ShutdownAndJoin() { - m_queue->Push({}); if (m_thread.joinable()) + { + m_queue->Push({}); m_thread.join(); + } } } // namespace generator diff --git a/generator/raw_generator_writer.hpp b/generator/raw_generator_writer.hpp index d286eb8ba4..bccaf31d6d 100644 --- a/generator/raw_generator_writer.hpp +++ b/generator/raw_generator_writer.hpp @@ -19,17 +19,17 @@ public: ~RawGeneratorWriter(); void Run(); + void ShutdownAndJoin(); std::vector GetNames(); private: using FeatureBuilderWriter = feature::FeatureBuilderWriter; void Write(std::vector const & vecChanks); - void ShutdownAndJoin(); std::thread m_thread; std::shared_ptr m_queue; std::string m_path; - std::unordered_map> m_collectors; + std::unordered_map> m_writers; }; } // namespace generator diff --git a/generator/translators_pool.cpp b/generator/translators_pool.cpp index 08e1d96773..c132ee05c8 100644 --- a/generator/translators_pool.cpp +++ b/generator/translators_pool.cpp @@ -6,47 +6,43 @@ namespace generator { TranslatorsPool::TranslatorsPool(std::shared_ptr const & original, std::shared_ptr const & cache, - size_t copyCount) - : m_translators({original}) - , m_threadPool(copyCount + 1) + size_t threadCount) + : m_threadPool(threadCount) { - m_freeTranslators.Push(0); - m_translators.reserve(copyCount + 1); - for (size_t i = 0; i < copyCount; ++i) - { - auto cache_ = cache->Clone(); - auto translator = original->Clone(cache_); - m_translators.emplace_back(translator); - m_freeTranslators.Push(i + 1); - } + m_translators.Push(original); + for (size_t i = 0; i < threadCount; ++i) + m_translators.Push(original->Clone()); + + CHECK_EQUAL(m_translators.size(), threadCount + 1, ()); } void TranslatorsPool::Emit(std::vector && elements) { - base::threads::DataWrapper d; - m_freeTranslators.WaitAndPop(d); - auto const idx = d.Get(); - m_threadPool.SubmitWork([&, idx, elements{move(elements)}]() mutable { + std::shared_ptr translator; + m_translators.WaitAndPop(translator); + m_threadPool.SubmitWork([&, translator, elements{move(elements)}]() mutable { for (auto & element : elements) - m_translators[idx]->Emit(element); + translator->Emit(element); - m_freeTranslators.Push(idx); + m_translators.Push(translator); }); } bool TranslatorsPool::Finish() { - m_threadPool.WaitAndStop(); + m_threadPool.WaitingStop(); using TranslatorPtr = std::shared_ptr; base::threads::ThreadSafeQueue> queue; - for (auto const & t : m_translators) + while (!m_translators.Empty()) { std::promise p; - p.set_value(t); + std::shared_ptr translator; + m_translators.TryPop(translator); + p.set_value(translator); queue.Push(p.get_future()); } - base::thread_pool::computational::ThreadPool pool(m_translators.size() / 2 + 1); + base::thread_pool::computational::ThreadPool pool(queue.size() / 2 + 1); while (queue.Size() != 1) { std::future left; diff --git a/generator/translators_pool.hpp b/generator/translators_pool.hpp index 8dcc3e9d47..2e1801bec4 100644 --- a/generator/translators_pool.hpp +++ b/generator/translators_pool.hpp @@ -17,14 +17,13 @@ class TranslatorsPool public: explicit TranslatorsPool(std::shared_ptr const & original, std::shared_ptr const & cache, - size_t copyCount); + size_t threadCount); void Emit(std::vector && elements); bool Finish(); private: - std::vector> m_translators; base::thread_pool::computational::ThreadPool m_threadPool; - base::threads::ThreadSafeQueue> m_freeTranslators; + base::threads::ThreadSafeQueue> m_translators; }; } // namespace generator