Avoid unconditional make_heap for priority_queue::push_range (#4025)

Co-authored-by: Stephan T. Lavavej <[email protected]>

benchmarks/CMakeLists.txt

@@ -108,6 +108,7 @@ endfunction()
 add_benchmark(bitset_to_string src/bitset_to_string.cpp)
 add_benchmark(locale_classic src/locale_classic.cpp)
 add_benchmark(path_lexically_normal src/path_lexically_normal.cpp)
+add_benchmark(priority_queue_push_range src/priority_queue_push_range.cpp)
 add_benchmark(random_integer_generation src/random_integer_generation.cpp)
 add_benchmark(std_copy src/std_copy.cpp)
 

benchmarks/src/priority_queue_push_range.cpp

@@ -0,0 +1,89 @@
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include <algorithm>
+#include <benchmark/benchmark.h>
+#include <cstddef>
+#include <cstdint>
+#include <cstdio>
+#include <queue>
+#include <random>
+#include <span>
+#include <string>
+#include <string_view>
+#include <vector>
+using namespace std;
+
+namespace {
+    constexpr size_t vec_size = 10'000;
+
+    template <class T, class Fn>
+    auto create_vec(Fn transformation) {
+        vector<T> vec(vec_size);
+        for (mt19937_64 rnd(1); auto& e : vec) {
+            e = transformation(rnd());
+        }
+        return vec;
+    }
+
+    template <class T>
+    T cast_to(uint64_t val) {
+        return static_cast<T>(val);
+    }
+
+    const auto vec_u8     = create_vec<uint8_t>(cast_to<uint8_t>);
+    const auto vec_u16    = create_vec<uint16_t>(cast_to<uint16_t>);
+    const auto vec_u32    = create_vec<uint32_t>(cast_to<uint32_t>);
+    const auto vec_u64    = create_vec<uint64_t>(cast_to<uint64_t>);
+    const auto vec_float  = create_vec<float>(cast_to<float>);
+    const auto vec_double = create_vec<double>(cast_to<double>);
+
+    const auto vec_str  = create_vec<string>([](uint64_t val) { return to_string(static_cast<uint32_t>(val)); });
+    const auto vec_wstr = create_vec<wstring>([](uint64_t val) { return to_wstring(static_cast<uint32_t>(val)); });
+
+    template <class T, const auto& Data>
+    void BM_push_range(benchmark::State& state) {
+        const size_t frag_size = static_cast<size_t>(state.range(0));
+
+        for (auto _ : state) {
+            priority_queue<T> que;
+            span spn{Data};
+
+            while (!spn.empty()) {
+                const size_t take_size = min(spn.size(), frag_size);
+                que.push_range(spn.first(take_size));
+                spn = spn.subspan(take_size);
+            }
+            benchmark::DoNotOptimize(que);
+        }
+    }
+
+    template <size_t L>
+    void putln(const benchmark::State&) {
+        static bool b = [] {
+            puts("");
+            return true;
+        }();
+    }
+} // namespace
+
+#define TEST_PUSH_RANGE(T, source)      \
+    BENCHMARK(BM_push_range<T, source>) \
+        ->Setup(putln<__LINE__>)        \
+        ->RangeMultiplier(100)          \
+        ->Range(1, vec_size)            \
+        ->Arg(vec_size / 2 + 1);
+
+TEST_PUSH_RANGE(uint8_t, vec_u8);
+TEST_PUSH_RANGE(uint16_t, vec_u16);
+TEST_PUSH_RANGE(uint32_t, vec_u32);
+TEST_PUSH_RANGE(uint64_t, vec_u64);
+TEST_PUSH_RANGE(float, vec_float);
+TEST_PUSH_RANGE(double, vec_double);
+
+TEST_PUSH_RANGE(string_view, vec_str);
+TEST_PUSH_RANGE(string, vec_str);
+TEST_PUSH_RANGE(wstring_view, vec_wstr);
+TEST_PUSH_RANGE(wstring, vec_wstr);
+
+BENCHMARK_MAIN();

stl/inc/queue

@@ -246,40 +246,40 @@ public:
         : c(), comp(_Pred) {}
 
     priority_queue(const _Pr& _Pred, const _Container& _Cont) : c(_Cont), comp(_Pred) {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     priority_queue(const _Pr& _Pred, _Container&& _Cont) : c(_STD move(_Cont)), comp(_Pred) {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _InIt, enable_if_t<_Is_iterator_v<_InIt>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, const _Container& _Cont) : c(_Cont), comp(_Pred) {
         c.insert(c.end(), _First, _Last);
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _InIt, enable_if_t<_Is_iterator_v<_InIt>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last) : c(_First, _Last), comp() {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _InIt, enable_if_t<_Is_iterator_v<_InIt>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred) : c(_First, _Last), comp(_Pred) {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _InIt, enable_if_t<_Is_iterator_v<_InIt>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, _Container&& _Cont) : c(_STD move(_Cont)), comp(_Pred) {
         c.insert(c.end(), _First, _Last);
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
 #if _HAS_CXX23 && defined(__cpp_lib_concepts) // TRANSITION, GH-395
     template <_Container_compatible_range<_Ty> _Rng>
     priority_queue(from_range_t, _Rng&& _Range, const _Pr& _Pred = _Pr())
         : c(_RANGES to<_Container>(_STD forward<_Rng>(_Range))), comp(_Pred) {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 #endif // _HAS_CXX23 && defined(__cpp_lib_concepts)
 
@@ -295,12 +295,12 @@ public:
 
     template <class _Alloc, enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(const _Pr& _Pred, const _Container& _Cont, const _Alloc& _Al) : c(_Cont, _Al), comp(_Pred) {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _Alloc, enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(const _Pr& _Pred, _Container&& _Cont, const _Alloc& _Al) : c(_STD move(_Cont), _Al), comp(_Pred) {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _Alloc, enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
@@ -315,45 +315,45 @@ public:
     template <class _InIt, class _Alloc,
         enable_if_t<_Is_iterator_v<_InIt> && uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Alloc& _Al) : c(_First, _Last, _Al), comp() {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _InIt, class _Alloc,
         enable_if_t<_Is_iterator_v<_InIt> && uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, const _Alloc& _Al)
         : c(_First, _Last, _Al), comp(_Pred) {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _InIt, class _Alloc,
         enable_if_t<_Is_iterator_v<_InIt> && uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, const _Container& _Cont, const _Alloc& _Al)
         : c(_Cont, _Al), comp(_Pred) {
         c.insert(c.end(), _First, _Last);
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <class _InIt, class _Alloc,
         enable_if_t<_Is_iterator_v<_InIt> && uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, _Container&& _Cont, const _Alloc& _Al)
         : c(_STD move(_Cont), _Al), comp(_Pred) {
         c.insert(c.end(), _First, _Last);
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
 #if _HAS_CXX23 && defined(__cpp_lib_concepts) // TRANSITION, GH-395
     template <_Container_compatible_range<_Ty> _Rng, class _Alloc,
         enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(from_range_t, _Rng&& _Range, const _Pr& _Pred, const _Alloc& _Al)
         : c(_RANGES to<_Container>(_STD forward<_Rng>(_Range), _Al)), comp(_Pred) {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 
     template <_Container_compatible_range<_Ty> _Rng, class _Alloc,
         enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(from_range_t, _Rng&& _Range, const _Alloc& _Al)
         : c(_RANGES to<_Container>(_STD forward<_Rng>(_Range), _Al)), comp() {
-        _STD make_heap(c.begin(), c.end(), comp);
+        _Make_heap();
     }
 #endif // _HAS_CXX23 && defined(__cpp_lib_concepts)
 
@@ -371,35 +371,47 @@ public:
 
     void push(const value_type& _Val) {
         c.push_back(_Val);
-        _STD push_heap(c.begin(), c.end(), comp);
+        _STD push_heap(c.begin(), c.end(), _STD _Pass_fn(comp));
     }
 
     void push(value_type&& _Val) {
         c.push_back(_STD move(_Val));
-        _STD push_heap(c.begin(), c.end(), comp);
+        _STD push_heap(c.begin(), c.end(), _STD _Pass_fn(comp));
     }
 
 #if _HAS_CXX23 && defined(__cpp_lib_concepts) // TRANSITION, GH-395
     template <_Container_compatible_range<_Ty> _Rng>
     void push_range(_Rng&& _Range) {
+        const size_type _Old_size = c.size();
+
         if constexpr (requires { c.append_range(_Range); }) {
             c.append_range(_Range);
         } else {
             _RANGES copy(_Range, back_insert_iterator{c});
         }
 
-        _STD make_heap(c.begin(), c.end(), comp);
+        const size_type _New_size = c.size();
+        if (_New_size / 2 > _Old_size) { // threshold chosen for performance
+            _Make_heap();
+        } else {
+            const auto _Begin = _STD _Get_unwrapped(c.begin());
+            auto _Heap_end    = _Begin + _Old_size;
+            const auto _End   = _STD _Get_unwrapped(c.end());
+            while (_Heap_end != _End) {
+                _STD push_heap(_Begin, ++_Heap_end, _STD _Pass_fn(comp));
+            }
+        }
     }
 #endif // _HAS_CXX23 && defined(__cpp_lib_concepts)
 
     template <class... _Valty>
     void emplace(_Valty&&... _Val) {
         c.emplace_back(_STD forward<_Valty>(_Val)...);
-        _STD push_heap(c.begin(), c.end(), comp);
+        _STD push_heap(c.begin(), c.end(), _STD _Pass_fn(comp));
     }
 
     void pop() {
-        _STD pop_heap(c.begin(), c.end(), comp);
+        _STD pop_heap(c.begin(), c.end(), _STD _Pass_fn(comp));
         c.pop_back();
     }
 
@@ -410,6 +422,11 @@ public:
         swap(comp, _Right.comp); // intentional ADL
     }
 
+private:
+    void _Make_heap() {
+        _STD make_heap(c.begin(), c.end(), _STD _Pass_fn(comp));
+    }
+
 protected:
     _Container c{};
     _Pr comp{};

stl/inc/xutility

@@ -543,7 +543,8 @@ struct _Ref_fn { // pass function object by value as a reference
     // _Ref_fn is an aggregate so it can be enregistered, unlike reference_wrapper
 
     template <class... _Args>
-    constexpr decltype(auto) operator()(_Args&&... _Vals) { // forward function call operator
+    constexpr decltype(auto) operator()(_Args&&... _Vals) noexcept(
+        _Select_invoke_traits<_Fx&, _Args...>::_Is_nothrow_invocable::value) { // forward function call operator
         if constexpr (is_member_pointer_v<_Fx>) {
             return _STD invoke(_Fn, _STD forward<_Args>(_Vals)...);
         } else {