From fa062c45a4f4c76ca313e9da41efe18948cea326 Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Fri, 6 Mar 2015 18:27:34 +0300 Subject: [PATCH] PR fixes --- coding/coding_tests/file_container_test.cpp | 1 - indexer/features_offsets_table.cpp | 29 +++++++++------------ indexer/features_offsets_table.hpp | 2 +- map/framework.cpp | 2 +- platform/platform.cpp | 4 +-- platform/platform.hpp | 6 +++-- platform/platform_android.cpp | 6 ++--- platform/platform_ios.mm | 4 +-- platform/platform_qt.cpp | 4 +-- platform/platform_tizen.cpp | 4 +-- routing/osrm2feature_map.cpp | 25 +++++++++--------- routing/osrm2feature_map.hpp | 9 +++---- 12 files changed, 46 insertions(+), 50 deletions(-) diff --git a/coding/coding_tests/file_container_test.cpp b/coding/coding_tests/file_container_test.cpp index 07fc7096d3..55be9717fb 100644 --- a/coding/coding_tests/file_container_test.cpp +++ b/coding/coding_tests/file_container_test.cpp @@ -277,7 +277,6 @@ UNIT_TEST(FilesMappingContainer_MoveHandle) } } - FileWriter::DeleteFileX(containerPath); } UNIT_TEST(FilesMappingContainer_Smoke) diff --git a/indexer/features_offsets_table.cpp b/indexer/features_offsets_table.cpp index 0a934cb5e4..23015bfd7f 100644 --- a/indexer/features_offsets_table.cpp +++ b/indexer/features_offsets_table.cpp @@ -25,8 +25,8 @@ namespace feature FeaturesOffsetsTable::FeaturesOffsetsTable(string const & fileName) { - m_pSrc = unique_ptr(new MmapReader(fileName)); - succinct::mapper::map(m_table, reinterpret_cast(m_pSrc->Data())); + m_pReader.reset(new MmapReader(fileName)); + succinct::mapper::map(m_table, reinterpret_cast(m_pReader->Data())); } // static @@ -97,29 +97,26 @@ namespace feature size_t FeaturesOffsetsTable::GetFeatureIndexbyOffset(uint64_t offset) const { + ASSERT_GREATER(size(), 0, ("We must not ask empty table")); ASSERT_LESS_OR_EQUAL(offset, m_table.select(size() - 1), ("Offset out of bounds", offset, m_table.select(size() - 1))); + ASSERT_GREATER_OR_EQUAL(offset, m_table.select(0), ("Offset out of bounds", offset, + m_table.select(size() - 1))); //Binary search in elias_fano list - size_t first = 0, last = size(); - size_t count = last - first, step, current; - while (count > 0) - { - step = count / 2; - current = first + step; - if (m_table.select(current) < offset) - { - first = ++current; - count -= step + 1; - } + size_t leftBound = 0, rightBound = size(); + while (leftBound + 1 < rightBound) { + size_t middle = leftBound + (rightBound - leftBound) / 2; + if (m_table.select(middle) <= offset) + leftBound = middle; else - count = step; + rightBound = middle; } - return current; + return leftBound; } string FeaturesOffsetsTable::GetIndexFileName(string const & countryName) { - return GetPlatform().WritablePathForFileIndexes(countryName) + countryName + FEATURES_OFFSETS_TABLE_FILE_EXT; + return GetPlatform().WritablePathForCountryIndexes(countryName) + countryName + FEATURES_OFFSETS_TABLE_FILE_EXT; } } // namespace feature diff --git a/indexer/features_offsets_table.hpp b/indexer/features_offsets_table.hpp index 7d735a3ac9..8f89c91ef2 100644 --- a/indexer/features_offsets_table.hpp +++ b/indexer/features_offsets_table.hpp @@ -117,6 +117,6 @@ namespace feature succinct::elias_fano m_table; - unique_ptr m_pSrc; + unique_ptr m_pReader; }; } // namespace feature diff --git a/map/framework.cpp b/map/framework.cpp index 64dff2ab42..d32ab0a18d 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -318,7 +318,7 @@ void Framework::DeleteCountryIndexes(TIndex const & index) m_routingSession.Reset(); Platform::FilesList files; Platform const & pl = GetPlatform(); - pl.GetFilesByRegExp(pl.WritablePathForFileIndexes(file), "*", files); + pl.GetFilesByRegExp(pl.WritablePathForCountryIndexes(file), "*", files); for (auto const & file : files) my::DeleteFileX(file); } diff --git a/platform/platform.cpp b/platform/platform.cpp index 021fe99289..ab89c3d7c1 100644 --- a/platform/platform.cpp +++ b/platform/platform.cpp @@ -95,9 +95,9 @@ string Platform::DeviceName() const return OMIM_OS_NAME; } -string Platform::WritablePathForFileIndexes(string const & country_name) const +string Platform::WritablePathForCountryIndexes(string const & fileName) const { - string dir = WritableDir() + country_name + my::GetNativeSeparator(); + string dir = WritableDir() + fileName + my::GetNativeSeparator(); if (!IsFileExistsByFullPath(dir)) MkDir(dir); return dir; diff --git a/platform/platform.hpp b/platform/platform.hpp index 447886c671..f90f32a01e 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -61,13 +61,15 @@ public: /// @return full path to file in user's writable directory string WritablePathForFile(string const & file) const { return WritableDir() + file; } /// @return full path to indexes directory for country file. Creates directory if it's not exists. - string WritablePathForFileIndexes(string const & country_name) const; + string WritablePathForCountryIndexes(string const & country_name) const; /// @return resource dir (on some platforms it's differ from Writable dir) string ResourcesDir() const { return m_resourcesDir; } /// Creates directory at filesystem - void MkDir(string const & directory_name) const; + void MkDir(string const & dirName) const; + + /// @TODO create join method for string concatenation /// @return path for directory with temporary files with slash at the end string TmpDir() const { return m_tmpDir; } diff --git a/platform/platform_android.cpp b/platform/platform_android.cpp index a265d060ca..aa330bc70a 100644 --- a/platform/platform_android.cpp +++ b/platform/platform_android.cpp @@ -11,7 +11,7 @@ #include "../base/string_utils.hpp" #include // for sysconf - +#include Platform::Platform() { @@ -244,9 +244,9 @@ bool Platform::GetFileSizeByName(string const & fileName, uint64_t & size) const } } -void Platform::MkDir(string const & directory_name) const +void Platform::MkDir(string const & dirName) const { - mkdir(directory_name.c_str(), 0755); + ::mkdir(dirName.c_str(), 0755); } namespace diff --git a/platform/platform_ios.mm b/platform/platform_ios.mm index 280212711a..fe8cdcca0d 100644 --- a/platform/platform_ios.mm +++ b/platform/platform_ios.mm @@ -49,9 +49,9 @@ Platform::Platform() [pool release]; } -void Platform::MkDir(string const & directory_name) const +void Platform::MkDir(string const & dirName) const { - ::mkdir(directory_name.c_str(), 0755); + ::mkdir(dirName.c_str(), 0755); } void Platform::GetFilesByRegExp(string const & directory, string const & regexp, FilesList & res) diff --git a/platform/platform_qt.cpp b/platform/platform_qt.cpp index 9a82b2ee08..1fae77cdfe 100644 --- a/platform/platform_qt.cpp +++ b/platform/platform_qt.cpp @@ -56,9 +56,9 @@ int Platform::VideoMemoryLimit() const return 20 * 1024 * 1024; } -void Platform::MkDir(string const & directory_name) const +void Platform::MkDir(string const & dirName) const { - QDir().mkdir(directory_name.c_str()); + QDir().mkdir(dirName.c_str()); } diff --git a/platform/platform_tizen.cpp b/platform/platform_tizen.cpp index 12d15da2fe..f474681476 100644 --- a/platform/platform_tizen.cpp +++ b/platform/platform_tizen.cpp @@ -43,9 +43,9 @@ Platform::Platform() m_flags[HAS_ROUTING] = true; } -void Platform::MkDir(string const & directory_name) const +void Platform::MkDir(string const & dirName) const { - Tizen::Io::Directory::Create(directory_name.c_str(), true); + Tizen::Io::Directory::Create(dirName.c_str(), true); } int Platform::CpuCores() const diff --git a/routing/osrm2feature_map.cpp b/routing/osrm2feature_map.cpp index d65a4d9652..536e4795fc 100644 --- a/routing/osrm2feature_map.cpp +++ b/routing/osrm2feature_map.cpp @@ -193,7 +193,6 @@ void OsrmFtSegMapping::GetOsrmNodes(FtSegSetT & segments, OsrmNodesT & res, vola for (auto it = segments.begin(); it != segments.end(); ++it) { OsrmMappingTypes::FtSeg const & seg = *(*it); - vector results; uint32_t nodeId = m_backwardIndex.GetNodeIdByFid(seg.m_fid); auto range = GetSegmentsRange(nodeId); @@ -281,11 +280,11 @@ void OsrmFtSegMappingBuilder::Append(OsrmNodeIdT nodeId, FtSegVectorT const & da size_t const count = data.size(); if (count == 0) - m_buffer.push_back(OsrmMappingTypes::FtSeg(OsrmMappingTypes::FtSeg::INVALID_FID, 0, 1).Store()); + m_buffer.emplace_back(OsrmMappingTypes::FtSeg(OsrmMappingTypes::FtSeg::INVALID_FID, 0, 1).Store()); else { for (size_t i = 0; i < count; ++i) - m_buffer.push_back(data[i].Store()); + m_buffer.emplace_back(data[i].Store()); } if (count > 1) @@ -295,7 +294,7 @@ void OsrmFtSegMappingBuilder::Append(OsrmNodeIdT nodeId, FtSegVectorT const & da uint32_t const off = static_cast(m_lastOffset); CHECK_EQUAL(m_lastOffset, off, ()); - m_offsets.push_back(OsrmMappingTypes::SegOffset(nodeId, off)); + m_offsets.emplace_back(OsrmMappingTypes::SegOffset(nodeId, off)); } } void OsrmFtSegMappingBuilder::Save(FilesContainerW & cont) const @@ -324,7 +323,7 @@ void OsrmFtSegMappingBuilder::Save(FilesContainerW & cont) const void OsrmFtSegBackwardIndex::Save(string const & countryName) { - string dir = GetPlatform().WritablePathForFileIndexes(countryName); + string const dir = GetPlatform().WritablePathForCountryIndexes(countryName); { string const nodesFileName = dir + countryName + FTSEG_MAPPING_BACKWARD_INDEX_NODES_EXT; string const nodesFileNameTmp = nodesFileName + EXTENSION_TMP; @@ -341,14 +340,14 @@ void OsrmFtSegBackwardIndex::Save(string const & countryName) bool OsrmFtSegBackwardIndex::Load(string const & countryName) { - string dir = GetPlatform().WritablePathForFileIndexes(countryName); + string const dir = GetPlatform().WritablePathForCountryIndexes(countryName); string const nodesName = dir + countryName + FTSEG_MAPPING_BACKWARD_INDEX_NODES_EXT; string const bitsName = dir + countryName + FTSEG_MAPPING_BACKWARD_INDEX_BITS_EXT; uint64_t size; if (!GetPlatform().GetFileSizeByFullPath(nodesName, size) || !GetPlatform().GetFileSizeByFullPath(bitsName, size)) return false; - m_pMappedNodes = unique_ptr(new MmapReader(nodesName)); - m_pMappedBits = unique_ptr(new MmapReader(bitsName)); + m_pMappedNodes.reset(new MmapReader(nodesName)); + m_pMappedBits.reset(new MmapReader(bitsName)); succinct::mapper::map(m_nodeIds, reinterpret_cast(m_pMappedNodes->Data())); succinct::mapper::map(m_rankIndex, reinterpret_cast(m_pMappedBits->Data())); @@ -368,7 +367,7 @@ void OsrmFtSegBackwardIndex::Construct(const OsrmFtSegMapping & mapping, const u return; // Generate temporary index to speedup processing - map temporaryBackwardIndex; + unordered_map temporaryBackwardIndex; for (uint32_t i = 0; i < maxNodeId; ++i) { auto indexes = mapping.GetSegmentsRange(i); @@ -398,8 +397,8 @@ void OsrmFtSegBackwardIndex::Construct(const OsrmFtSegMapping & mapping, const u // Pack and save index succinct::elias_fano_compressed_list(nodeIds).swap(m_nodeIds); succinct::rs_bit_vector(inIndex).swap(m_rankIndex); - LOG(LINFO, ("Writing section to data file", routingName)); + LOG(LINFO, ("Writing section to data file", routingName)); Save(name); } @@ -419,9 +418,9 @@ void OsrmFtSegBackwardIndex::Clear() { ClearContainer(m_nodeIds); ClearContainer(m_rankIndex); - m_table = unique_ptr(); - m_pMappedBits = nullptr; - m_pMappedNodes = nullptr; + m_table.reset(); + m_pMappedBits.reset(); + m_pMappedNodes.reset(); } } diff --git a/routing/osrm2feature_map.hpp b/routing/osrm2feature_map.hpp index 7343ff1af9..3e7889c8ad 100644 --- a/routing/osrm2feature_map.hpp +++ b/routing/osrm2feature_map.hpp @@ -3,19 +3,19 @@ #include "../coding/file_container.hpp" #include "../coding/mmap_reader.hpp" -#include "../base/scope_guard.hpp" +#include "../indexer/features_offsets_table.hpp" #include "../platform/platform.hpp" -#include "../indexer/features_offsets_table.hpp" - -#include "../3party/succinct/rs_bit_vector.hpp" +#include "../base/scope_guard.hpp" #include "../std/string.hpp" #include "../std/vector.hpp" #include "../std/unordered_map.hpp" #include "../std/utility.hpp" +#include "../3party/succinct/rs_bit_vector.hpp" + #include "../3party/succinct/elias_fano_compressed_list.hpp" #include "../defines.hpp" @@ -117,7 +117,6 @@ public: uint32_t GetNodeIdByFid(uint32_t const fid) const; void Clear(); - }; class OsrmFtSegMapping