[k2] make sort functions implementation runtime specific #1208
Conversation
| task_t<void> f$asort(array<T> &a, int64_t flag = SORT_REGULAR) { | ||
| switch (flag) { | ||
| case SORT_REGULAR: | ||
| co_return co_await array_functions_impl_::sort<task_t<void>>(a, array_functions_impl_::sort_compare<T>(), false); | ||
| case SORT_NUMERIC: | ||
| co_return co_await array_functions_impl_::sort<task_t<void>>(a, array_functions_impl_::sort_compare_numeric<T>(), false); | ||
| case SORT_STRING: | ||
| co_return co_await array_functions_impl_::sort<task_t<void>>(a, array_functions_impl_::sort_compare_string<T>(), false); |
There was a problem hiding this comment.
Why is it interruptible?
I thought that all sort function that are not receiving callable as argument should not be interruptible.
Am I wrong?
There was a problem hiding this comment.
You are right
I did it interruptible for easy implementation. But maybe it would be better to make it a right way in this pr
There was a problem hiding this comment.
If it's a big deal then it's OK to leave as is.
astrophysik
force-pushed
the
vsadokhov/support-interruptible-sort
branch
from
fdbc659 to
01a0899
Compare
|
|
||
| template<typename T, typename Comparator> | ||
| requires(std::invocable<Comparator, T, T>) task_t<void> sort(T *begin_init, T *end_init, Comparator compare) noexcept { | ||
| auto compare_call = [compare]<typename U>(U lhs, U rhs) -> task_t<int64_t> { |
There was a problem hiding this comment.
Lambda coroutine should not have captured values as they're not saved in a coroutine frame
| T *j = end; | ||
|
|
||
| while (true) { | ||
| while (i < j && (co_await compare_call(*begin, *i)) > 0) { |
There was a problem hiding this comment.
We can get rid of co_await completely unless Comparator is async function.
There was a problem hiding this comment.
Yes, we can, but it require to create copy paste function without co_await. Is it good decision - I don't think so. In any case, we should make base sort functions not interruptible, it's part of a larger task
| namespace array_functions_impl_ { | ||
|
|
||
| template<typename Result, typename U, typename Comparator> | ||
| Result sort(array<U> &arr, Comparator comparator, bool renumber) noexcept { |
There was a problem hiding this comment.
It's unsafe to pass an array to a coroutine by reference. In this function you access internal pointers of the array. Having such an array passed by reference, we can face with a case of the array being changed while we were blocked after co_await call.
There was a problem hiding this comment.
Sort functions works inplace
No description provided.