Review fixes.

This commit is contained in:
Yuri Gorshenin 2016-06-03 00:29:19 +03:00
parent 573021d809
commit 85ee45b482
6 changed files with 73 additions and 18 deletions

View file

@ -6,7 +6,6 @@
#include "std/cstring.hpp"
#ifndef OMIM_OS_WINDOWS
#include <errno.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/mman.h>
@ -20,6 +19,7 @@
#include <windows.h>
#endif
#include <errno.h>
template <class TSource, class InfoT> void Read(TSource & src, InfoT & i)
{
@ -124,7 +124,17 @@ void MappedFile::Open(string const & fName)
#else
m_fd = open(fName.c_str(), O_RDONLY | O_NONBLOCK);
if (m_fd == -1)
MYTHROW(Reader::OpenException, ("Can't open file:", fName, ", reason:", strerror(errno)));
{
if (errno == EMFILE || errno == ENFILE)
{
MYTHROW(Reader::TooManyFilesException,
("Can't open file:", fName, ", reason:", strerror(errno)));
}
else
{
MYTHROW(Reader::OpenException, ("Can't open file:", fName, ", reason:", strerror(errno)));
}
}
#endif
}

View file

@ -19,6 +19,7 @@ public:
DECLARE_EXCEPTION(OpenException, Exception);
DECLARE_EXCEPTION(SizeException, Exception);
DECLARE_EXCEPTION(ReadException, Exception);
DECLARE_EXCEPTION(TooManyFilesException, Exception);
virtual ~Reader() {}
virtual uint64_t Size() const = 0;

View file

