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.

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:regions] Speedup locality index building: parallel regions stripping #79

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

cc-engineering
Copy link
Contributor

@cc-engineering cc-engineering commented Dec 28, 2019

Ускорение генерации regions_index.locidx.
Некоторое регионы требую большого объёма вычислений. Такие объекты обрабатываются параллельно.

Сам по себе PR ускоряет генерацию regions_index.locidx лишь 3m:30s (27m vs 30m:30s), но совместно с PR #78 генерация сокращается в 4 раза (7m:15s vs 30m:30s)

@@ -78,7 +80,8 @@ class GeometryHolder

bool NeedProcessTriangles() const { return !m_trgInner || m_buffer.m_innerTrg.empty(); }

bool TryToMakeStrip(Points & points)
bool TryToMakeStrip(Points & points,
base::thread_pool::computational::ThreadPool * threadPool = nullptr)
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.

++

@@ -35,15 +36,51 @@ bool FindSingleStripForIndex(size_t i, size_t n, IsVisibleF isVisible)

// If polygon with n vertices is a single strip, return the start index of the strip or n otherwise.
template <typename IsVisibleF>
size_t FindSingleStrip(size_t n, IsVisibleF isVisible)
size_t FindSingleStrip(size_t n, IsVisibleF isVisible,
base::thread_pool::computational::ThreadPool * threadPool = nullptr)
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.

Тогда не удобно её вызывать из TryToMakeStrip(), нужен вызов методов по условию ?:. Переход на два метода вызывает только неудобство и неоправданное нагромождение кода (что усложняет чтение).

Copy link
Contributor

Choose a reason for hiding this comment

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

TryToMakeStrip() тоже предлагается разбить на две, там же есть комментарий

Copy link
Contributor Author

Choose a reason for hiding this comment

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

добавил ф-цию с стратегией

size_t const threadsCount =
std::min(std::max(size_t{2}, tasksCount / tasksPerThread), threadPool->Size());

std::vector<std::future<void>> threadTasks{};
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.

Заменил на вызов PerformParallelWorks()

@cc-engineering cc-engineering force-pushed the generator.regions.locality-parallel-strip branch from 7a556b2 to b8245dd Compare January 10, 2020 09:13
@cc-engineering cc-engineering force-pushed the generator.regions.locality-parallel-strip branch from b8245dd to 2aaf3a0 Compare January 13, 2020 14:54
return n;
}

// If polygon with n vertices is a single strip, return the start index of the strip or n otherwise.
template <typename IsVisibleF>
Copy link
Contributor

Choose a reason for hiding this comment

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

s/IsVisibleF/IsVisibleFn/

Copy link
Contributor

@syershov syershov left a comment

Choose a reason for hiding this comment

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

Хорошо получилось

@LaGrunge LaGrunge merged commit 895b474 into master Jan 15, 2020
@cc-engineering cc-engineering deleted the generator.regions.locality-parallel-strip branch January 15, 2020 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants