Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[generator:streets] Speedup streets building: split regions into arenas #75

Merged
merged 1 commit into from
Jan 13, 2020
Merged

[generator:streets] Speedup streets building: split regions into arenas #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 107 additions & 29 deletions generator/streets/streets_builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ namespace streets
{
StreetsBuilder::StreetsBuilder(RegionFinder const & regionFinder,
unsigned int threadsCount)
: m_regionFinder{regionFinder}, m_threadsCount{threadsCount}
: m_regionsArenas(GetArenasCount(threadsCount))
, m_featuresArenas{GetArenasCount(threadsCount)}
, m_regionFinder{regionFinder}
, m_threadsCount{threadsCount}
{
}

Expand Down Expand Up @@ -70,8 +73,10 @@ void StreetsBuilder::RegenerateAggregatedStreetsFeatures(

std::set<Street const *> processedStreets;
auto const transform = [&](FeatureBuilder & fb, uint64_t /* currPos */) {
auto street = m_streetFeatures2Streets.find(fb.GetMostGenericOsmId());
if (street == m_streetFeatures2Streets.end())
auto const osmId = fb.GetMostGenericOsmId();
auto const & featuresArena = GetFeaturesArena(osmId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а что такое арена?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Что бы уменьшить конкуренцию на глобальный контейнер, здесь он разбит на несколько контейнеров, каждый со своим мьютексом. Эти контейнеры с мьютексом и называются аренами.

auto street = featuresArena.m_streetFeatures2Streets.find(osmId);
if (street == featuresArena.m_streetFeatures2Streets.end())
return;

if (!processedStreets.insert(street->second).second)
Expand Down Expand Up @@ -131,11 +136,14 @@ void StreetsBuilder::WriteAsAggregatedStreet(FeatureBuilder & fb, Street const &
void StreetsBuilder::SaveStreetsKv(RegionGetter const & regionGetter,
std::ostream & streamStreetsKv)
{
for (auto const & region : m_regions)
for (auto const & regionsArena : m_regionsArenas)
{
auto const & regionObject = regionGetter(region.first);
CHECK(regionObject, ());
SaveRegionStreetsKv(region.second, region.first, *regionObject, streamStreetsKv);
for (auto const & region : regionsArena.m_regions)
{
auto const & regionObject = regionGetter(region.first);
CHECK(regionObject, ());
SaveRegionStreetsKv(region.second, region.first, *regionObject, streamStreetsKv);
}
}
}

Expand Down Expand Up @@ -174,17 +182,30 @@ void StreetsBuilder::AddStreetHighway(FeatureBuilder & fb)
};
StreetRegionsTracing regionsTracing(fb.GetOuterGeometry(), streetRegionInfoGetter);

std::lock_guard<std::mutex> lock{m_updateMutex};

auto && pathSegments = regionsTracing.StealPathSegments();
for (auto & segment : pathSegments)
{
auto && region = segment.m_region;
auto & street = InsertStreet(region.first, fb.GetName(), fb.GetMultilangName());
auto const osmId = pathSegments.size() == 1 ? fb.GetMostGenericOsmId() : NextOsmSurrogateId();
street.m_geometry.AddHighwayLine(osmId, std::move(segment.m_path));

m_streetFeatures2Streets.emplace(fb.GetMostGenericOsmId(), &street);
auto const osmId = fb.GetMostGenericOsmId();
auto const streetId = pathSegments.size() == 1 ? osmId : NextOsmSurrogateId();
Street const * featureStreetPtr = nullptr;

{
auto & regionsArena = GetRegionsArena(region.first);
std::lock_guard<std::mutex> lock{regionsArena.m_updateMutex};

auto & street = regionsArena.InsertStreet(region.first, fb.GetName(), fb.GetMultilangName());
street.m_geometry.AddHighwayLine(streetId, std::move(segment.m_path));
featureStreetPtr = &street;
}

{
auto & featuresArena = GetFeaturesArena(osmId);
std::lock_guard<std::mutex> lock{featuresArena.m_updateMutex};

featuresArena.m_streetFeatures2Streets.emplace(osmId, featureStreetPtr);
}
}
}

Expand All @@ -194,13 +215,24 @@ void StreetsBuilder::AddStreetArea(FeatureBuilder & fb)
if (!region)
return;

std::lock_guard<std::mutex> lock{m_updateMutex};
auto const osmId = fb.GetMostGenericOsmId();
Street const * featureStreetPtr = nullptr;

auto & street = InsertStreet(region->first, fb.GetName(), fb.GetMultilangName());
auto osmId = fb.GetMostGenericOsmId();
street.m_geometry.AddHighwayArea(osmId, fb.GetOuterGeometry());
{
auto & regionsArena = GetRegionsArena(region->first);
std::lock_guard<std::mutex> lock{regionsArena.m_updateMutex};

auto & street = regionsArena.InsertStreet(region->first, fb.GetName(), fb.GetMultilangName());
street.m_geometry.AddHighwayArea(osmId, fb.GetOuterGeometry());
featureStreetPtr = &street;
}

{
auto & featuresArena = GetFeaturesArena(osmId);
std::lock_guard<std::mutex> lock{featuresArena.m_updateMutex};

m_streetFeatures2Streets.emplace(osmId, &street);
featuresArena.m_streetFeatures2Streets.emplace(osmId, featureStreetPtr);
}
}

void StreetsBuilder::AddStreetPoint(FeatureBuilder & fb)
Expand All @@ -209,13 +241,24 @@ void StreetsBuilder::AddStreetPoint(FeatureBuilder & fb)
if (!region)
return;

std::lock_guard<std::mutex> lock{m_updateMutex};
auto const osmId = fb.GetMostGenericOsmId();
Street const * featureStreetPtr = nullptr;

{
auto & regionsArena = GetRegionsArena(region->first);
std::lock_guard<std::mutex> lock{regionsArena.m_updateMutex};

auto & street = regionsArena.InsertStreet(region->first, fb.GetName(), fb.GetMultilangName());
street.m_geometry.SetPin({fb.GetKeyPoint(), osmId});
featureStreetPtr = &street;
}

auto osmId = fb.GetMostGenericOsmId();
auto & street = InsertStreet(region->first, fb.GetName(), fb.GetMultilangName());
street.m_geometry.SetPin({fb.GetKeyPoint(), osmId});
{
auto & featuresArena = GetFeaturesArena(osmId);
std::lock_guard<std::mutex> lock{featuresArena.m_updateMutex};

m_streetFeatures2Streets.emplace(osmId, &street);
featuresArena.m_streetFeatures2Streets.emplace(osmId, featureStreetPtr);
}
}

void StreetsBuilder::AddStreetBinding(std::string && streetName, FeatureBuilder & fb,
Expand All @@ -225,10 +268,36 @@ void StreetsBuilder::AddStreetBinding(std::string && streetName, FeatureBuilder
if (!region)
return;

std::lock_guard<std::mutex> lock{m_updateMutex};
auto const osmId = NextOsmSurrogateId();

{
auto & regionsArena = GetRegionsArena(region->first);
std::lock_guard<std::mutex> lock{regionsArena.m_updateMutex};

auto & street = InsertStreet(region->first, std::move(streetName), multiLangName);
street.m_geometry.AddBinding(NextOsmSurrogateId(), fb.GetKeyPoint());
auto & street = regionsArena.InsertStreet(region->first, std::move(streetName), multiLangName);
street.m_geometry.AddBinding(osmId, fb.GetKeyPoint());
}
}

StreetsBuilder::RegionsArena & StreetsBuilder::GetRegionsArena(uint64_t regionId)
{
return m_regionsArenas[std::hash<uint64_t>{}(regionId) % m_regionsArenas.size()];
Copy link
Contributor

@maksimandrianov maksimandrianov Dec 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут коллизий не будет?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

с hash map будет сильно хуже?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нет. Это выбор арены, а вставка происходит уже внутрь контейнера арены

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

с hash map будет сильно хуже?

Так не нужно. Через вектор явнее степень контеншена.

}

StreetsBuilder::RegionsArena const & StreetsBuilder::GetRegionsArena(uint64_t regionId) const
{
return m_regionsArenas[std::hash<uint64_t>{}(regionId) % m_regionsArenas.size()];
}

StreetsBuilder::FeaturesArena & StreetsBuilder::GetFeaturesArena(base::GeoObjectId const & osmId)
{
return m_featuresArenas[std::hash<base::GeoObjectId>{}(osmId) % m_featuresArenas.size()];
}

StreetsBuilder::FeaturesArena const & StreetsBuilder::GetFeaturesArena(
base::GeoObjectId const & osmId) const
{
return m_featuresArenas[std::hash<base::GeoObjectId>{}(osmId) % m_featuresArenas.size()];
}

boost::optional<KeyValue> StreetsBuilder::FindStreetRegionOwner(m2::PointD const & point,
Expand Down Expand Up @@ -265,8 +334,8 @@ StringUtf8Multilang MergeNames(const StringUtf8Multilang & first,
return result;
}

StreetsBuilder::Street & StreetsBuilder::InsertStreet(uint64_t regionId, std::string && streetName,
StringUtf8Multilang const & multilangName)
StreetsBuilder::Street & StreetsBuilder::RegionsArena::InsertStreet(
uint64_t regionId, std::string && streetName, StringUtf8Multilang const & multilangName)
{
auto & regionStreets = m_regions[regionId];
StreetsBuilder::Street & street = regionStreets[std::move(streetName)];
Expand Down Expand Up @@ -310,7 +379,8 @@ base::JSONPtr StreetsBuilder::MakeStreetValue(uint64_t regionId, JsonValue const

base::GeoObjectId StreetsBuilder::NextOsmSurrogateId()
{
return base::GeoObjectId{base::GeoObjectId::Type::OsmSurrogate, ++m_osmSurrogateCounter};
auto id = m_osmSurrogateCounter.fetch_add(1, std::memory_order_relaxed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это дает большое укорение?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

прямо тут
static std::atomic<uint64_t> osmSurrogateCounter{0}; ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не сравнивал, но это быстрее, чем другие политики

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

прямо тут
static std::atomic<uint64_t> osmSurrogateCounter{0}; ?

не понял вопроса

Copy link
Contributor

@maksimandrianov maksimandrianov Dec 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

что бы не делать поле класса и метод можно написать функцию в cpp c static std::atomic<uint64_t> osmSurrogateCounter{0}; , но лан, это не принципиально

return base::GeoObjectId{base::GeoObjectId::Type::OsmSurrogate, id};
}

// static
Expand Down Expand Up @@ -344,5 +414,13 @@ bool StreetsBuilder::IsStreet(FeatureBuilder const & fb)

return false;
}

// static
unsigned int StreetsBuilder::GetArenasCount(unsigned int threadsCount)
{
// N ^ 2 arenas to minimize concurrency of each thread (of N threads)
// with any N - 1 other threads.
return threadsCount * threadsCount;
}
} // namespace streets
} // namespace generator
34 changes: 28 additions & 6 deletions generator/streets/streets_builder.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@

#include "base/geo_object_id.hpp"

#include <atomic>
#include <functional>
#include <memory>
#include <mutex>
#include <ostream>
#include <stdint.h>
#include <string>
#include <vector>
#include <unordered_map>

#include <boost/optional.hpp>
Expand Down Expand Up @@ -54,8 +56,28 @@ class StreetsBuilder
StringUtf8Multilang m_name;
StreetGeometry m_geometry;
};

using RegionStreets = std::unordered_map<std::string, Street>;

struct RegionsArena
{
std::unordered_map<uint64_t, RegionStreets> m_regions;
std::mutex m_updateMutex;

Street & InsertStreet(uint64_t regionId, std::string && streetName,
StringUtf8Multilang const & multilangName);
};

struct FeaturesArena
{
std::unordered_multimap<base::GeoObjectId, Street const *> m_streetFeatures2Streets;
std::mutex m_updateMutex;
};

RegionsArena & GetRegionsArena(uint64_t regionsId);
RegionsArena const & GetRegionsArena(uint64_t regionId) const;
FeaturesArena & GetFeaturesArena(base::GeoObjectId const & osmId);
FeaturesArena const & GetFeaturesArena(base::GeoObjectId const & osmId) const;
void WriteAsAggregatedStreet(feature::FeatureBuilder & fb, Street const & street,
feature::FeaturesCollector & collector) const;

Expand All @@ -70,19 +92,19 @@ class StreetsBuilder
StringUtf8Multilang const & multiLangName);
boost::optional<KeyValue> FindStreetRegionOwner(m2::PointD const & point,
bool needLocality = false);
Street & InsertStreet(uint64_t regionId, std::string && streetName,
StringUtf8Multilang const & multilangName);
base::JSONPtr MakeStreetValue(uint64_t regionId, JsonValue const & regionObject,
const StringUtf8Multilang & streetName, m2::RectD const & bbox,
m2::PointD const & pinPoint);
base::GeoObjectId NextOsmSurrogateId();

std::unordered_map<uint64_t, RegionStreets> m_regions;
std::unordered_multimap<base::GeoObjectId, Street const *> m_streetFeatures2Streets;
static unsigned int GetArenasCount(unsigned int threadsCount);

std::vector<RegionsArena> m_regionsArenas;
std::vector<FeaturesArena> m_featuresArenas;

RegionFinder m_regionFinder;
uint64_t m_osmSurrogateCounter{0};
std::atomic<uint64_t> m_osmSurrogateCounter{0};
unsigned int m_threadsCount;
std::mutex m_updateMutex;
};
} // namespace streets
} // namespace generator