[generator] review notes

This commit is contained in:
LaGrunge 2019-08-13 11:45:46 +03:00 committed by cc-engineering
parent e14930a6a5
commit 055b778528
6 changed files with 75 additions and 104 deletions

View file

@ -429,15 +429,9 @@ int GeneratorToolMain(int argc, char ** argv)
if (!FLAGS_geo_objects_key_value.empty())
{
geo_objects::GeoObjectsGenerator geoObjectsGenerator{FLAGS_regions_index,
FLAGS_regions_key_value,
FLAGS_geo_objects_features,
FLAGS_ids_without_addresses,
FLAGS_geo_objects_key_value,
FLAGS_verbose,
threadsCount};
if (!geoObjectsGenerator.GenerateGeoObjects())
if (!geo_objects::GenerateGeoObjects(FLAGS_regions_index, FLAGS_regions_key_value,
FLAGS_geo_objects_features, FLAGS_ids_without_addresses,
FLAGS_geo_objects_key_value, FLAGS_verbose, threadsCount))
return EXIT_FAILURE;
}

View file

@ -269,7 +269,7 @@ void AddPoisEnrichedWithHouseAddresses(GeoObjectMaintainer & geoObjectMaintainer
bool verbose, size_t threadsCount)
{
std::atomic_size_t counter{0};
std::mutex streamMutex;
auto const & view = geoObjectMaintainer.CreateView();
auto const concurrentTransformer = [&](FeatureBuilder & fb, uint64_t /* currPos */) {
@ -288,9 +288,11 @@ void AddPoisEnrichedWithHouseAddresses(GeoObjectMaintainer & geoObjectMaintainer
counter++;
if (counter % 100000 == 0)
LOG(LINFO, (counter, "pois added added"));
LOG(LINFO, (counter, "pois added"));
geoObjectMaintainer.WriteToStorage(id, JsonValue{std::move(jsonValue)});
std::lock_guard<std::mutex> lock(streamMutex);
streamPoiIdsToAddToLocalityIndex << id << "\n";
};

View file

@ -23,25 +23,10 @@ namespace generator
{
namespace geo_objects
{
GeoObjectsGenerator::GeoObjectsGenerator(std::string pathInRegionsIndex,
std::string pathInRegionsKv,
std::string pathInGeoObjectsTmpMwm,
std::string pathOutIdsWithoutAddress,
std::string pathOutGeoObjectsKv, bool verbose,
size_t threadsCount)
: m_pathInGeoObjectsTmpMwm(std::move(pathInGeoObjectsTmpMwm))
, m_pathOutPoiIdsToAddToLocalityIndex(std::move(pathOutIdsWithoutAddress))
, m_pathOutGeoObjectsKv(std::move(pathOutGeoObjectsKv))
, m_verbose(verbose)
, m_threadsCount(threadsCount)
, m_geoObjectMaintainer{m_pathOutGeoObjectsKv, RegionInfoGetterProxy(pathInRegionsIndex, pathInRegionsKv)}
{
}
GeoObjectsGenerator::GeoObjectsGenerator(
RegionInfoGetterProxy::RegionInfoGetter && regionInfoGetter,
RegionInfoGetterProxy::RegionIdGetter && regionIdGetter, std::string pathInGeoObjectsTmpMwm,
GeoObjectMaintainer::RegionInfoGetter && regionInfoGetter,
GeoObjectMaintainer::RegionIdGetter && regionIdGetter, std::string pathInGeoObjectsTmpMwm,
std::string pathOutIdsWithoutAddress, std::string pathOutGeoObjectsKv, bool verbose,
size_t threadsCount)
: m_pathInGeoObjectsTmpMwm(std::move(pathInGeoObjectsTmpMwm))
@ -49,8 +34,7 @@ GeoObjectsGenerator::GeoObjectsGenerator(
, m_pathOutGeoObjectsKv(std::move(pathOutGeoObjectsKv))
, m_verbose(verbose)
, m_threadsCount(threadsCount)
, m_geoObjectMaintainer{m_pathOutGeoObjectsKv, RegionInfoGetterProxy(std::move(regionInfoGetter),
std::move(regionIdGetter))}
, m_geoObjectMaintainer{m_pathOutGeoObjectsKv, std::move(regionInfoGetter), std::move(regionIdGetter)}
{
}
@ -99,5 +83,31 @@ bool GeoObjectsGenerator::GenerateGeoObjectsPrivate()
return true;
}
bool GenerateGeoObjects(std::string const & regionsIndex, std::string const & regionsKeyValue,
std::string const & geoObjectsFeatures,
std::string const & nodesListToIndex, std::string const & geoObjectKeyValue,
bool verbose, size_t threadsCount)
{
auto regionInfoGetter = regions::RegionInfoGetter(regionsIndex, regionsKeyValue);
LOG(LINFO, ("Size of regions key-value storage:", regionInfoGetter.GetStorage().Size()));
auto findDeepest = [&regionInfoGetter](auto && point) {
return regionInfoGetter.FindDeepest(point);
};
auto keyValueFind = [&regionInfoGetter](auto && id) {
return regionInfoGetter.GetStorage().Find(id.GetEncodedId());
};
geo_objects::GeoObjectsGenerator geoObjectsGenerator{std::move(findDeepest),
std::move(keyValueFind),
geoObjectsFeatures,
nodesListToIndex,
geoObjectKeyValue,
verbose,
threadsCount};
return geoObjectsGenerator.GenerateGeoObjects();
}
} // namespace geo_objects
} // namespace generator

View file

@ -18,12 +18,8 @@ namespace geo_objects
class GeoObjectsGenerator
{
public:
GeoObjectsGenerator(std::string pathInRegionsIndex, std::string pathInRegionsKv,
std::string pathInGeoObjectsTmpMwm, std::string pathOutIdsWithoutAddress,
std::string pathOutGeoObjectsKv, bool verbose, size_t threadsCount);
GeoObjectsGenerator(RegionInfoGetterProxy::RegionInfoGetter && regionInfoGetter,
RegionInfoGetterProxy::RegionIdGetter && regionIdGetter,
GeoObjectsGenerator(GeoObjectMaintainer::RegionInfoGetter && regionInfoGetter,
GeoObjectMaintainer::RegionIdGetter && regionIdGetter,
std::string pathInGeoObjectsTmpMwm, std::string pathOutIdsWithoutAddress,
std::string pathOutGeoObjectsKv, bool verbose, size_t threadsCount);
@ -49,5 +45,10 @@ private:
size_t m_threadsCount = 1;
GeoObjectMaintainer m_geoObjectMaintainer;
};
bool GenerateGeoObjects(std::string const & regionsIndex, std::string const & regionsKeyValue,
std::string const & geoObjectsFeatures,
std::string const & nodesListToIndex, std::string const & geoObjectKeyValue,
bool verbose, size_t threadsCount);
} // namespace geo_objects
} // namespace generator

View file

@ -11,9 +11,11 @@ namespace generator
namespace geo_objects
{
GeoObjectMaintainer::GeoObjectMaintainer(std::string const & pathOutGeoObjectsKv,
RegionInfoGetterProxy && regionInfoGetter)
RegionInfoGetter && regionInfoGetter,
RegionIdGetter && regionIdGetter)
: m_geoObjectsKvStorage{InitGeoObjectsKv(pathOutGeoObjectsKv)}
, m_regionInfoGetter{std::move(regionInfoGetter)}
, m_regionIdGetter(std::move(regionIdGetter))
{
}
@ -25,7 +27,7 @@ std::fstream GeoObjectMaintainer::InitGeoObjectsKv(std::string const & pathOutGe
if (!result)
MYTHROW(Reader::OpenException, ("Failed to open file", pathOutGeoObjectsKv));
return std::move(result);
return result;
}
void UpdateCoordinates(m2::PointD const & point, base::JSONPtr & json)
@ -65,10 +67,6 @@ base::JSONPtr AddAddress(std::string const & street, std::string const & house,
ToJSONObject(*properties, "rank", kHouseOrPoiRank);
ToJSONObject(*properties, "dref", KeyValueStorage::SerializeDref(regionKeyValue.first));
// auto locales = json_object_get(result.get(), "locales");
// auto locales = json_object_get(result.get(), "locales");
// auto en = json_object_get(result.get(), "en");
// todo(maksimandrianov): Add en locales.
return result;
}
@ -77,26 +75,25 @@ void GeoObjectMaintainer::StoreAndEnrich(feature::FeatureBuilder & fb)
if (!GeoObjectsFilter::IsBuilding(fb) && !GeoObjectsFilter::HasHouse(fb))
return;
auto regionKeyValue = m_regionInfoGetter.FindDeepest(fb.GetKeyPoint());
auto regionKeyValue = m_regionInfoGetter(fb.GetKeyPoint());
if (!regionKeyValue)
return;
auto const id = fb.GetMostGenericOsmId();
auto jsonValue = AddAddress(fb.GetParams().GetStreet(), fb.GetParams().house.Get(),
fb.GetKeyPoint(), fb.GetMultilangName(), *regionKeyValue);
{
std::lock_guard<std::mutex> lock(m_updateMutex);
std::lock_guard<std::mutex> lock(m_updateMutex);
auto const it = m_geoId2GeoData.emplace(
std::make_pair(id, GeoObjectData{fb.GetParams().GetStreet(), fb.GetParams().house.Get(),
base::GeoObjectId(regionKeyValue->first)}));
auto const it = m_geoId2GeoData.emplace(
std::make_pair(id, GeoObjectData{fb.GetParams().GetStreet(), fb.GetParams().house.Get(),
base::GeoObjectId(regionKeyValue->first)}));
// Duplicate ID's are possible
if (!it.second)
return;
m_geoObjectsKvStorage << KeyValueStorage::SerializeFullLine(id.GetEncodedId(),
JsonValue{std::move(jsonValue)});
// Duplicate ID's are possible
if (!it.second)
return;
}
WriteToStorage(id, JsonValue{std::move(jsonValue)});
}
void GeoObjectMaintainer::WriteToStorage(base::GeoObjectId id, JsonValue && value)
@ -106,7 +103,6 @@ void GeoObjectMaintainer::WriteToStorage(base::GeoObjectId id, JsonValue && valu
}
// GeoObjectMaintainer::GeoObjectsView
base::JSONPtr GeoObjectMaintainer::GeoObjectsView::GetFullGeoObject(
m2::PointD point,
std::function<bool(GeoObjectMaintainer::GeoObjectData const &)> && pred) const
@ -123,7 +119,7 @@ base::JSONPtr GeoObjectMaintainer::GeoObjectsView::GetFullGeoObject(
continue;
auto regionJsonValue = m_regionInfoGetter.FindById(geoData.m_regionId);
auto regionJsonValue = m_regionIdGetter(geoData.m_regionId);
if (!regionJsonValue)
return {};
@ -142,7 +138,7 @@ base::JSONPtr GeoObjectMaintainer::GeoObjectsView::GetFullGeoObjectWithoutNameAn
return {};
auto const geoData = it->second;
auto regionJsonValue = m_regionInfoGetter.FindById(geoData.m_regionId);
auto regionJsonValue = m_regionIdGetter(geoData.m_regionId);
if (!regionJsonValue)
return {};

View file

@ -27,51 +27,14 @@ namespace generator
{
namespace geo_objects
{
class RegionInfoGetterProxy
{
public:
using RegionInfoGetter = std::function<boost::optional<KeyValue>(m2::PointD const & pathPoint)>;
using RegionIdGetter = std::function<std::shared_ptr<JsonValue>(base::GeoObjectId id)>;
RegionInfoGetterProxy(std::string const & pathInRegionsIndex, std::string const & pathInRegionsKv)
{
m_regionInfoGetter = regions::RegionInfoGetter(pathInRegionsIndex, pathInRegionsKv);
LOG(LINFO, ("Size of regions key-value storage:", m_regionInfoGetter->GetStorage().Size()));
}
explicit RegionInfoGetterProxy(RegionInfoGetter && regionInfoGetter, RegionIdGetter && regionIdGetter)
: m_externalInfoGetter(std::move(regionInfoGetter))
, m_externalRegionIdGetter(std::move(regionIdGetter))
{
LOG(LINFO, ("External regions info provided"));
}
boost::optional<KeyValue> FindDeepest(m2::PointD const & point) const
{
return m_regionInfoGetter ? m_regionInfoGetter->FindDeepest(point)
: m_externalInfoGetter->operator()(point);
}
std::shared_ptr<JsonValue> FindById(base::GeoObjectId id) const
{
return m_externalRegionIdGetter ? m_externalRegionIdGetter->operator()(id) :
m_regionInfoGetter->GetStorage().Find(id.GetEncodedId());
}
private:
boost::optional<regions::RegionInfoGetter> m_regionInfoGetter;
boost::optional<RegionInfoGetter> m_externalInfoGetter;
boost::optional<RegionIdGetter> m_externalRegionIdGetter;
};
void UpdateCoordinates(m2::PointD const & point, base::JSONPtr & json);
class GeoObjectMaintainer
{
public:
using RegionInfoGetter = std::function<boost::optional<KeyValue>(m2::PointD const & pathPoint)>;
using RegionIdGetter = std::function<std::shared_ptr<JsonValue>(base::GeoObjectId id)>;
struct GeoObjectData
{
std::string m_street;
@ -86,10 +49,12 @@ public:
{
public:
GeoObjectsView(GeoIndex const & geoIndex, GeoId2GeoData const & geoId2GeoData,
RegionInfoGetterProxy const & regionInfoGetter, std::mutex & updateMutex)
RegionInfoGetter const & regionInfoGetter, RegionIdGetter const & regionIdGetter,
std::mutex & updateMutex)
: m_geoIndex(geoIndex)
, m_geoId2GeoData(geoId2GeoData)
, m_regionInfoGetter(regionInfoGetter)
, m_regionIdGetter(regionIdGetter)
, m_lock(updateMutex, std::defer_lock)
{
CHECK(m_lock.try_lock(), ("Cannot create GeoObjectView on locked mutex"));
@ -116,12 +81,13 @@ public:
private:
GeoIndex const & m_geoIndex;
GeoId2GeoData const & m_geoId2GeoData;
RegionInfoGetterProxy const & m_regionInfoGetter;
RegionInfoGetter const & m_regionInfoGetter;
RegionIdGetter const & m_regionIdGetter;
std::unique_lock<std::mutex> m_lock;
};
GeoObjectMaintainer(std::string const & pathOutGeoObjectsKv,
RegionInfoGetterProxy && regionInfoGetter);
GeoObjectMaintainer(std::string const & pathOutGeoObjectsKv, RegionInfoGetter && regionInfoGetter,
RegionIdGetter && regionIdGetter);
void SetIndex(GeoIndex && index) { m_index = std::move(index); }
@ -130,9 +96,10 @@ public:
size_t Size() const { return m_geoId2GeoData.size(); }
const GeoObjectsView CreateView()
GeoObjectsView CreateView()
{
return GeoObjectsView(m_index, m_geoId2GeoData, m_regionInfoGetter, m_updateMutex);
return GeoObjectsView(m_index, m_geoId2GeoData, m_regionInfoGetter, m_regionIdGetter,
m_updateMutex);
}
private:
@ -143,7 +110,8 @@ private:
std::mutex m_storageMutex;
GeoIndex m_index;
RegionInfoGetterProxy m_regionInfoGetter;
RegionInfoGetter m_regionInfoGetter;
RegionIdGetter m_regionIdGetter;
GeoId2GeoData m_geoId2GeoData;
};
} // namespace geo_objects