Skip to content

shp::sort() - merge instead 2nd sort #740

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

Merged
merged 3 commits into from
May 24, 2024

Conversation

mateuszpn
Copy link
Contributor

No description provided.

@mateuszpn mateuszpn added the enhancement New feature or request label Apr 18, 2024
@BenBrock
Copy link
Contributor

Looking at this code again, it seems like we could use a parallel_for instead of a single_task here, as I originally wrote it. Do you want to try switching to parallel_for? It should be embarrassingly parallel.

Copy link
Contributor

@lslusarczyk lslusarczyk left a comment

Choose a reason for hiding this comment

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

ok, i've self applied some comments and did refactor in #759

for (int s = 0; s < _segments / 2; s++) {

std::size_t l = (2 * s + 2 < _segments) ? chunks_ind[2 * s + 2]
: sorted_seg_sizes[t];
Copy link
Contributor

Choose a reason for hiding this comment

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

sorted_seg_sizes is not initialized, fix it please

events.clear();
std::size_t v = 0;
for (std::size_t i = 0; i < n_segments; i++) {
v += (t == 0) ? splitter_indices[i][0]
Copy link
Contributor

Choose a reason for hiding this comment

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

these are push_positions[i][t], no need to recalculate it

@lslusarczyk lslusarczyk merged commit b5dadfc into oneapi-src:main May 24, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants