Use custom d-ary heap implementation

.github/workflows/osrm-backend.yml

@@ -731,6 +731,8 @@ jobs:
           sudo umount ~/benchmarks | true
           rm -rf ~/benchmarks 
           mkdir -p ~/benchmarks
+
+          for i in {1..15}; do sudo cset shield --reset && break || echo "cset shield reset attempt $i failed"; sleep 1; done
       # see https://llvm.org/docs/Benchmarking.html
       - name: Run PR Benchmarks 
         run: |
@@ -744,7 +746,9 @@ jobs:
 
           sudo cset shield --exec -- ./pr/scripts/ci/run_benchmarks.sh -f ~/benchmarks -r $(pwd)/pr_results -s $(pwd)/pr -b ~/benchmarks/build -o ~/data.osm.pbf -g ~/gps_traces.csv
           sudo umount ~/benchmarks
-          sudo cset shield --reset
+
+          for i in {1..15}; do sudo cset shield --reset && break || echo "cset shield reset attempt $i failed"; sleep 1; done
+
       - name: Run Base Benchmarks
         run: |
           sudo cset shield -c 2-3 -k on
@@ -760,9 +764,10 @@ jobs:
             cp base/src/benchmarks/portugal_to_korea.json ~/benchmarks/test/data/portugal_to_korea.json
           fi
           # we intentionally use scripts from PR branch to be able to update them and see results in the same PR
-          sudo cset shield --exec -- cset shield --exec -- ./pr/scripts/ci/run_benchmarks.sh -f ~/benchmarks -r $(pwd)/base_results -s $(pwd)/pr -b ~/benchmarks/build -o ~/data.osm.pbf -g ~/gps_traces.csv
+          sudo cset shield --exec -- ./pr/scripts/ci/run_benchmarks.sh -f ~/benchmarks -r $(pwd)/base_results -s $(pwd)/pr -b ~/benchmarks/build -o ~/data.osm.pbf -g ~/gps_traces.csv
           sudo umount ~/benchmarks
-          sudo cset shield --reset
+
+          for i in {1..15}; do sudo cset shield --reset && break || echo "cset shield reset attempt $i failed"; sleep 1; done
       - name: Post Benchmark Results
         run: |
           python3 pr/scripts/ci/post_benchmark_results.py base_results pr_results

include/util/d_ary_heap.hpp

