diff --git a/generator/generator_tests/cities_ids_tests.cpp b/generator/generator_tests/cities_ids_tests.cpp index 482d1e1494..77f430f0d1 100644 --- a/generator/generator_tests/cities_ids_tests.cpp +++ b/generator/generator_tests/cities_ids_tests.cpp @@ -78,8 +78,11 @@ UNIT_CLASS_TEST(CitiesIdsTest, BuildCitiesIds) auto const worldMwmPath = testWorldId.GetInfo()->GetLocalFile().GetPath(MapOptions::Map); - indexer::FeatureIdToGeoObjectIdBimap bimap(GetDataSource()); - TEST(bimap.Load(), ()); + indexer::FeatureIdToGeoObjectIdOneWay oneWayMap(GetDataSource()); + TEST(oneWayMap.Load(), ()); + + indexer::FeatureIdToGeoObjectIdTwoWay twoWayMap(GetDataSource()); + TEST(twoWayMap.Load(), ()); std::unordered_map originalMapping; CHECK(ParseFeatureIdToTestIdMapping(worldMwmPath, originalMapping), ()); @@ -95,12 +98,13 @@ UNIT_CLASS_TEST(CitiesIdsTest, BuildCitiesIds) bool const mustExist = ftypes::IsLocalityChecker::Instance()(ft); - TEST_EQUAL(bimap.GetGeoObjectId(fid, gid), mustExist, (index, ft.GetNames())); + TEST_EQUAL(oneWayMap.GetGeoObjectId(fid, gid), mustExist, (index, ft.GetNames())); + TEST_EQUAL(twoWayMap.GetGeoObjectId(fid, gid), mustExist, (index, ft.GetNames())); if (!mustExist) return; ++numLocalities; - TEST_EQUAL(bimap.GetFeatureID(gid, receivedFid), mustExist, (index)); + TEST_EQUAL(twoWayMap.GetFeatureID(gid, receivedFid), mustExist, (index)); TEST_EQUAL(receivedFid, fid, ()); CHECK(originalMapping.find(index) != originalMapping.end(), ()); diff --git a/indexer/feature_to_osm.cpp b/indexer/feature_to_osm.cpp index ab310d85f7..031580d1bf 100644 --- a/indexer/feature_to_osm.cpp +++ b/indexer/feature_to_osm.cpp @@ -12,12 +12,13 @@ namespace indexer { -FeatureIdToGeoObjectIdBimap::FeatureIdToGeoObjectIdBimap(DataSource const & dataSource) +// FeatureIdToGeoObjectIdOneWay -------------------------------------------------------------------- +FeatureIdToGeoObjectIdOneWay::FeatureIdToGeoObjectIdOneWay(DataSource const & dataSource) : m_dataSource(dataSource), m_reader(std::unique_ptr()) { } -bool FeatureIdToGeoObjectIdBimap::Load() +bool FeatureIdToGeoObjectIdOneWay::Load() { auto const handle = indexer::FindWorld(m_dataSource); if (!handle.IsAlive()) @@ -58,7 +59,7 @@ bool FeatureIdToGeoObjectIdBimap::Load() return false; } -bool FeatureIdToGeoObjectIdBimap::GetGeoObjectId(FeatureID const & fid, base::GeoObjectId & id) +bool FeatureIdToGeoObjectIdOneWay::GetGeoObjectId(FeatureID const & fid, base::GeoObjectId & id) { if (fid.m_mwmId != m_mwmId) { @@ -66,9 +67,6 @@ bool FeatureIdToGeoObjectIdBimap::GetGeoObjectId(FeatureID const & fid, base::Ge return false; } - if (m_memAll != nullptr) - return m_memAll->GetValue(fid.m_index, id); - if (m_mapNodes == nullptr || m_mapWays == nullptr || m_mapRelations == nullptr) return false; @@ -94,33 +92,74 @@ bool FeatureIdToGeoObjectIdBimap::GetGeoObjectId(FeatureID const & fid, base::Ge return false; } -bool FeatureIdToGeoObjectIdBimap::GetFeatureID(base::GeoObjectId const & id, FeatureID & fid) +// FeatureIdToGeoObjectIdTwoWay -------------------------------------------------------------------- +FeatureIdToGeoObjectIdTwoWay::FeatureIdToGeoObjectIdTwoWay(DataSource const & dataSource) + : m_dataSource(dataSource) { - LOG(LWARNING, ("Getting GeoObjectId by FeatureId uses a lot of RAM in current implementation")); +} - if (!m_mwmId.IsAlive()) - return false; - - if (m_memAll == nullptr) +bool FeatureIdToGeoObjectIdTwoWay::Load() +{ + auto const handle = indexer::FindWorld(m_dataSource); + if (!handle.IsAlive()) { - m_memAll = std::make_unique(); - bool const success = FeatureIdToGeoObjectIdSerDes::Deserialize(*m_reader.GetPtr(), *m_memAll); - if (!success) - { - m_memAll.reset(); - LOG(LERROR, ("Could not deserialize the FeatureId to OsmId mapping into memory.")); - return false; - } + LOG(LWARNING, ("Can't find World map file.")); + return false; } - uint32_t index; - if (!m_memAll->GetKey(id, index)) - return false; + if (handle.GetId() == m_mwmId) + return true; + auto const & cont = handle.GetValue()->m_cont; + + if (!cont.IsExist(FEATURE_TO_OSM_FILE_TAG)) + { + LOG(LWARNING, ("No cities fid bimap in the world map.")); + return false; + } + + bool success = false; + try + { + FilesContainerR::TReader const reader = cont.GetReader(FEATURE_TO_OSM_FILE_TAG); + success = FeatureIdToGeoObjectIdSerDes::Deserialize(*reader.GetPtr(), m_map); + } + catch (Reader::Exception const & e) + { + LOG(LERROR, ("Can't read cities fid bimap from the world map:", e.Msg())); + return false; + } + + if (success) + { + m_mwmId = handle.GetId(); + return true; + } + + return false; +} + +bool FeatureIdToGeoObjectIdTwoWay::GetGeoObjectId(FeatureID const & fid, base::GeoObjectId & id) +{ + if (fid.m_mwmId != m_mwmId) + { + LOG(LERROR, ("Wrong mwm: feature has", fid.m_mwmId, "expected:", m_mwmId)); + return false; + } + + return m_map.GetValue(fid.m_index, id); +} + +bool FeatureIdToGeoObjectIdTwoWay::GetFeatureID(base::GeoObjectId const & id, FeatureID & fid) +{ + uint32_t index; + if (!m_map.GetKey(id, index)) + return false; fid = FeatureID(m_mwmId, index); return true; } +// FeatureIdToGeoObjectIdSerDes -------------------------------------------------------------------- // static std::string const FeatureIdToGeoObjectIdSerDes::kHeaderMagic = "mwmftosm"; FeatureIdToGeoObjectIdSerDes::Version const FeatureIdToGeoObjectIdSerDes::kLatestVersion = diff --git a/indexer/feature_to_osm.hpp b/indexer/feature_to_osm.hpp index b71a6c5392..5dce510ac0 100644 --- a/indexer/feature_to_osm.hpp +++ b/indexer/feature_to_osm.hpp @@ -28,25 +28,24 @@ class DataSource; namespace indexer { -// An in-memory implementation of the data structure behind the FeatureIdToGeoObjectIdBimap. +// An in-memory implementation of the data structure that provides a +// serializable mapping of FeatureIDs to base::GeoObjectIds and back. using FeatureIdToGeoObjectIdBimapMem = base::BidirectionalMap; -// A serializable bidirectional read-only map of FeatureIds from a single +// A unidirectional read-only map of FeatureIds from a single // mwm of a fixed version to GeoObjectIds. // Currently, only World.mwm of the latest version is supported. -class FeatureIdToGeoObjectIdBimap +class FeatureIdToGeoObjectIdOneWay { public: friend class FeatureIdToGeoObjectIdSerDes; - explicit FeatureIdToGeoObjectIdBimap(DataSource const & dataSource); + explicit FeatureIdToGeoObjectIdOneWay(DataSource const & dataSource); bool Load(); bool GetGeoObjectId(FeatureID const & fid, base::GeoObjectId & id); - bool GetFeatureID(base::GeoObjectId const & id, FeatureID & fid); - template void ForEachEntry(Fn && fn) const { @@ -75,8 +74,6 @@ private: MwmSet::MwmId m_mwmId; FilesContainerR::TReader m_reader; - std::unique_ptr m_memAll; - std::unique_ptr> m_mapNodes; std::unique_ptr> m_mapWays; std::unique_ptr> m_mapRelations; @@ -86,6 +83,39 @@ private: std::unique_ptr m_relationsReader; }; +// A bidirectional read-only map of FeatureIds from a single +// mwm of a fixed version to GeoObjectIds. +// Currently, only World.mwm of the latest version is supported. +// This class will likely be much heavier on RAM than FeatureIdToGeoObjectIdOneWay. +class FeatureIdToGeoObjectIdTwoWay +{ +public: + friend class FeatureIdToGeoObjectIdSerDes; + + explicit FeatureIdToGeoObjectIdTwoWay(DataSource const & dataSource); + + bool Load(); + + bool GetGeoObjectId(FeatureID const & fid, base::GeoObjectId & id); + + bool GetFeatureID(base::GeoObjectId const & id, FeatureID & fid); + + template + void ForEachEntry(Fn && fn) const + { + if (!m_mwmId.IsAlive()) + return; + + m_map.ForEachEntry(std::forward(fn)); + } + +private: + DataSource const & m_dataSource; + MwmSet::MwmId m_mwmId; + + FeatureIdToGeoObjectIdBimapMem m_map; +}; + // Section format. // // Versions supported Name Offset in bytes Size in bytes @@ -238,7 +268,7 @@ public: std::vector> entries; entries.reserve(map.Size()); type = NormalizedType(type); - map.ForEachEntry([&sink, &entries, type](uint32_t const fid, base::GeoObjectId gid) { + map.ForEachEntry([&entries, type](uint32_t const fid, base::GeoObjectId gid) { if (NormalizedType(gid.GetType()) == type) entries.emplace_back(fid, gid); }); @@ -257,7 +287,7 @@ public: template static void DeserializeV0(Reader & reader, HeaderV0 const & header, - FeatureIdToGeoObjectIdBimap & map) + FeatureIdToGeoObjectIdOneWay & map) { auto const nodesSize = header.m_waysOffset - header.m_nodesOffset; auto const waysSize = header.m_relationsOffset - header.m_waysOffset; diff --git a/indexer/indexer_tests/feature_to_osm_tests.cpp b/indexer/indexer_tests/feature_to_osm_tests.cpp index 918ceaeba7..77d87eb0eb 100644 --- a/indexer/indexer_tests/feature_to_osm_tests.cpp +++ b/indexer/indexer_tests/feature_to_osm_tests.cpp @@ -40,14 +40,14 @@ Entries GetEntries(Cont const & cont) return res; }; -class FeatureIdToGeoObjectIdBimapTest : public TestWithCustomMwms +class FeatureIdToGeoObjectIdTest : public TestWithCustomMwms { public: DataSource const & GetDataSource() const { return m_dataSource; } }; } // namespace -UNIT_CLASS_TEST(FeatureIdToGeoObjectIdBimapTest, Smoke) +UNIT_CLASS_TEST(FeatureIdToGeoObjectIdTest, Smoke) { Entries const kEntries = { {0, base::MakeOsmNode(123)}, @@ -80,25 +80,33 @@ UNIT_CLASS_TEST(FeatureIdToGeoObjectIdBimapTest, Smoke) FeatureIdToGeoObjectIdSerDes::Deserialize(reader, deserMem); } - indexer::FeatureIdToGeoObjectIdBimap deserM(GetDataSource()); - TEST(deserM.Load(), ()); + indexer::FeatureIdToGeoObjectIdOneWay deserOneWay(GetDataSource()); + TEST(deserOneWay.Load(), ()); + + indexer::FeatureIdToGeoObjectIdTwoWay deserTwoWay(GetDataSource()); + TEST(deserTwoWay.Load(), ()); Entries actualEntriesMem = GetEntries(deserMem); - Entries actualEntries = GetEntries(deserM); + Entries actualEntriesOneWay = GetEntries(deserOneWay); + Entries actualEntriesTwoWay = GetEntries(deserTwoWay); TEST_EQUAL(kEntries, actualEntriesMem, ()); - TEST_EQUAL(kEntries, actualEntries, ()); + TEST_EQUAL(kEntries, actualEntriesOneWay, ()); + TEST_EQUAL(kEntries, actualEntriesTwoWay, ()); for (auto const & entry : kEntries) { base::GeoObjectId gid; - TEST(deserM.GetGeoObjectId(FeatureID(testWorldId, entry.first), gid), ()); + TEST(deserOneWay.GetGeoObjectId(FeatureID(testWorldId, entry.first), gid), ()); + TEST_EQUAL(entry.second, gid, ()); + + TEST(deserTwoWay.GetGeoObjectId(FeatureID(testWorldId, entry.first), gid), ()); TEST_EQUAL(entry.second, gid, ()); } for (auto const & entry : kEntries) { FeatureID fid; - TEST(deserM.GetFeatureID(entry.second, fid), ()); + TEST(deserTwoWay.GetFeatureID(entry.second, fid), ()); TEST_EQUAL(entry.first, fid.m_index, ()); } } diff --git a/map/promo_delegate.cpp b/map/promo_delegate.cpp index 2026a72902..71a925fe66 100644 --- a/map/promo_delegate.cpp +++ b/map/promo_delegate.cpp @@ -29,7 +29,7 @@ std::string PromoDelegate::GetCityId(m2::PointD const & point) if (!m_cities) { - m_cities = std::make_unique(m_dataSource); + m_cities = std::make_unique(m_dataSource); m_cities->Load(); } diff --git a/map/promo_delegate.hpp b/map/promo_delegate.hpp index 7da68935c3..e5a823afbc 100644 --- a/map/promo_delegate.hpp +++ b/map/promo_delegate.hpp @@ -26,5 +26,6 @@ public: private: DataSource const & m_dataSource; search::CityFinder & m_cityFinder; - std::unique_ptr m_cities; + // todo(@a, @m) Drop the unique_ptr and add an IsLoaded() method? + std::unique_ptr m_cities; };