-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
Conversation
auto street = m_streetFeatures2Streets.find(fb.GetMostGenericOsmId()); | ||
if (street == m_streetFeatures2Streets.end()) | ||
auto const osmId = fb.GetMostGenericOsmId(); | ||
auto const & featuresArena = GetFeaturesArena(osmId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а что такое арена?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Что бы уменьшить конкуренцию на глобальный контейнер, здесь он разбит на несколько контейнеров, каждый со своим мьютексом. Эти контейнеры с мьютексом и называются аренами.
|
||
StreetsBuilder::RegionsArena & StreetsBuilder::GetRegionsArena(uint64_t regionId) | ||
{ | ||
return m_regionsArenas[std::hash<uint64_t>{}(regionId) % m_regionsArenas.size()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тут коллизий не будет?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
с hash map будет сильно хуже?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нет. Это выбор арены, а вставка происходит уже внутрь контейнера арены
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
с hash map будет сильно хуже?
Так не нужно. Через вектор явнее степень контеншена.
@@ -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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это дает большое укорение?
There was a problem hiding this comment.
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}; ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не сравнивал, но это быстрее, чем другие политики
There was a problem hiding this comment.
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}; ?
не понял вопроса
There was a problem hiding this comment.
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}; , но лан, это не принципиально
как тестировалось? |
unittest и на глаз сверил размер файла |
@@ -28,7 +28,10 @@ namespace streets | |||
{ | |||
StreetsBuilder::StreetsBuilder(RegionFinder const & regionFinder, | |||
unsigned int threadsCount) | |||
: m_regionFinder{regionFinder}, m_threadsCount{threadsCount} | |||
: m_regionsArenas(threadsCount * threadsCount) | |||
, m_featuresArenas{threadsCount * threadsCount} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Напиши пожалуйста коммент про количество арен.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
добавил статик метод GetArenasCount()
5d33640
to
c247509
Compare
Ускорение стадии построения улиц в ~10 раз (9m vs 1h:50m).