@ -20,14 +20,23 @@ MwmValue::MwmValue(LocalCountryFile const & localFile)
: m_cont(platform::GetCountryReader(localFile, MapOptions::Map)), m_file(localFile)
{
m_factory.Load(m_cont);
}
void MwmValue::SetTable(MwmInfoEx & info)
{
auto const version = GetHeader().GetFormat();
if (version < version::Format::v5)
;
else if (version == version::Format::v5)
m_table = feature::FeaturesOffsetsTable::CreateIfNotExistsAndLoad(m_file, m_cont);
else
m_table = feature::FeaturesOffsetsTable::Load(m_cont);
return;
m_table = info.m_table.lock();
if (!m_table)
{
if (version == version::Format::v5)
m_table = feature::FeaturesOffsetsTable::CreateIfNotExistsAndLoad(m_file, m_cont);
else
m_table = feature::FeaturesOffsetsTable::Load(m_cont);
info.m_table = m_table;
}
}
//////////////////////////////////////////////////////////////////////////////////
@ -42,7 +51,7 @@ unique_ptr<MwmInfo> Index::CreateInfo(platform::LocalCountryFile const & localFi
if (!h.IsMWMSuitable())
return nullptr;
auto info = make_unique<MwmInfo>();
auto info = make_unique<MwmInfoEx>();
info->m_limitRect = h.GetBounds();
pair<int, int> const scaleR = h.GetScaleRange();
@ -50,7 +59,7 @@ unique_ptr<MwmInfo> Index::CreateInfo(platform::LocalCountryFile const & localFi
info->m_maxScale = static_cast<uint8_t>(scaleR.second);
info->m_version = value.GetMwmVersion();
return info;
return unique_ptr<MwmInfo>(move(info));
}
unique_ptr<MwmSet::MwmValueBase> Index::CreateValue(MwmInfo & info) const
@ -58,6 +67,7 @@ unique_ptr<MwmSet::MwmValueBase> Index::CreateValue(MwmInfo & info) const
// Create a section with rank table if it does not exist.
platform::LocalCountryFile const & localFile = info.GetLocalFile();
unique_ptr<MwmValue> p(new MwmValue(localFile));
p->SetTable(dynamic_cast<MwmInfoEx &>(info));
ASSERT(p->GetHeader().IsMWMSuitable(), ());
return unique_ptr<MwmSet::MwmValueBase>(move(p));
}
@ -75,10 +85,8 @@ bool Index::DeregisterMap(CountryFile const & countryFile) { return Deregister(c
Index::FeaturesLoaderGuard::FeaturesLoaderGuard(Index const & parent, MwmId const & id)
: m_handle(parent.GetMwmHandleById(id))
,
/// @note This guard is suitable when mwm is loaded
m_vector(m_handle.GetValue<MwmValue>()->m_cont, m_handle.GetValue<MwmValue>()->GetHeader(),
m_handle.GetValue<MwmValue>()->m_table.get())
, m_vector(m_handle.GetValue<MwmValue>()->m_cont, m_handle.GetValue<MwmValue>()->GetHeader(),
m_handle.GetValue<MwmValue>()->m_table.get())
{
}

View file

@ -19,6 +19,24 @@
#include "std/limits.hpp"
#include "std/utility.hpp"
#include "std/vector.hpp"
#include "std/weak_ptr.hpp"
class MwmInfoEx : public MwmInfo
{
private:
friend class Index;
friend class MwmValue;
// weak_ptr is needed here to access offsets table in already
// instantiated MwmValue-s for the MWM, including MwmValues in the
// MwmSet's cache. We can't use shared_ptr because of offsets table
// must be removed as soon as the last corresponding MwmValue is
// destroyed. Also, note that this value must be used and modified
// only in MwmValue::SetTable() method, which, in turn, is called
// only in the MwmSet critical section, protected by a lock. So,
// there's an implicit synchronization on this field.
weak_ptr<feature::FeaturesOffsetsTable> m_table;
};
class MwmValue : public MwmSet::MwmValueBase
{
@ -26,9 +44,11 @@ public:
FilesContainerR const m_cont;
IndexFactory m_factory;
platform::LocalCountryFile const m_file;
unique_ptr<feature::FeaturesOffsetsTable> m_table;
shared_ptr<feature::FeaturesOffsetsTable> m_table;
explicit MwmValue(platform::LocalCountryFile const & localFile);
void SetTable(MwmInfoEx & info);
inline feature::DataHeader const & GetHeader() const { return m_factory.GetHeader(); }
inline version::MwmVersion const & GetMwmVersion() const { return m_factory.GetMwmVersion(); }
@ -246,6 +266,7 @@ public:
}
/// Guard for loading features from particular MWM by demand.
/// @note This guard is suitable when mwm is loaded.
class FeaturesLoaderGuard
{
public:

View file

@ -1,7 +1,7 @@
#include "indexer/mwm_set.hpp"
#include "indexer/scales.hpp"
#include "defines.hpp"
#include "coding/reader.hpp"
#include "base/assert.hpp"
#include "base/exception.hpp"
@ -11,6 +11,7 @@
#include "std/algorithm.hpp"
#include "std/sstream.hpp"
#include "defines.hpp"
using platform::CountryFile;
using platform::LocalCountryFile;
@ -162,6 +163,14 @@ bool MwmSet::DeregisterImpl(MwmId const & id, EventList & events)
SetStatus(*info, MwmInfo::STATUS_DEREGISTERED, events);
vector<shared_ptr<MwmInfo>> & infos = m_info[info->GetCountryName()];
infos.erase(remove(infos.begin(), infos.end(), info), infos.end());
for (auto it = m_cache.begin(); it != m_cache.end(); ++it)
{
if (it->first == id)
{
m_cache.erase(it);
break;
}
}
return true;
}
@ -284,6 +293,12 @@ unique_ptr<MwmSet::MwmValueBase> MwmSet::LockValueImpl(MwmId const & id, EventLi
{
return CreateValue(*info);
}
catch (Reader::TooManyFilesException const & ex)
{
LOG(LERROR, ("Too many open files, can't open:", info->GetCountryName()));
--info->m_numRefs;
return nullptr;
}
catch (exception const & ex)
{
LOG(LERROR, ("Can't create MWMValue for", info->GetCountryName(), "Reason", ex.what()));

View file

@ -24,8 +24,8 @@
class MwmInfo
{
public:
friend class MwmSet;
friend class Index;
friend class MwmSet;
enum MwmTypeT
{
@ -69,7 +69,7 @@ public:
/// Returns the lock counter value for test needs.
uint8_t GetNumRefs() const { return m_numRefs; }
private:
protected:
inline Status SetStatus(Status status)
{
Status result = m_status;
@ -79,7 +79,7 @@ private:
platform::LocalCountryFile m_file; ///< Path to the mwm file.
atomic<Status> m_status; ///< Current country status.
uint8_t m_numRefs; ///< Number of active handles.
uint32_t m_numRefs; ///< Number of active handles.
};
class MwmSet