From cb9720baabf9bed1f501556cf6fbce88cd657514 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Wed, 8 Sep 2021 03:07:34 +0300 Subject: [PATCH] AK: Set IntrusiveRBTree Node key on insertion instead of construction This makes the API look much nicer. --- AK/IntrusiveRedBlackTree.h | 8 +--- AK/RedBlackTree.h | 3 ++ Tests/AK/TestIntrusiveRedBlackTree.cpp | 59 ++++++++++++-------------- 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/AK/IntrusiveRedBlackTree.h b/AK/IntrusiveRedBlackTree.h index 811ff5d23c..8d372b30e7 100644 --- a/AK/IntrusiveRedBlackTree.h +++ b/AK/IntrusiveRedBlackTree.h @@ -48,10 +48,11 @@ public: return node_to_value(*node); } - void insert(V& value) + void insert(K key, V& value) { auto& node = value.*member; VERIFY(!node.m_in_tree); + node.key = key; BaseTree::insert(&node); if constexpr (!TreeNode::IsRaw) node.m_self.reference = &value; // Note: Self-reference ensures that the object will keep a ref to itself when the Container is a smart pointer. @@ -165,11 +166,6 @@ namespace Detail { template class IntrusiveRedBlackTreeNode : public BaseRedBlackTree::Node { public: - IntrusiveRedBlackTreeNode(K key) - : BaseRedBlackTree::Node(key) - { - } - ~IntrusiveRedBlackTreeNode() { VERIFY(!is_in_tree()); diff --git a/AK/RedBlackTree.h b/AK/RedBlackTree.h index df23ed4607..adce1cc5f7 100644 --- a/AK/RedBlackTree.h +++ b/AK/RedBlackTree.h @@ -38,6 +38,9 @@ public: : key(key) { } + Node() + { + } virtual ~Node() {}; }; diff --git a/Tests/AK/TestIntrusiveRedBlackTree.cpp b/Tests/AK/TestIntrusiveRedBlackTree.cpp index 3ae0c7fe56..a27e8abb18 100644 --- a/Tests/AK/TestIntrusiveRedBlackTree.cpp +++ b/Tests/AK/TestIntrusiveRedBlackTree.cpp @@ -12,9 +12,8 @@ class IntrusiveTest { public: - IntrusiveTest(int key, int value) - : m_tree_node(key) - , m_some_value(value) + IntrusiveTest(int value) + : m_some_value(value) { } @@ -33,12 +32,12 @@ TEST_CASE(construct) TEST_CASE(ints) { IntrusiveRBTree test; - IntrusiveTest first { 1, 10 }; - test.insert(first); - IntrusiveTest second { 3, 20 }; - test.insert(second); - IntrusiveTest third { 2, 30 }; - test.insert(third); + IntrusiveTest first { 10 }; + test.insert(1, first); + IntrusiveTest second { 20 }; + test.insert(3, second); + IntrusiveTest third { 30 }; + test.insert(2, third); EXPECT_EQ(test.size(), 3u); EXPECT_EQ(test.find(3)->m_some_value, 20); EXPECT_EQ(test.find(2)->m_some_value, 30); @@ -53,12 +52,12 @@ TEST_CASE(ints) TEST_CASE(largest_smaller_than) { IntrusiveRBTree test; - IntrusiveTest first { 1, 10 }; - test.insert(first); - IntrusiveTest second { 11, 20 }; - test.insert(second); - IntrusiveTest third { 21, 30 }; - test.insert(third); + IntrusiveTest first { 10 }; + test.insert(1, first); + IntrusiveTest second { 20 }; + test.insert(11, second); + IntrusiveTest third { 30 }; + test.insert(21, third); EXPECT_EQ(test.size(), 3u); EXPECT_EQ(test.find_largest_not_above(3)->m_some_value, 10); EXPECT_EQ(test.find_largest_not_above(17)->m_some_value, 20); @@ -86,8 +85,8 @@ TEST_CASE(key_ordered_iteration) // insert random keys for (size_t i = 0; i < amount; i++) { - auto entry = make(keys[i], keys[i]); - test.insert(*entry); + auto entry = make(keys[i]); + test.insert(keys[i], *entry); m_entries.append(move(entry)); } @@ -108,8 +107,8 @@ TEST_CASE(clear) IntrusiveRBTree test; NonnullOwnPtrVector m_entries; for (size_t i = 0; i < 1000; i++) { - auto entry = make(i, i); - test.insert(*entry); + auto entry = make(i); + test.insert(i, *entry); m_entries.append(move(entry)); } test.clear(); @@ -118,8 +117,7 @@ TEST_CASE(clear) class IntrusiveRefPtrTest : public RefCounted { public: - IntrusiveRefPtrTest(int key) - : m_tree_node(key) + IntrusiveRefPtrTest() { } @@ -129,11 +127,11 @@ using IntrusiveRefPtrRBTree = IntrusiveRedBlackTreeref_count()); IntrusiveRefPtrRBTree ref_tree; - ref_tree.insert(*item); + ref_tree.insert(0, *item); EXPECT_EQ(2u, item->ref_count()); ref_tree.remove(0); @@ -142,11 +140,11 @@ TEST_CASE(intrusive_ref_ptr_no_ref_leaks) TEST_CASE(intrusive_ref_ptr_clear) { - auto item = adopt_ref(*new IntrusiveRefPtrTest(0)); + auto item = adopt_ref(*new IntrusiveRefPtrTest()); EXPECT_EQ(1u, item->ref_count()); IntrusiveRefPtrRBTree ref_tree; - ref_tree.insert(*item); + ref_tree.insert(0, *item); EXPECT_EQ(2u, item->ref_count()); ref_tree.clear(); @@ -155,12 +153,12 @@ TEST_CASE(intrusive_ref_ptr_clear) TEST_CASE(intrusive_ref_ptr_destructor) { - auto item = adopt_ref(*new IntrusiveRefPtrTest(0)); + auto item = adopt_ref(*new IntrusiveRefPtrTest()); EXPECT_EQ(1u, item->ref_count()); { IntrusiveRefPtrRBTree ref_tree; - ref_tree.insert(*item); + ref_tree.insert(0, *item); EXPECT_EQ(2u, item->ref_count()); } @@ -169,8 +167,7 @@ TEST_CASE(intrusive_ref_ptr_destructor) class IntrusiveNonnullRefPtrTest : public RefCounted { public: - IntrusiveNonnullRefPtrTest(int key) - : m_tree_node(key) + IntrusiveNonnullRefPtrTest() { } @@ -180,11 +177,11 @@ using IntrusiveNonnullRefPtrRBTree = IntrusiveRedBlackTreeref_count()); IntrusiveNonnullRefPtrRBTree nonnull_ref_tree; - nonnull_ref_tree.insert(*item); + nonnull_ref_tree.insert(0, *item); EXPECT_EQ(2u, item->ref_count()); EXPECT(!nonnull_ref_tree.is_empty());