@@ -0,0 +1,113 @@
+#pragma once
+
+#include <boost/assert.hpp>
+#include <functional>
+#include <limits>
+#include <utility>
+#include <vector>
+
+namespace osrm::util
+{
+template <typename HeapData, int Arity, typename Comparator = std::less<HeapData>> class DAryHeap
+{
+  public:
+    using HeapHandle = size_t;
+
+    static constexpr HeapHandle INVALID_HANDLE = std::numeric_limits<size_t>::max();
+
+  public:
+    const HeapData &top() const { return heap[0]; }
+
+    size_t size() const { return heap.size(); }
+
+    bool empty() const { return heap.empty(); }
+
+    const HeapData &operator[](HeapHandle handle) const { return heap[handle]; }
+
+    template <typename ReorderHandler>
+    void emplace(HeapData &&data, ReorderHandler &&reorderHandler)
+    {
+        heap.emplace_back(std::forward<HeapData>(data));
+        heapifyUp(heap.size() - 1, std::forward<ReorderHandler>(reorderHandler));
+    }
+
+    template <typename ReorderHandler>
+    void decrease(HeapHandle handle, HeapData &&data, ReorderHandler &&reorderHandler)
+    {
+        BOOST_ASSERT(handle < heap.size());
+
+        heap[handle] = std::forward<HeapData>(data);
+        heapifyUp(handle, std::forward<ReorderHandler>(reorderHandler));
+    }
+
+    void clear() { heap.clear(); }
+
+    template <typename ReorderHandler> void pop(ReorderHandler &&reorderHandler)
+    {
+        BOOST_ASSERT(!heap.empty());
+        heap[0] = std::move(heap.back());
+        heap.pop_back();
+        if (!heap.empty())
+        {
+            heapifyDown(0, std::forward<ReorderHandler>(reorderHandler));
+        }
+    }
+
+  private:
+    size_t parent(size_t index) { return (index - 1) / Arity; }
+
+    size_t kthChild(size_t index, size_t k) { return Arity * index + k + 1; }
+
+    template <typename ReorderHandler> void heapifyUp(size_t index, ReorderHandler &&reorderHandler)
+    {
+        HeapData temp = std::move(heap[index]);
+        while (index > 0 && comp(temp, heap[parent(index)]))
+        {
+            size_t parentIndex = parent(index);
+            heap[index] = std::move(heap[parentIndex]);
+            reorderHandler(heap[index], index);
+            index = parentIndex;
+        }
+        heap[index] = std::move(temp);
+        reorderHandler(heap[index], index);
+    }
+
+    template <typename ReorderHandler>
+    void heapifyDown(size_t index, ReorderHandler &&reorderHandler)
+    {
+        HeapData temp = std::move(heap[index]);
+        size_t child;
+        while (kthChild(index, 0) < heap.size())
+        {
+            child = minChild(index);
+            if (!comp(heap[child], temp))
+            {
+                break;
+            }
+            heap[index] = std::move(heap[child]);
+            reorderHandler(heap[index], index);
+            index = child;
+        }
+        heap[index] = std::move(temp);
+        reorderHandler(heap[index], index);
+    }
+
+    size_t minChild(size_t index)
+    {
+        size_t bestChild = kthChild(index, 0);
+        for (size_t k = 1; k < Arity; ++k)
+        {
+            size_t pos = kthChild(index, k);
+            if (pos < heap.size() && comp(heap[pos], heap[bestChild]))
+            {
+                bestChild = pos;
+            }
+        }
+        return bestChild;
+    }
+
+  private:
+    Comparator comp;
+    std::vector<HeapData> heap;
+};
+} // namespace osrm::util

include/util/query_heap.hpp

@@ -4,8 +4,10 @@
 #include <boost/assert.hpp>
 #include <boost/heap/d_ary_heap.hpp>
 
+#include "d_ary_heap.hpp"
 #include <algorithm>
 #include <cstdint>
+#include <cstdlib>
 #include <limits>
 #include <map>
 #include <optional>
@@ -133,20 +135,17 @@ class QueryHeap
         Weight weight;
         Key index;
 
-        bool operator>(const HeapData &other) const
+        bool operator<(const HeapData &other) const
         {
             if (weight == other.weight)
             {
-                return index > other.index;
+                return index < other.index;
             }
-            return weight > other.weight;
+            return weight < other.weight;
         }
     };
-    using HeapContainer = boost::heap::d_ary_heap<HeapData,
-                                                  boost::heap::arity<4>,
-                                                  boost::heap::mutable_<true>,
-                                                  boost::heap::compare<std::greater<HeapData>>>;
-    using HeapHandle = typename HeapContainer::handle_type;
+    using HeapContainer = DAryHeap<HeapData, 4>;
+    using HeapHandle = typename HeapContainer::HeapHandle;
 
   public:
     using WeightType = Weight;
@@ -178,11 +177,31 @@ class QueryHeap
 
     void Insert(NodeID node, Weight weight, const Data &data)
     {
+        checkInvariants();
+
         BOOST_ASSERT(node < std::numeric_limits<NodeID>::max());
         const auto index = static_cast<Key>(inserted_nodes.size());
-        const auto handle = heap.emplace(HeapData{weight, index});
-        inserted_nodes.emplace_back(HeapNode{handle, node, weight, data});
+        inserted_nodes.emplace_back(HeapNode{heap.size(), node, weight, data});
+
+        heap.emplace(HeapData{weight, index},
+                     [this](const auto &heapData, auto new_handle)
+                     { inserted_nodes[heapData.index].handle = new_handle; });
         node_index[node] = index;
+
+        checkInvariants();
+    }
+
+    void checkInvariants()
+    {
+#ifndef NDEBUG
+        for (size_t handle = 0; handle < heap.size(); ++handle)
+        {
+            auto &in_heap = heap[handle];
+            auto &inserted = inserted_nodes[in_heap.index];
+            BOOST_ASSERT(in_heap.weight == inserted.weight);
+            BOOST_ASSERT(inserted.handle == handle);
+        }
+#endif // !NDEBUG
     }
 
     Data &GetData(NodeID node)
@@ -216,16 +235,7 @@ class QueryHeap
     {
         BOOST_ASSERT(WasInserted(node));
         const Key index = node_index.peek_index(node);
-
-        // Use end iterator as a reliable "non-existent" handle.
-        // Default-constructed handles are singular and
-        // can only be checked-compared to another singular instance.
-        // Behaviour investigated at https://lists.boost.org/boost-users/2017/08/87787.php,
-        // eventually confirmation at https://stackoverflow.com/a/45622940/151641.
-        // Corrected in https://github.com/Project-OSRM/osrm-backend/pull/4396
-        auto const end_it = const_cast<HeapContainer &>(heap).end();  // non-const iterator
-        auto const none_handle = heap.s_handle_from_iterator(end_it); // from non-const iterator
-        return inserted_nodes[index].handle == none_handle;
+        return inserted_nodes[index].handle == HeapContainer::INVALID_HANDLE;
     }
 
     bool WasInserted(const NodeID node) const
@@ -276,26 +286,30 @@ class QueryHeap
     {
         BOOST_ASSERT(!heap.empty());
         const Key removedIndex = heap.top().index;
-        heap.pop();
-        inserted_nodes[removedIndex].handle = heap.s_handle_from_iterator(heap.end());
+        inserted_nodes[removedIndex].handle = HeapContainer::INVALID_HANDLE;
+
+        heap.pop([this](const auto &heapData, auto new_handle)
+                 { inserted_nodes[heapData.index].handle = new_handle; });
         return inserted_nodes[removedIndex].node;
     }
 
     HeapNode &DeleteMinGetHeapNode()
     {
         BOOST_ASSERT(!heap.empty());
+        checkInvariants();
         const Key removedIndex = heap.top().index;
-        heap.pop();
-        inserted_nodes[removedIndex].handle = heap.s_handle_from_iterator(heap.end());
+        inserted_nodes[removedIndex].handle = HeapContainer::INVALID_HANDLE;
+        heap.pop([this](const auto &heapData, auto new_handle)
+                 { inserted_nodes[heapData.index].handle = new_handle; });
+        checkInvariants();
         return inserted_nodes[removedIndex];
     }
 
     void DeleteAll()
     {
-        auto const none_handle = heap.s_handle_from_iterator(heap.end());
         std::for_each(inserted_nodes.begin(),
                       inserted_nodes.end(),
-                      [&none_handle](auto &node) { node.handle = none_handle; });
+                      [&](auto &node) { node.handle = HeapContainer::INVALID_HANDLE; });
         heap.clear();
     }
 
@@ -305,13 +319,19 @@ class QueryHeap
         const auto index = node_index.peek_index(node);
         auto &reference = inserted_nodes[index];
         reference.weight = weight;
-        heap.increase(reference.handle, HeapData{weight, static_cast<Key>(index)});
+        heap.decrease(reference.handle,
+                      HeapData{weight, static_cast<Key>(index)},
+                      [this](const auto &heapData, auto new_handle)
+                      { inserted_nodes[heapData.index].handle = new_handle; });
     }
 
     void DecreaseKey(const HeapNode &heapNode)
     {
         BOOST_ASSERT(!WasRemoved(heapNode.node));
-        heap.increase(heapNode.handle, HeapData{heapNode.weight, (*heapNode.handle).index});
+        heap.decrease(heapNode.handle,
+                      HeapData{heapNode.weight, heap[heapNode.handle].index},
+                      [this](const auto &heapData, auto new_handle)
+                      { inserted_nodes[heapData.index].handle = new_handle; });
     }
 
   private:
@@ -319,6 +339,7 @@ class QueryHeap
     HeapContainer heap;
     IndexStorage node_index;
 };
+
 } // namespace osrm::util
 
 #endif // OSRM_UTIL_QUERY_HEAP_HPP

unit_tests/util/d_ary_heap.cpp

