[libcxx-commits] [PATCH] D59791: [pstl] Add a serial backend for the PSTL

[Mikhail Dvorskiy via Phabricator via libcxx-commits]

MikeDvorskiy added inline comments.


================
Comment at: pstl/include/pstl/internal/parallel_backend_serial.h:94
+    // TODO: What to do with __nsort?
+    __leaf_sort(__first, __last, __comp);
+}
----------------
ldionne wrote:
> Is `__nsort` the number of elements that should be sorted in the resulting range? IOW, this is a partial sort where you want to get the `n` smallest elements of the whole range in sorted order.
> 
> It's not clear to me how to achieve this without a `__leaf_sort` that itself accepts a `n` to only partially sort the first `n` elements. I mean, I could probably find a way to do it iteratively by sorting a bit more every time, but what I really want is to just call `std::partial_sort`. And actually, while we're at it, calling `std::partial_sort` isn't enough, since this sort needs to be stable. But we don't have a `std::stable_partial_sort`.
Actually, __leaf_sort is not only std::sort or std::partial_sort.
__leaf_sort (as lambda) has already captured information about "sort" or "partial sort" - "__n" variable.
See, the code of the passed lambda into parallel_stable_sort:


```
[__n](_RandomAccessIterator __begin, _RandomAccessIterator __end, _Compare __comp) {
                if (__n < __end - __begin)
                    std::partial_sort(__begin, __begin + __n, __end, __comp);
                else
                    std::sort(__begin, __end, __comp);
            },
```

So, in case of serial case here you don't use __nsort parameter, due to you the special lambda which takes into account "sort" or "partial sort" logic.

In case of parallel implementation __n parameter may be useful - for  merging of  two sorted sub-ranges , just first __n elements....


================
Comment at: pstl/include/pstl/internal/parallel_backend_serial.h:106
+{
+    __leaf_merge(__first1, __last1, __first2, __last2, __out, __comp);
+}
----------------
ldionne wrote:
> Note that the obvious implementation doesn't work:
> 
> ```
> std::merge(__first1, __last1, __first2, __last2, __out, __comp);
> ```
> 
> because it requires the elements to be copyable, but pstl apparently expects the merge to only move elements around without copying them.
Why "doesnt work"?

In special case
...
    if (__n <= __merge_cut_off)
    {
        // Fall back on serial merge
        __leaf_merge(__xs, __xe, __ys, __ye, __zs, __comp);
    }
...
where __leaf_merge is std::merge


Repository:
  rPSTL pstl

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59791/new/

https://reviews.llvm.org/D59791