From e0ab7763da348198894e3cb544079931373fcaf2 Mon Sep 17 00:00:00 2001 From: Lenny Maiorani Date: Sat, 17 Dec 2022 18:06:29 -0700 Subject: [PATCH] AK: Combine SinglyLinkedList and SinglyLinkedListWithCount Using policy based design `SinglyLinkedList` and `SinglyLinkedListWithCount` can be combined into one class which takes a policy to determine how to keep track of the size of the list. The default policy is to use list iteration to count the items in the list each time. The `WithCount` form is a different policy which tracks the size, but comes with the overhead of storing the count and incrementing/decrementing on each modification. This model is extensible to have other forms of counting by implementing only a new policy instead of implementing a totally new type. --- AK/Forward.h | 3 +- AK/SinglyLinkedList.h | 18 ++-- AK/SinglyLinkedListSizePolicy.h | 47 ++++++++++ AK/SinglyLinkedListWithCount.h | 144 ------------------------------ Kernel/Net/IPv4Socket.h | 4 +- Tests/AK/TestSinglyLinkedList.cpp | 123 ++++++++++++++++++++++++- 6 files changed, 183 insertions(+), 156 deletions(-) create mode 100644 AK/SinglyLinkedListSizePolicy.h delete mode 100644 AK/SinglyLinkedListWithCount.h diff --git a/AK/Forward.h b/AK/Forward.h index bb487627b7..3803009a4a 100644 --- a/AK/Forward.h +++ b/AK/Forward.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include namespace AK { @@ -64,7 +65,7 @@ using Bytes = Span; template class Atomic; -template +template class SinglyLinkedList; template diff --git a/AK/SinglyLinkedList.h b/AK/SinglyLinkedList.h index 06b7c0eedb..3c04b0eea9 100644 --- a/AK/SinglyLinkedList.h +++ b/AK/SinglyLinkedList.h @@ -63,7 +63,7 @@ private: bool m_removed { false }; }; -template +template class SinglyLinkedList { private: struct Node { @@ -96,12 +96,9 @@ public: bool is_empty() const { return !head(); } - inline size_t size_slow() const + inline size_t size() const { - size_t size = 0; - for (auto* node = m_head; node; node = node->next) - ++size; - return size; + return m_size_policy.size(m_head); } void clear() @@ -113,6 +110,7 @@ public: } m_head = nullptr; m_tail = nullptr; + m_size_policy.reset(); } T& first() @@ -144,6 +142,7 @@ public: if (m_tail == m_head) m_tail = nullptr; m_head = m_head->next; + m_size_policy.decrease_size(value); delete prev_head; return value; } @@ -154,6 +153,7 @@ public: auto* node = new (nothrow) Node(forward(value)); if (!node) return Error::from_errno(ENOMEM); + m_size_policy.increase_size(value); if (!m_head) { m_head = node; m_tail = node; @@ -170,6 +170,7 @@ public: auto* node = new (nothrow) Node(forward(value)); if (!node) return Error::from_errno(ENOMEM); + m_size_policy.increase_size(value); if (!m_head) { m_head = node; m_tail = node; @@ -237,6 +238,7 @@ public: auto* node = new (nothrow) Node(forward(value)); if (!node) return Error::from_errno(ENOMEM); + m_size_policy.increase_size(value); node->next = iterator.m_node; if (m_head == iterator.m_node) m_head = node; @@ -254,6 +256,7 @@ public: auto* node = new (nothrow) Node(forward(value)); if (!node) return Error::from_errno(ENOMEM); + m_size_policy.increase_size(value); node->next = iterator.m_node->next; iterator.m_node->next = node; @@ -286,6 +289,7 @@ public: m_tail = iterator.m_prev; if (iterator.m_prev) iterator.m_prev->next = iterator.m_node->next; + m_size_policy.decrease_size(iterator.m_node->value); delete iterator.m_node; } @@ -298,8 +302,8 @@ private: Node* m_head { nullptr }; Node* m_tail { nullptr }; + TSizeCalculationPolicy m_size_policy {}; }; - } #if USING_AK_GLOBALLY diff --git a/AK/SinglyLinkedListSizePolicy.h b/AK/SinglyLinkedListSizePolicy.h new file mode 100644 index 0000000000..b0123f01b6 --- /dev/null +++ b/AK/SinglyLinkedListSizePolicy.h @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2022, the SerenityOS developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace AK { + +struct DefaultSizeCalculationPolicy { + constexpr void increase_size(auto const&) { } + + constexpr void decrease_size(auto const&) { } + + constexpr void reset() { } + + constexpr size_t size(auto const* head) const + { + size_t size = 0; + for (auto* node = head; node; node = node->next) + ++size; + return size; + } +}; + +struct CountingSizeCalculationPolicy { + constexpr void increase_size(auto const&) { ++m_size; } + + constexpr void decrease_size(auto const&) { --m_size; } + + constexpr void reset() { m_size = 0; } + + constexpr size_t size(auto const*) const { return m_size; } + +private: + size_t m_size { 0 }; +}; + +} + +#ifdef USING_AK_GLOBALLY +using AK::CountingSizeCalculationPolicy; +using AK::DefaultSizeCalculationPolicy; +#endif diff --git a/AK/SinglyLinkedListWithCount.h b/AK/SinglyLinkedListWithCount.h deleted file mode 100644 index 5fad36955f..0000000000 --- a/AK/SinglyLinkedListWithCount.h +++ /dev/null @@ -1,144 +0,0 @@ -/* - * Copyright (c) 2021, Brian Gianforcaro - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include - -namespace AK { - -template -class SinglyLinkedListWithCount : private SinglyLinkedList { - -public: - SinglyLinkedListWithCount() = default; - ~SinglyLinkedListWithCount() = default; - - using List = SinglyLinkedList; - - using List::is_empty; - using List::size_slow; - - inline size_t size() const - { - return m_count; - } - - void clear() - { - List::clear(); - m_count = 0; - } - - T& first() - { - return List::first(); - } - - T const& first() const - { - return List::first(); - } - - T& last() - { - return List::last(); - } - - T const& last() const - { - return List::last(); - } - - T take_first() - { - m_count--; - return List::take_first(); - } - - template - ErrorOr try_append(U&& value) - { - auto result = List::try_append(forward(value)); - if (!result.is_error()) - m_count++; - return result; - } - -#ifndef KERNEL - template - void append(U&& value) - { - MUST(try_append(forward(value))); - } -#endif - - bool contains_slow(T const& value) const - { - return List::contains_slow(value); - } - - using Iterator = typename List::Iterator; - friend Iterator; - Iterator begin() { return List::begin(); } - Iterator end() { return List::end(); } - - using ConstIterator = typename List::ConstIterator; - friend ConstIterator; - ConstIterator begin() const { return List::begin(); } - ConstIterator end() const { return List::end(); } - - template - ConstIterator find(TUnaryPredicate&& pred) const - { - return List::find_if(forward(pred)); - } - - template - Iterator find(TUnaryPredicate&& pred) - { - return List::find_if(forward(pred)); - } - - ConstIterator find(T const& value) const - { - return List::find(value); - } - - Iterator find(T const& value) - { - return List::find(value); - } - - void remove(Iterator iterator) - { - m_count--; - return List::remove(iterator); - } - - template - void insert_before(Iterator iterator, U&& value) - { - m_count++; - List::insert_before(iterator, forward(value)); - } - - template - void insert_after(Iterator iterator, U&& value) - { - m_count++; - List::insert_after(iterator, forward(value)); - } - -private: - size_t m_count { 0 }; -}; - -} - -#if USING_AK_GLOBALLY -using AK::SinglyLinkedListWithCount; -#endif diff --git a/Kernel/Net/IPv4Socket.h b/Kernel/Net/IPv4Socket.h index fb4784d3a4..74169b5d8a 100644 --- a/Kernel/Net/IPv4Socket.h +++ b/Kernel/Net/IPv4Socket.h @@ -7,7 +7,7 @@ #pragma once #include -#include +#include #include #include #include @@ -116,7 +116,7 @@ private: OwnPtr data; }; - SinglyLinkedListWithCount m_receive_queue; + SinglyLinkedList m_receive_queue; OwnPtr m_receive_buffer; diff --git a/Tests/AK/TestSinglyLinkedList.cpp b/Tests/AK/TestSinglyLinkedList.cpp index 308e59f15a..3356c491d6 100644 --- a/Tests/AK/TestSinglyLinkedList.cpp +++ b/Tests/AK/TestSinglyLinkedList.cpp @@ -63,10 +63,129 @@ TEST_CASE(should_find_const_with_predicate) TEST_CASE(removal_during_iteration) { auto list = make_list(); - auto size = list.size_slow(); + auto size = list.size(); for (auto it = list.begin(); it != list.end(); ++it, --size) { - VERIFY(list.size_slow() == size); + VERIFY(list.size() == size); it.remove(list); } } + +static size_t calls_to_increase { 0 }; +static size_t calls_to_decrease { 0 }; +static size_t calls_to_reset { 0 }; +static size_t calls_to_get_size { 0 }; + +static void setup() +{ + calls_to_increase = 0; + calls_to_decrease = 0; + calls_to_reset = 0; + calls_to_get_size = 0; +} + +struct TestSizeCalculationPolicy { + void increase_size(auto const&) { ++calls_to_increase; } + + void decrease_size(auto const&) { ++calls_to_decrease; } + + void reset() { ++calls_to_reset; } + + size_t size(auto const*) const + { + ++calls_to_get_size; + return 42; + } +}; + +TEST_CASE(should_increase_size_when_appending) +{ + setup(); + SinglyLinkedList list {}; + list.append(0); + EXPECT_EQ(1u, calls_to_increase); +} + +TEST_CASE(should_decrease_size_when_removing) +{ + setup(); + SinglyLinkedList list {}; + list.append(0); + auto begin = list.begin(); + list.remove(begin); + EXPECT_EQ(1u, calls_to_decrease); +} + +TEST_CASE(should_reset_size_when_clearing) +{ + setup(); + SinglyLinkedList list {}; + list.append(0); + list.clear(); + EXPECT_EQ(1u, calls_to_reset); +} + +TEST_CASE(should_get_size_from_policy) +{ + setup(); + SinglyLinkedList list {}; + EXPECT_EQ(42u, list.size()); + EXPECT_EQ(1u, calls_to_get_size); +} + +TEST_CASE(should_decrease_size_when_taking_first) +{ + setup(); + SinglyLinkedList list {}; + list.append(0); + list.take_first(); + EXPECT_EQ(1u, calls_to_decrease); +} + +TEST_CASE(should_increase_size_when_try_appending) +{ + setup(); + SinglyLinkedList list {}; + MUST(list.try_append(0)); + EXPECT_EQ(1u, calls_to_increase); +} + +TEST_CASE(should_increase_size_when_try_prepending) +{ + setup(); + SinglyLinkedList list {}; + MUST(list.try_prepend(0)); + EXPECT_EQ(1u, calls_to_increase); +} + +TEST_CASE(should_increase_size_when_try_inserting_before) +{ + setup(); + SinglyLinkedList list {}; + MUST(list.try_insert_before(list.begin(), 42)); + EXPECT_EQ(1u, calls_to_increase); +} + +TEST_CASE(should_increase_size_when_try_inserting_after) +{ + setup(); + SinglyLinkedList list {}; + MUST(list.try_insert_after(list.begin(), 42)); + EXPECT_EQ(1u, calls_to_increase); +} + +TEST_CASE(should_increase_size_when_inserting_before) +{ + setup(); + SinglyLinkedList list {}; + list.insert_before(list.begin(), 42); + EXPECT_EQ(1u, calls_to_increase); +} + +TEST_CASE(should_increase_size_when_inserting_after) +{ + setup(); + SinglyLinkedList list {}; + list.insert_after(list.begin(), 42); + EXPECT_EQ(1u, calls_to_increase); +}