@@ -0,0 +1,175 @@
+
+#include "util/d_ary_heap.hpp"
+#include <boost/test/unit_test.hpp>
+
+using namespace osrm::util;
+
+BOOST_AUTO_TEST_SUITE(d_ary_heap_test)
+
+BOOST_AUTO_TEST_CASE(test_empty_heap)
+{
+    DAryHeap<int, 2> heap;
+    BOOST_CHECK(heap.empty());
+    BOOST_CHECK_EQUAL(heap.size(), 0);
+    heap.emplace(10, [](int &, size_t) {});
+    BOOST_CHECK(!heap.empty());
+    BOOST_CHECK_EQUAL(heap.size(), 1);
+}
+
+BOOST_AUTO_TEST_CASE(test_emplace_and_top)
+{
+    DAryHeap<int, 2> heap;
+    heap.emplace(10, [](int &, size_t) {});
+    heap.emplace(5, [](int &, size_t) {});
+    heap.emplace(8, [](int &, size_t) {});
+
+    BOOST_CHECK_EQUAL(heap.top(), 5);
+    BOOST_CHECK_EQUAL(heap.size(), 3);
+}
+
+BOOST_AUTO_TEST_CASE(test_pop)
+{
+    DAryHeap<int, 2> heap;
+    heap.emplace(10, [](int &, size_t) {});
+    heap.emplace(5, [](int &, size_t) {});
+    heap.emplace(8, [](int &, size_t) {});
+
+    heap.pop([](int &, size_t) {});
+    BOOST_CHECK_EQUAL(heap.top(), 8);
+    BOOST_CHECK_EQUAL(heap.size(), 2);
+
+    heap.pop([](int &, size_t) {});
+    BOOST_CHECK_EQUAL(heap.top(), 10);
+    BOOST_CHECK_EQUAL(heap.size(), 1);
+}
+
+BOOST_AUTO_TEST_CASE(test_decrease)
+{
+    struct HeapData
+    {
+        int key;
+        int data;
+
+        bool operator<(const HeapData &other) const { return key < other.key; }
+    };
+    DAryHeap<HeapData, 2> heap;
+    size_t handle = DAryHeap<HeapData, 2>::INVALID_HANDLE;
+
+    auto reorder_handler = [&](const HeapData &value, size_t new_handle)
+    {
+        if (value.data == 42)
+        {
+            handle = new_handle;
+        }
+    };
+
+    heap.emplace({10, 42}, reorder_handler);
+    heap.emplace({5, 73}, reorder_handler);
+    heap.emplace({8, 37}, reorder_handler);
+
+    heap.decrease(handle, {3, 42}, reorder_handler);
+    BOOST_CHECK_EQUAL(heap.size(), 3);
+    BOOST_CHECK_EQUAL(heap.top().key, 3);
+    BOOST_CHECK_EQUAL(heap.top().data, 42);
+    heap.pop(reorder_handler);
+    BOOST_CHECK_EQUAL(heap.size(), 2);
+    BOOST_CHECK_EQUAL(heap.top().key, 5);
+    BOOST_CHECK_EQUAL(heap.top().data, 73);
+    heap.pop(reorder_handler);
+    BOOST_CHECK_EQUAL(heap.size(), 1);
+    BOOST_CHECK_EQUAL(heap.top().key, 8);
+    BOOST_CHECK_EQUAL(heap.top().data, 37);
+    heap.pop(reorder_handler);
+    BOOST_CHECK_EQUAL(heap.size(), 0);
+    BOOST_CHECK(heap.empty());
+}
+
+BOOST_AUTO_TEST_CASE(test_reorder_handler)
+{
+    std::vector<int> reordered_values;
+    std::vector<size_t> reordered_indices;
+    auto reorder_handler = [&](int value, size_t index)
+    {
+        reordered_values.push_back(value);
+        reordered_indices.push_back(index);
+    };
+    DAryHeap<int, 2> heap;
+    std::vector<int> expected_reordered_values;
+    std::vector<int> expected_reordered_indices;
+
+    heap.emplace(10, reorder_handler);
+
+    expected_reordered_values = {10};
+    expected_reordered_indices = {0};
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_values.begin(),
+                                  reordered_values.end(),
+                                  expected_reordered_values.begin(),
+                                  expected_reordered_values.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_indices.begin(),
+                                  reordered_indices.end(),
+                                  expected_reordered_indices.begin(),
+                                  expected_reordered_indices.end());
+
+    heap.emplace(5, reorder_handler);
+    expected_reordered_values = {10, 10, 5};
+    expected_reordered_indices = {0, 1, 0};
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_values.begin(),
+                                  reordered_values.end(),
+                                  expected_reordered_values.begin(),
+                                  expected_reordered_values.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_indices.begin(),
+                                  reordered_indices.end(),
+                                  expected_reordered_indices.begin(),
+                                  expected_reordered_indices.end());
+
+    heap.emplace(8, reorder_handler);
+    expected_reordered_values = {10, 10, 5, 8};
+    expected_reordered_indices = {0, 1, 0, 2};
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_values.begin(),
+                                  reordered_values.end(),
+                                  expected_reordered_values.begin(),
+                                  expected_reordered_values.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_indices.begin(),
+                                  reordered_indices.end(),
+                                  expected_reordered_indices.begin(),
+                                  expected_reordered_indices.end());
+
+    heap.pop(reorder_handler);
+    expected_reordered_values = {10, 10, 5, 8, 8};
+    expected_reordered_indices = {0, 1, 0, 2, 0};
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_values.begin(),
+                                  reordered_values.end(),
+                                  expected_reordered_values.begin(),
+                                  expected_reordered_values.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_indices.begin(),
+                                  reordered_indices.end(),
+                                  expected_reordered_indices.begin(),
+                                  expected_reordered_indices.end());
+
+    heap.pop(reorder_handler);
+
+    expected_reordered_values = {10, 10, 5, 8, 8, 10};
+    expected_reordered_indices = {0, 1, 0, 2, 0, 0};
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_values.begin(),
+                                  reordered_values.end(),
+                                  expected_reordered_values.begin(),
+                                  expected_reordered_values.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_indices.begin(),
+                                  reordered_indices.end(),
+                                  expected_reordered_indices.begin(),
+                                  expected_reordered_indices.end());
+
+    heap.pop(reorder_handler);
+    expected_reordered_values = {10, 10, 5, 8, 8, 10};
+    expected_reordered_indices = {0, 1, 0, 2, 0, 0};
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_values.begin(),
+                                  reordered_values.end(),
+                                  expected_reordered_values.begin(),
+                                  expected_reordered_values.end());
+    BOOST_CHECK_EQUAL_COLLECTIONS(reordered_indices.begin(),
+                                  reordered_indices.end(),
+                                  expected_reordered_indices.begin(),
+                                  expected_reordered_indices.end());
+}
+
+BOOST_AUTO_TEST_SUITE_END()

unit_tests/util/query_heap.cpp

@@ -120,6 +120,28 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE(delete_all_test, T, storage_types, RandomDataFi
     BOOST_CHECK(heap.Empty());
 }
 
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(smoke_test, T, storage_types, RandomDataFixture<NUM_NODES>)
+{
+    QueryHeap<TestNodeID, TestKey, TestWeight, TestData, T> heap(NUM_NODES);
+
+    for (unsigned idx : order)
+    {
+        heap.Insert(ids[idx], weights[idx], data[idx]);
+    }
+
+    while (!heap.Empty())
+    {
+        auto old_weight = heap.MinKey();
+        auto node = heap.GetHeapNodeIfWasInserted(heap.Min());
+        BOOST_CHECK(old_weight == node->weight);
+        BOOST_CHECK(node);
+        node->weight = node->weight - 1;
+        heap.DecreaseKey(*node);
+        BOOST_CHECK(heap.MinKey() == node->weight);
+        heap.DeleteMin();
+    }
+}
+
 BOOST_FIXTURE_TEST_CASE_TEMPLATE(decrease_key_test, T, storage_types, RandomDataFixture<10>)
 {
     QueryHeap<TestNodeID, TestKey, TestWeight, TestData, T> heap(10);