From 6571e0d6402c737845409fbf343d578ca9ecf997 Mon Sep 17 00:00:00 2001 From: Jeremiah Morgan Date: Thu, 7 Nov 2024 23:11:12 +0000 Subject: [PATCH 1/2] data structure: implement pairing heap without use of shared_ptr --- libopenage/datastructure/pairing_heap.h | 167 ++++++++++++------------ libopenage/datastructure/tests.cpp | 5 +- libopenage/event/eventstore.cpp | 4 +- 3 files changed, 91 insertions(+), 85 deletions(-) diff --git a/libopenage/datastructure/pairing_heap.h b/libopenage/datastructure/pairing_heap.h index dd5438091a..8a571d1ea2 100644 --- a/libopenage/datastructure/pairing_heap.h +++ b/libopenage/datastructure/pairing_heap.h @@ -38,7 +38,7 @@ class PairingHeap; template > -class PairingHeapNode : public std::enable_shared_from_this> { +class PairingHeapNode { public: using this_type = PairingHeapNode; @@ -47,7 +47,6 @@ class PairingHeapNode : public std::enable_shared_from_this &node) { - node->add_child(this->shared_from_this()); + void become_child_of(this_type *const node) { + node->add_child(this); } /** * Add the given node as a child to this one. */ - void add_child(const std::shared_ptr &new_child) { + void add_child(this_type *const new_child) { // first child is the most recently attached one // it must not have siblings as they will get lost. @@ -85,7 +88,7 @@ class PairingHeapNode : public std::enable_shared_from_thisfirst_child = new_child; - new_child->parent = this->shared_from_this(); + new_child->parent = this; } /** @@ -93,23 +96,23 @@ class PairingHeapNode : public std::enable_shared_from_this link_with(const std::shared_ptr &node) { - std::shared_ptr new_root; - std::shared_ptr new_child; + this_type *link_with(this_type *const node) { + this_type *new_root; + this_type *new_child; if (this->cmp(this->data, node->data)) { - new_root = this->shared_from_this(); + new_root = this; new_child = node; } else { new_root = node; - new_child = this->shared_from_this(); + new_child = this; } // children of new root become siblings of new new_child // -> parent of new child = new root - // this whll be set by the add_child method + // this will be set by the add_child method new_child->prev_sibling = nullptr; new_child->next_sibling = nullptr; @@ -128,15 +131,15 @@ class PairingHeapNode : public std::enable_shared_from_this link_backwards() { + this_type *link_backwards() { if (this->next_sibling == nullptr) { // reached end, return this as current root, // the previous siblings will be linked to it. - return this->shared_from_this(); + return this; } // recurse to last sibling, - std::shared_ptr node = this->next_sibling->link_backwards(); + this_type *node = this->next_sibling->link_backwards(); // then link ourself to the new root. this->next_sibling = nullptr; @@ -153,9 +156,9 @@ class PairingHeapNode : public std::enable_shared_from_thisparent and this->parent->first_child == this->shared_from_this()) { - // we are the first child - // make the next sibling the first child + if (this->parent and this->parent->first_child == this) { + // we are child + // make the next sibling child this->parent->first_child = this->next_sibling; } // if we have a previous sibling @@ -176,10 +179,10 @@ class PairingHeapNode : public std::enable_shared_from_this first_child; - std::shared_ptr prev_sibling; - std::shared_ptr next_sibling; - std::shared_ptr parent; // for decrease-key and delete + this_type *first_child = nullptr; + this_type *prev_sibling = nullptr; + this_type *next_sibling = nullptr; + this_type *parent = nullptr; // for decrease-key and delete }; @@ -191,10 +194,8 @@ template > class PairingHeap final { public: - using node_t = heapnode_t; - using element_t = std::shared_ptr; - using this_type = PairingHeap; - using cmp_t = compare; + using element_t = heapnode_t *; + using this_type = PairingHeap; /** * create a empty heap. @@ -204,14 +205,16 @@ class PairingHeap final { root_node(nullptr) { } - ~PairingHeap() = default; + ~PairingHeap() { + this->clear(); + }; /** * adds the given item to the heap. * O(1) */ element_t push(const T &item) { - element_t new_node = std::make_shared(item); + element_t new_node = new heapnode_t(item); this->push_node(new_node); return new_node; } @@ -221,25 +224,18 @@ class PairingHeap final { * O(1) */ element_t push(T &&item) { - element_t new_node = std::make_shared(std::move(item)); + element_t new_node = new heapnode_t(std::move(item)); this->push_node(new_node); return new_node; } - /** - * returns and removes the smallest item on the heap. - */ - T pop() { - return std::move(this->pop_node()->data); - } - /** * returns the smallest item on the heap and deletes it. * also known as delete_min. * _________ * Ω(log log n), O(2^(2*√log log n')) */ - element_t pop_node() { + T pop() { if (this->root_node == nullptr) { throw Error{MSG(err) << "Can't pop an empty heap!"}; } @@ -316,36 +312,9 @@ class PairingHeap final { ret->first_child = nullptr; // and it's done! - return ret; - } - - /** - * Unlink a node from the heap. - * - * If the item is the current root, just pop(). - * else, cut the node from its parent, pop() that subtree - * and merge these trees. - * - * O(pop_node) - */ - void unlink_node(const element_t &node) { - if (node == this->root_node) { - this->pop_node(); - } - else { - node->loosen(); - - element_t real_root = this->root_node; - this->root_node = node; - this->pop_node(); - - element_t new_root = this->root_node; - this->root_node = real_root; - - if (new_root != nullptr) { - this->root_insert(new_root); - } - } + T data = std::move(ret->data); + delete ret; + return data; } /** @@ -391,14 +360,43 @@ class PairingHeap final { * * O(1) (but slower than decrease), and O(pop) when node is the root. */ - void update(const element_t &node) { + void update(element_t &node) { if (node != this->root_node) [[likely]] { - this->unlink_node(node); - this->push_node(node); + node = this->push(this->remove_node(node)); } else { // it's the root node, so we just pop and push it. - this->push_node(this->pop_node()); + node = this->push(this->pop()); + } + } + + /** + * remove a node from the heap. Return its data. + * + * If the item is the current root, just pop(). + * else, cut the node from its parent, pop() that subtree + * and merge these trees. + * + * O(pop_node) + */ + T remove_node(const element_t &node) { + if (node == this->root_node) { + return this->pop(); + } + else { + node->loosen(); + + element_t real_root = this->root_node; + this->root_node = node; + T data = this->pop(); + + element_t new_root = this->root_node; + this->root_node = real_root; + + if (new_root != nullptr) { + this->root_insert(new_root); + } + return data; } } @@ -406,6 +404,8 @@ class PairingHeap final { * erase all elements on the heap. */ void clear() { + auto delete_node = [](element_t node) { delete node; }; + this->iter_all(delete_node, true); this->root_node = nullptr; this->node_count = 0; #if OPENAGE_PAIRINGHEAP_DEBUG @@ -579,14 +579,17 @@ class PairingHeap final { } #endif - void iter_all(const std::function &func) const { - this->walk_tree(this->root_node, func); + void iter_all(const std::function &func, bool reverse = true) const { + this->walk_tree(this->root_node, func, reverse); } -protected: +private: void walk_tree(const element_t &root, - const std::function &func) const { - func(root); + const std::function &func, + bool reverse = false) const { + if (!reverse) { + func(root); + } if (root) { auto node = root->first_child; @@ -595,12 +598,16 @@ class PairingHeap final { break; } - this->walk_tree(node, func); + this->walk_tree(node, func, reverse); node = node->next_sibling; } + if (reverse) { + func(root); + } } } + /** * adds the given node to the heap. * use this if the node was not in the heap before. @@ -608,10 +615,9 @@ class PairingHeap final { */ void push_node(const element_t &node) { this->root_insert(node); - #if OPENAGE_PAIRINGHEAP_DEBUG - auto ins = this->nodes.insert(node); - if (not ins.second) { + auto [iter, result] = this->nodes.insert(node); + if (not result) { throw Error{ERR << "node already known"}; } #endif @@ -631,7 +637,6 @@ class PairingHeap final { } } -protected: compare cmp; size_t node_count; element_t root_node; diff --git a/libopenage/datastructure/tests.cpp b/libopenage/datastructure/tests.cpp index 95fa02a9da..c375f78c17 100644 --- a/libopenage/datastructure/tests.cpp +++ b/libopenage/datastructure/tests.cpp @@ -1,4 +1,4 @@ -// Copyright 2014-2023 the openage authors. See copying.md for legal info. +// Copyright 2014-2024 the openage authors. See copying.md for legal info. #include "tests.h" @@ -118,7 +118,8 @@ void pairing_heap_2() { heap.push(heap_elem{3}); // state: 1 2 3, now remove 2 - heap.unlink_node(node); + auto data = heap.remove_node(node); + TESTEQUALS(data.data, 2); // state: 1 3 TESTEQUALS(heap.pop().data, 1); diff --git a/libopenage/event/eventstore.cpp b/libopenage/event/eventstore.cpp index 7e88b4e638..f8a949f7a9 100644 --- a/libopenage/event/eventstore.cpp +++ b/libopenage/event/eventstore.cpp @@ -1,4 +1,4 @@ -// Copyright 2018-2023 the openage authors. See copying.md for legal info. +// Copyright 2018-2024 the openage authors. See copying.md for legal info. #include "eventstore.h" @@ -56,7 +56,7 @@ bool EventStore::erase(const std::shared_ptr &event) { bool erased = false; auto it = this->events.find(event); if (it != std::end(this->events)) { - this->heap.unlink_node(it->second); + this->heap.remove_node(it->second); this->events.erase(it); erased = true; } From 5a0c3eccccd5aac6eb855b4771bb83214980ee46 Mon Sep 17 00:00:00 2001 From: heinezen Date: Mon, 25 Nov 2024 19:00:18 +0100 Subject: [PATCH 2/2] datastructure: Make iteration order a constexpr template parameter. --- libopenage/datastructure/pairing_heap.h | 40 +++++++++++++++++-------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/libopenage/datastructure/pairing_heap.h b/libopenage/datastructure/pairing_heap.h index 8a571d1ea2..dd348d8c55 100644 --- a/libopenage/datastructure/pairing_heap.h +++ b/libopenage/datastructure/pairing_heap.h @@ -405,7 +405,7 @@ class PairingHeap final { */ void clear() { auto delete_node = [](element_t node) { delete node; }; - this->iter_all(delete_node, true); + this->iter_all(delete_node); this->root_node = nullptr; this->node_count = 0; #if OPENAGE_PAIRINGHEAP_DEBUG @@ -579,30 +579,44 @@ class PairingHeap final { } #endif - void iter_all(const std::function &func, bool reverse = true) const { - this->walk_tree(this->root_node, func, reverse); + /** + * Apply the given function to all nodes in the tree. + * + * @tparam reverse If true, the function is applied to the nodes in reverse order. + * @param func Function to apply to each node. + */ + template + void iter_all(const std::function &func) const { + this->walk_tree(this->root_node, func); } private: - void walk_tree(const element_t &root, - const std::function &func, - bool reverse = false) const { - if (!reverse) { - func(root); + /** + * Apply the given function to all nodes in the tree. + * + * @tparam reverse If true, the function is applied to the nodes in reverse order. + * @param start Starting node. + * @param func Function to apply to each node. + */ + template + void walk_tree(const element_t &start, + const std::function &func) const { + if constexpr (not reverse) { + func(start); } - if (root) { - auto node = root->first_child; + if (start) { + auto node = start->first_child; while (true) { if (not node) { break; } - this->walk_tree(node, func, reverse); + this->walk_tree(node, func); node = node->next_sibling; } - if (reverse) { - func(root); + if constexpr (reverse) { + func(start); } } }