Review fixes.

3 files changed, 49 insertions, 45 deletions

diff --git a/indexer/altitude_loader.cpp b/indexer/altitude_loader.cpp
index c25fef6346..84ffc8a72d 100644
--- a/indexer/altitude_loader.cpp
+++ b/indexer/altitude_loader.cpp
@@ -12,46 +12,44 @@
 
 namespace
 {
-void ReadBuffer(ReaderSource<FilesContainerR::TReader> & rs, vector<char> & buf)
+void ReadBuffer(ReaderSource<FilesContainerR::TReader> & src, vector<char> & buf)
 {
   uint32_t bufSz = 0;
-  rs.Read(&bufSz, sizeof(bufSz));
-  if (bufSz > rs.Size() + rs.Pos())
+  src.Read(&bufSz, sizeof(bufSz));
+  if (bufSz > src.Size() + src.Pos())
   {
     ASSERT(false, ());
     return;
   }
   buf.clear();
   buf.resize(bufSz);
-  rs.Read(buf.data(), bufSz);
+  src.Read(buf.data(), bufSz);
 }
 } // namespace
 
 namespace feature
 {
-AltitudeLoader::AltitudeLoader(MwmValue const * mwmValue)
+AltitudeLoader::AltitudeLoader(MwmValue const & mwmValue)
 {
-  if (!mwmValue || mwmValue->GetHeader().GetFormat() < version::Format::v8 )
+  if (mwmValue.GetHeader().GetFormat() < version::Format::v8 )
     return;
 
-  if (!mwmValue->m_cont.IsExist(ALTITUDES_FILE_TAG))
+  if (!mwmValue.m_cont.IsExist(ALTITUDES_FILE_TAG))
     return;
 
   try
   {
-    m_reader = make_unique<FilesContainerR::TReader>(mwmValue->m_cont.GetReader(ALTITUDES_FILE_TAG));
-    ReaderSource<FilesContainerR::TReader> rs(*m_reader);
-    m_header.Deserialize(rs);
-
-    // Reading rs_bit_vector with altitude availability information.
-    ReadBuffer(rs, m_altitudeAvailabilitBuf);
-    m_altitudeAvailability = make_unique<succinct::rs_bit_vector>();
-    succinct::mapper::map(*m_altitudeAvailability, m_altitudeAvailabilitBuf.data());
-
-    // Reading table with altitude ofsets for features.
-    ReadBuffer(rs, m_featureTableBuf);
-    m_featureTable = make_unique<succinct::elias_fano>();
-    succinct::mapper::map(*m_featureTable, m_featureTableBuf.data());
+    m_reader = make_unique<FilesContainerR::TReader>(mwmValue.m_cont.GetReader(ALTITUDES_FILE_TAG));
+    ReaderSource<FilesContainerR::TReader> src(*m_reader);
+    m_header.Deserialize(src);
+
+    // Reading src_bit_vector with altitude availability information.
+    ReadBuffer(src, m_altitudeAvailabilitBuf);
+    succinct::mapper::map(m_altitudeAvailability, m_altitudeAvailabilitBuf.data());
+
+    // Reading table with altitude offsets for features.
+    ReadBuffer(src, m_featureTableBuf);
+    succinct::mapper::map(m_featureTable, m_featureTableBuf.data());
   }
   catch (Reader::OpenException const & e)
   {
@@ -65,24 +63,28 @@ bool AltitudeLoader::IsAvailable() const
   return m_header.minAltitude != kInvalidAltitude && m_header.altitudeInfoOffset != 0;
 }
 
-TAltitudes AltitudeLoader::GetAltitude(uint32_t featureId, size_t pointCount) const
+TAltitudes const & AltitudeLoader::GetAltitudes(uint32_t featureId, size_t pointCount) const
 {
   if (m_header.altitudeInfoOffset == 0)
   {
     // The version of mwm is less then version::Format::v8 or there's no altitude section in mwm.
-    return TAltitudes();
+    return m_dummy;
   }
 
-  if (!(*m_altitudeAvailability)[featureId])
+  auto const it = m_cache.find(featureId);
+  if (it != m_cache.end())
+    return it->second;
+
+  if (!m_altitudeAvailability[featureId])
   {
     LOG(LINFO, ("Feature featureId =", featureId, "does not contain any altitude information."));
-    return TAltitudes();
+    return m_cache.insert(make_pair(featureId, TAltitudes())).first->second;
   }
 
-  uint64_t const r = m_altitudeAvailability->rank(featureId);
-  CHECK_LESS(r, m_altitudeAvailability->size(), (featureId));
-  uint64_t const offset = m_featureTable->select(r);
-  CHECK_LESS_OR_EQUAL(offset, m_featureTable->size(), (featureId));
+  uint64_t const r = m_altitudeAvailability.rank(featureId);
+  CHECK_LESS(r, m_altitudeAvailability.size(), (featureId));
+  uint64_t const offset = m_featureTable.select(r);
+  CHECK_LESS_OR_EQUAL(offset, m_featureTable.size(), (featureId));
 
   uint64_t const altitudeInfoOffsetInSection = m_header.altitudeInfoOffset + offset;
   CHECK_LESS(altitudeInfoOffsetInSection, m_reader->Size(), ());
@@ -90,17 +92,16 @@ TAltitudes AltitudeLoader::GetAltitude(uint32_t featureId, size_t pointCount) co
   try
   {
     Altitude a;
-    ReaderSource<FilesContainerR::TReader> rs(*m_reader);
-    rs.Skip(altitudeInfoOffsetInSection);
-    a.Deserialize(m_header.minAltitude, pointCount, rs);
+    ReaderSource<FilesContainerR::TReader> src(*m_reader);
+    src.Skip(altitudeInfoOffsetInSection);
+    a.Deserialize(m_header.minAltitude, pointCount, src);
 
-    // @TODO(bykoianko) Considers using move semantic for returned value here.
-    return a.GetAltitudes();
+    return m_cache.insert(make_pair(featureId, a.GetAltitudes())).first->second;
   }
   catch (Reader::OpenException const & e)
   {
     LOG(LERROR, ("Error while getting mwm data", e.Msg()));
-    return TAltitudes();
+    return m_cache.insert(make_pair(featureId, TAltitudes())).first->second;
   }
 }
 } // namespace feature
diff --git a/indexer/altitude_loader.hpp b/indexer/altitude_loader.hpp
index cd2d1efbb4..4588e620c9 100644
--- a/indexer/altitude_loader.hpp
+++ b/indexer/altitude_loader.hpp
@@ -2,27 +2,29 @@
 #include "indexer/feature_altitude.hpp"
 #include "indexer/index.hpp"
 
-#include "3party/succinct/rs_bit_vector.hpp"
-
 #include "std/unique_ptr.hpp"
 #include "std/vector.hpp"
 
+#include "3party/succinct/rs_bit_vector.hpp"
+
 namespace feature
 {
 class AltitudeLoader
 {
 public:
-  explicit AltitudeLoader(MwmValue const * mwmValue);
+  explicit AltitudeLoader(MwmValue const & mwmValue);
 
-  TAltitudes GetAltitude(uint32_t featureId, size_t pointCount) const;
+  TAltitudes const & GetAltitudes(uint32_t featureId, size_t pointCount) const;
   bool IsAvailable() const;
 
 private:
   vector<char> m_altitudeAvailabilitBuf;
   vector<char> m_featureTableBuf;
-  unique_ptr<succinct::rs_bit_vector> m_altitudeAvailability;
-  unique_ptr<succinct::elias_fano> m_featureTable;
+  succinct::rs_bit_vector m_altitudeAvailability;
+  succinct::elias_fano m_featureTable;
   unique_ptr<FilesContainerR::TReader> m_reader;
+  mutable map<uint32_t, TAltitudes> m_cache;
+  TAltitudes const m_dummy;
   AltitudeHeader m_header;
 };
 } // namespace feature
diff --git a/indexer/feature_altitude.hpp b/indexer/feature_altitude.hpp
index 52f1032f53..8cb8502501 100644
--- a/indexer/feature_altitude.hpp
+++ b/indexer/feature_altitude.hpp
@@ -11,13 +11,14 @@ namespace feature
 {
 using TAltitude = int16_t;
 using TAltitudes = vector<feature::TAltitude>;
-using TAltitudeSectionVersion = uint16_t;
 using TAltitudeSectionOffset = uint32_t;
 
 TAltitude constexpr kInvalidAltitude = numeric_limits<TAltitude>::min();
 
 struct AltitudeHeader
 {
+  using TAltitudeSectionVersion = uint16_t;
+
   AltitudeHeader()
   {
     Reset();
@@ -109,10 +110,10 @@ public:
 
 private:
   /// \note |m_altitudes| is a vector of feature point altitudes. There's two posibilities:
-  /// * |m_altitudes| is empty. It means no altitude information defines.
-  /// * size of |m_altitudes| is equal to feature point number. In that case every item of
-  /// |m_altitudes| defines altitude in meters for every feature point. If altitude is not defined
-  ///  for some feature point corresponding vector items are equel to |kInvalidAltitude|
+  /// * |m_altitudes| is empty. It means there is no altitude information for this feature.
+  /// * size of |m_pointAlt| is equal to the number of this feature's points.
+  /// In this case the i'th element of |m_pointAlt| corresponds to the altitude of the
+  /// i'th point of the feature and is set to kInvalidAltitude when there is no information about the point.
   TAltitudes m_altitudes;
 };
 }  // namespace feature