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

Avoid if call in the __parallel_merge_submitter_large::run_parallel_merge() #1980

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

SergeyKopienko
Copy link
Contributor

In this PR we remove additional if call from __parallel_merge_submitter_large::run_parallel_merge()
Our experience shows that it should work faster.

…avoid if call in the __parallel_merge_submitter_large::run_parallel_merge()

Signed-off-by: Sergey Kopienko <[email protected]>
@@ -283,6 +283,15 @@ struct __parallel_merge_submitter_large<_IdType, _CustomName,
});
}

template <typename _Rng1, typename _Rng2, typename _Compare>
inline static _split_point_t<_IdType>
__find_start_point_w(const _Rng1& __rng1, const _Rng2& __rng2, const _split_point_t<_IdType>& __sp_left,
Copy link
Contributor

@MikeDvorskiy MikeDvorskiy Dec 27, 2024

Choose a reason for hiding this comment

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

What's a purpose of __find_start_point_w? It seems, it does not any additional things, beside re-calling __find_start_point...

@MikeDvorskiy
Copy link
Contributor

MikeDvorskiy commented Dec 27, 2024

Our experience shows that it should work faster.

Is it fair in case -O3 as well?

Comment on lines +323 to +327
const _split_point_t<_IdType> __start =
__global_idx % __nd_range_params.steps_between_two_base_diags != 0
? __find_start_point_w(__rng1, __rng2, __base_diagonals_sp_global_ptr[__diagonal_idx],
__base_diagonals_sp_global_ptr[__diagonal_idx + 1], __i_elem, __comp)
: __base_diagonals_sp_global_ptr[__diagonal_idx];
Copy link
Contributor

Choose a reason for hiding this comment

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

From a performance perspective, as far as I know a ternary operator (?) is equivalent to an if statement. Both end up as branches.

Is this PR intended to clean up the code to be improve readability or performance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants