From c854f3ea8269ffcd08b4106ab9a0254917540830 Mon Sep 17 00:00:00 2001 From: Yuri Gorshenin Date: Fri, 9 Oct 2015 14:07:44 +0300 Subject: [PATCH] Review fixes. --- generator/generator_tool/generator_tool.cpp | 13 +- indexer/index.cpp | 9 +- indexer/rank_table.cpp | 168 +++++++++++++------- indexer/rank_table.hpp | 9 +- 4 files changed, 120 insertions(+), 79 deletions(-) diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp index b784cc444c..9cc640cb8f 100644 --- a/generator/generator_tool/generator_tool.cpp +++ b/generator/generator_tool/generator_tool.cpp @@ -22,7 +22,6 @@ #include "indexer/rank_table.hpp" #include "indexer/search_index_builder.hpp" -#include "coding/file_container.hpp" #include "coding/file_name_utils.hpp" #include "base/timer.hpp" @@ -37,7 +36,6 @@ #include "std/fstream.hpp" #include "std/iomanip.hpp" #include "std/numeric.hpp" -#include "std/vector.hpp" DEFINE_bool(generate_update, false, @@ -210,15 +208,8 @@ int main(int argc, char ** argv) LOG(LCRITICAL, ("Error generating search index.")); LOG(LINFO, ("Generating rank table for ", datFile)); - vector ranks; - { - FilesContainerR rcont(datFile); - search::RankTableBuilder::CalcSearchRanks(rcont, ranks); - } - { - FilesContainerW wcont(datFile, FileWriter::OP_WRITE_EXISTING); - search::RankTableBuilder::Create(ranks, wcont); - } + if (!search::RankTableBuilder::CreateIfNotExists(datFile)) + LOG(LCRITICAL, ("Error generating rank table.")); } } diff --git a/indexer/index.cpp b/indexer/index.cpp index adedb68d4a..a39138fa44 100644 --- a/indexer/index.cpp +++ b/indexer/index.cpp @@ -72,14 +72,7 @@ unique_ptr Index::CreateValue(MwmInfo & info) const if (!infoEx.m_rankTableCtorCalled) { infoEx.m_rankTableCtorCalled = true; - try - { - search::RankTableBuilder::CreateIfNotExists(localFile); - } - catch (exception const & e) - { - LOG(LWARNING, ("Can't create rank table for:", localFile, ":", e.what())); - } + search::RankTableBuilder::CreateIfNotExists(localFile); } unique_ptr p(new MwmValue(localFile)); diff --git a/indexer/rank_table.cpp b/indexer/rank_table.cpp index 2c0b89d36a..fa71ca1c2b 100644 --- a/indexer/rank_table.cpp +++ b/indexer/rank_table.cpp @@ -166,19 +166,18 @@ public: unique_ptr table; switch (CheckEndianness(MemReader(region->ImmutableData(), region->Size()))) { - case CheckResult::CorruptedHeader: - break; - case CheckResult::EndiannessMismatch: - table.reset(new RankTableV0()); - coding::ReverseMap(table->m_coding, region->MutableData() + kHeaderSize, - "SimpleDenseCoding"); - table->m_region = move(region); - break; - case CheckResult::EndiannessMatch: - table.reset(new RankTableV0()); - coding::Map(table->m_coding, region->ImmutableData() + kHeaderSize, "SimpleDenseCoding"); - table->m_region = move(region); - break; + case CheckResult::CorruptedHeader: + break; + case CheckResult::EndiannessMismatch: + table.reset(new RankTableV0()); + coding::ReverseMap(table->m_coding, region->MutableData() + kHeaderSize, "SimpleDenseCoding"); + table->m_region = move(region); + break; + case CheckResult::EndiannessMatch: + table.reset(new RankTableV0()); + coding::Map(table->m_coding, region->ImmutableData() + kHeaderSize, "SimpleDenseCoding"); + table->m_region = move(region); + break; } return table; } @@ -188,7 +187,9 @@ private: coding::SimpleDenseCoding m_coding; }; -// Creates a rank section and serializes |table| to it. +// Following two functions create a rank section and serialize |table| +// to it. If there was an old section with ranks, these functions +// overwrite it. void SerializeRankTable(RankTable & table, FilesContainerW & wcont) { if (wcont.IsExist(RANKS_FILE_TAG)) @@ -205,6 +206,12 @@ void SerializeRankTable(RankTable & table, FilesContainerW & wcont) wcont.Finish(); } +void SerializeRankTable(RankTable & table, string const & mapPath) +{ + FilesContainerW wcont(mapPath); + SerializeRankTable(table, wcont); +} + // Deserializes rank table from a rank section. Returns null when it's // not possible to load a rank table (no rank section, corrupted // header, endianness mismatch for a mapped mwm). @@ -221,8 +228,8 @@ unique_ptr LoadRankTable(unique_ptr && region) static_cast(region->ImmutableData()[kVersionOffset]); switch (version) { - case RankTable::V0: - return RankTableV0::Load(move(region)); + case RankTable::V0: + return RankTableV0::Load(move(region)); } return unique_ptr(); } @@ -240,6 +247,50 @@ uint8_t CalcSearchRank(FeatureType const & ft) m2::PointD const center = feature::GetCenter(ft); return feature::GetSearchRank(types, center, ft.GetPopulation()); } + +// Creates rank table if it does not exists in |rcont| or has wrong +// endianness. Otherwise (table exists and has correct format) returns +// null. +unique_ptr CreateRankTableIfNotExists(FilesContainerR & rcont) +{ + unique_ptr table; + + if (rcont.IsExist(RANKS_FILE_TAG)) + { + switch (CheckEndianness(rcont.GetReader(RANKS_FILE_TAG))) + { + case CheckResult::CorruptedHeader: + { + // Worst case - we need to create rank table from scratch. + break; + } + case CheckResult::EndiannessMismatch: + { + // Try to copy whole serialized data and instantiate table via + // reverse mapping. + auto region = GetMemoryRegionForTag(rcont, RANKS_FILE_TAG); + table = LoadRankTable(move(region)); + break; + } + case CheckResult::EndiannessMatch: + { + // Table exists and has proper format. Nothing to do here. + return unique_ptr(); + } + } + } + + // Table doesn't exist or has wrong format. It's better to create it + // from scratch. + if (!table) + { + vector ranks; + RankTableBuilder::CalcSearchRanks(rcont, ranks); + table = make_unique(ranks); + } + + return table; +} } // namespace // static @@ -267,56 +318,57 @@ void RankTableBuilder::CalcSearchRanks(FilesContainerR & rcont, vector } // static -void RankTableBuilder::CreateIfNotExists(platform::LocalCountryFile const & localFile) +bool RankTableBuilder::CreateIfNotExists(platform::LocalCountryFile const & localFile) noexcept { - string mapPath; - - unique_ptr table; + try { - ModelReaderPtr reader = platform::GetCountryReader(localFile, MapOptions::Map); - if (!reader.GetPtr()) - return; + string mapPath; - mapPath = reader.GetName(); - - FilesContainerR rcont(reader); - if (rcont.IsExist(RANKS_FILE_TAG)) + unique_ptr table; { - switch (CheckEndianness(rcont.GetReader(RANKS_FILE_TAG))) - { - case CheckResult::CorruptedHeader: - { - // Worst case - we need to create rank table from scratch. - break; - } - case CheckResult::EndiannessMismatch: - { - // Try to copy whole serialized data and instantiate table via reverse mapping. - auto region = GetMemoryRegionForTag(rcont, RANKS_FILE_TAG); - table = LoadRankTable(move(region)); - break; - } - case CheckResult::EndiannessMatch: - { - // Table exists and has proper format. Nothing to do here. - return; - } - } + ModelReaderPtr reader = platform::GetCountryReader(localFile, MapOptions::Map); + if (!reader.GetPtr()) + return false; + + mapPath = reader.GetName(); + + FilesContainerR rcont(reader); + table = CreateRankTableIfNotExists(rcont); } - // Table doesn't exist or has wrong format. It's better to create - // it from scratch. - if (!table) - { - vector ranks; - CalcSearchRanks(rcont, ranks); - table = make_unique(ranks); - } + if (table) + SerializeRankTable(*table, mapPath); + + return true; } + catch (exception & e) + { + LOG(LWARNING, ("Can' create rank table for:", localFile, ":", e.what())); + return false; + } +} - ASSERT(table.get(), ()); - FilesContainerW wcont(mapPath, FileWriter::OP_WRITE_EXISTING); - SerializeRankTable(*table, wcont); +// static +bool RankTableBuilder::CreateIfNotExists(string const & mapPath) noexcept +{ + try + { + unique_ptr table; + { + FilesContainerR rcont(mapPath); + table = CreateRankTableIfNotExists(rcont); + } + + if (table) + SerializeRankTable(*table, mapPath); + + return true; + } + catch (exception & e) + { + LOG(LWARNING, ("Can' create rank table for:", mapPath, ":", e.what())); + return false; + } } // static diff --git a/indexer/rank_table.hpp b/indexer/rank_table.hpp index 0917c4d639..2d784c2a91 100644 --- a/indexer/rank_table.hpp +++ b/indexer/rank_table.hpp @@ -1,6 +1,7 @@ #pragma once #include "std/cstdint.hpp" +#include "std/string.hpp" #include "std/unique_ptr.hpp" #include "std/vector.hpp" @@ -86,13 +87,17 @@ public: // Calculates search ranks for all features in an mwm. static void CalcSearchRanks(FilesContainerR & rcont, vector & ranks); - // Creates rank table for an mwm. + // Following methods create rank table for an mwm. // * When rank table already exists and has proper endianness, does nothing. // * When rank table already exists but has improper endianness, re-creates it by // reverse mapping. // * When rank table does not exist or exists but is damaged, calculates all // features' ranks and creates rank table. - static void CreateIfNotExists(platform::LocalCountryFile const & localFile); + // + // Return true if rank table was successfully generated and written + // or already exists and has correct format. + static bool CreateIfNotExists(platform::LocalCountryFile const & localFile) noexcept; + static bool CreateIfNotExists(string const & mapPath) noexcept; // Force creation of a rank table from array of ranks. Existing rank // table is removed (if any). Note that |wcont| must be instantiated