first pass to review the implementation, still to integrate

src/entt/core/algorithm.hpp

@@ -2,9 +2,10 @@
 #define ENTT_CORE_ALGORITHM_HPP
 
 
-#include <functional>
-#include <algorithm>
 #include <utility>
+#include <iterator>
+#include <algorithm>
+#include <functional>
 #include "utility.hpp"
 
 
@@ -76,35 +77,10 @@ struct insertion_sort {
  * @tparam Bit Number of bits processed per pass.
  * @tparam N Maximum number of bits to sort.
  */
-template<std::size_t Pass, std::size_t N>
-class radix_sort {
-    static_assert((N % Pass) == 0);
-
-    template<typename In, typename Out, typename Getter>
-    void sort(In first, In last, Out out_begin, Getter getter = Getter{}, uint32_t pass = 0) const {
-        uint32_t start_bit = pass * Pass;
-
-        constexpr int n_buckets = 1 << Pass;
-        int bucket_count[n_buckets] = {0};
-        constexpr int bit_mask = (1 << Pass) - 1;
+template<std::size_t Bit, std::size_t N>
+struct radix_sort {
+    static_assert((N % Bit) == 0);
 
-        for(auto it = first; it < last; ++it) {
-            int bucket = (getter(*it) >> start_bit) & bit_mask;
-            ++bucket_count[bucket];
-        }
-
-        int out_index[n_buckets];
-        out_index[0] = 0;
-        for (int i = 1; i < n_buckets; ++i)
-            out_index[i] = out_index[i - 1] + bucket_count[i - 1];
-
-        for(auto it = first; it < last; ++it) {
-            int bucket = (getter(*it) >> start_bit) & bit_mask;
-            out_begin[out_index[bucket]++] = *it;
-        }
-    }
-
-public:
     /**
      * @brief Sorts the elements in a range.
      *
@@ -123,27 +99,43 @@ class radix_sort {
     template<typename It, typename Getter = identity>
     void operator()(It first, It last, Getter getter = Getter{}) const {
         if(first < last) {
+            static constexpr auto mask = (1 << Bit) - 1;
+            static constexpr auto buckets = 1 << Bit;
+            static constexpr auto passes = N / Bit;
+
             using size_type = typename std::iterator_traits<It>::value_type;
             std::vector<size_type> aux(std::distance(first, last));
-            constexpr uint32_t n_passes = N / Pass;
 
-            for (size_t pass = 0; pass < n_passes; ++pass) {
-                if (!(pass & 1)) {
-                    sort(first, last, aux.begin(), getter, pass);
-                } else {
-                    sort(aux.begin(), aux.end(), first, getter, pass);
-                }
-            }
+            auto part = [getter = std::move(getter)](auto from, auto to, auto out, auto start) {
+                int index[buckets]{};
+                int count[buckets]{};
 
-            // Move final result from _aux_ vector, if needed
-            if constexpr (n_passes & 1) {
-                auto it = first;
+                std::for_each(from, to, [&getter, &count, start](const auto &item) {
+                    ++count[(getter(item) >> start) & mask];
+                });
 
-                for(auto &v : aux) {
-                    *(it++) = std::move(v);
+                std::for_each(std::next(std::begin(index)), std::end(index), [index = std::begin(index), count = std::begin(count)](auto &item) mutable {
+                    item = *(index++) + *(count++);
+                });
+
+                std::for_each(from, to, [&getter, &out, &index, start](const auto &item) {
+                    out[index[(getter(item) >> start) & mask]++] = std::move(item);
+                });
+            };
+
+            for(std::size_t pass = 0; pass < passes; ++pass) {
+                const auto start = pass * Bit;
+
+                if(pass & 1) {
+                    part(aux.begin(), aux.end(), first, start);
+                } else {
+                    part(first, last, aux.begin(), start);
                 }
             }
 
+            if constexpr(passes & 1) {
+                std::move(aux.begin(), aux.end(), first);
+            }
         }
     }
 };

test/entt/core/algorithm.cpp

@@ -2,6 +2,10 @@
 #include <gtest/gtest.h>
 #include <entt/core/algorithm.hpp>
 
+struct boxed_int {
+    int value;
+};
+
 TEST(Algorithm, StdSort) {
     // well, I'm pretty sure it works, it's std::sort!!
     std::array<int, 5> arr{{4, 1, 3, 2, 0}};
@@ -14,6 +18,20 @@ TEST(Algorithm, StdSort) {
     }
 }
 
+TEST(Algorithm, StdSortBoxedInt) {
+    // well, I'm pretty sure it works, it's std::sort!!
+    std::array<boxed_int, 5> arr{{{4}, {1}, {3}, {2}, {0}}};
+    entt::std_sort sort;
+
+    sort(arr.begin(), arr.end(), [](const auto &lhs, const auto &rhs) {
+        return lhs.value > rhs.value;
+    });
+
+    for(typename decltype(arr)::size_type i = 0; i < (arr.size() - 1); ++i) {
+        ASSERT_GT(arr[i].value, arr[i+1].value);
+    }
+}
+
 TEST(Algorithm, InsertionSort) {
     std::array<int, 5> arr{{4, 1, 3, 2, 0}};
     entt::insertion_sort sort;
@@ -25,23 +43,55 @@ TEST(Algorithm, InsertionSort) {
     }
 }
 
+TEST(Algorithm, InsertionSortBoxedInt) {
+    std::array<boxed_int, 5> arr{{{4}, {1}, {3}, {2}, {0}}};
+    entt::insertion_sort sort;
+
+    sort(arr.begin(), arr.end(), [](const auto &lhs, const auto &rhs) {
+        return lhs.value > rhs.value;
+    });
+
+    for(typename decltype(arr)::size_type i = 0; i < (arr.size() - 1); ++i) {
+        ASSERT_GT(arr[i].value, arr[i+1].value);
+    }
+}
+
 TEST(Algorithm, InsertionSortEmptyContainer) {
     std::vector<int> vec{};
     entt::insertion_sort sort;
     // this should crash with asan enabled if we break the constraint
     sort(vec.begin(), vec.end());
 }
 
-TEST(Algorithm, RadixSort)
-{
+TEST(Algorithm, RadixSort) {
     std::array<uint32_t, 5> arr{{4, 1, 3, 2, 0}};
     entt::radix_sort<8, 32> sort;
 
-    sort(arr.begin(), arr.end(), [](const auto &v) {
-        return v;
+    sort(arr.begin(), arr.end(), [](const auto &value) {
+        return value;
     });
 
     for(typename decltype(arr)::size_type i = 0; i < (arr.size() - 1); ++i) {
         ASSERT_LT(arr[i], arr[i+1]);
     }
 }
+
+TEST(Algorithm, RadixSortBoxedInt) {
+    std::array<boxed_int, 5> arr{{{4}, {1}, {3}, {2}, {0}}};
+    entt::radix_sort<8, 32> sort;
+
+    sort(arr.rbegin(), arr.rend(), [](const auto &instance) {
+        return instance.value;
+    });
+
+    for(typename decltype(arr)::size_type i = 0; i < (arr.size() - 1); ++i) {
+        ASSERT_GT(arr[i].value, arr[i+1].value);
+    }
+}
+
+TEST(Algorithm, RadixSortEmptyContainer) {
+    std::vector<int> vec{};
+    entt::radix_sort<8, 32> sort;
+    // this should crash with asan enabled if we break the constraint
+    sort(vec.begin(), vec.end());
+}