From fb5d77158a183da47619db5a7885644fe8f6ccc3 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 12 Nov 2022 08:52:58 +0100 Subject: [PATCH] util/IntrusiveList: add enum LinkMode Compile-time code simplification. --- src/RemoteTagCache.hxx | 2 +- src/client/Client.hxx | 2 +- src/db/plugins/simple/Directory.hxx | 2 +- src/db/plugins/simple/Song.hxx | 2 +- src/event/SocketEvent.hxx | 4 +- src/input/cache/Lease.hxx | 2 +- src/io/uring/CancellableOperation.hxx | 3 +- src/lib/nfs/Cancellable.hxx | 2 +- src/lib/upnp/Discovery.hxx | 2 +- src/output/plugins/httpd/HttpdClient.hxx | 2 +- src/output/plugins/snapcast/Client.hxx | 2 +- src/util/IntrusiveHookMode.hxx | 56 ++++++++++++ src/util/IntrusiveList.hxx | 104 +++++++++++------------ test/util/TestIntrusiveList.cxx | 28 +++--- 14 files changed, 132 insertions(+), 81 deletions(-) create mode 100644 src/util/IntrusiveHookMode.hxx diff --git a/src/RemoteTagCache.hxx b/src/RemoteTagCache.hxx index b99b7624c..a2051a575 100644 --- a/src/RemoteTagCache.hxx +++ b/src/RemoteTagCache.hxx @@ -46,7 +46,7 @@ class RemoteTagCache final { struct Item final : public boost::intrusive::unordered_set_base_hook>, - public IntrusiveListHook, + public IntrusiveListHook<>, RemoteTagHandler { RemoteTagCache &parent; diff --git a/src/client/Client.hxx b/src/client/Client.hxx index 0ff71ae7f..a8770483c 100644 --- a/src/client/Client.hxx +++ b/src/client/Client.hxx @@ -53,7 +53,7 @@ class Client final friend struct ClientPerPartitionListHook; friend class ClientList; - IntrusiveListHook list_siblings, partition_siblings; + IntrusiveListHook<> list_siblings, partition_siblings; CoarseTimerEvent timeout_event; diff --git a/src/db/plugins/simple/Directory.hxx b/src/db/plugins/simple/Directory.hxx index 69a6b3195..00499cea6 100644 --- a/src/db/plugins/simple/Directory.hxx +++ b/src/db/plugins/simple/Directory.hxx @@ -51,7 +51,7 @@ static constexpr unsigned DEVICE_PLAYLIST = -3; class SongFilter; -struct Directory : IntrusiveListHook { +struct Directory : IntrusiveListHook<> { /* Note: the #IntrusiveListHook is protected with the global #db_mutex. Read access in the update thread does not need protection. */ diff --git a/src/db/plugins/simple/Song.hxx b/src/db/plugins/simple/Song.hxx index 17b6b6a83..f575a6c9d 100644 --- a/src/db/plugins/simple/Song.hxx +++ b/src/db/plugins/simple/Song.hxx @@ -39,7 +39,7 @@ class ArchiveFile; * A song file inside the configured music directory. Internal * #SimpleDatabase class. */ -struct Song : IntrusiveListHook { +struct Song : IntrusiveListHook<> { /* Note: the #IntrusiveListHook is protected with the global #db_mutex. Read access in the update thread does not need protection. */ diff --git a/src/event/SocketEvent.hxx b/src/event/SocketEvent.hxx index 0e8f95470..9603a1a1d 100644 --- a/src/event/SocketEvent.hxx +++ b/src/event/SocketEvent.hxx @@ -40,7 +40,9 @@ class EventLoop; * thread that runs the #EventLoop, except where explicitly documented * as thread-safe. */ -class SocketEvent final : IntrusiveListHook, public EventPollBackendEvents +class SocketEvent final + : IntrusiveListHook, + public EventPollBackendEvents { friend class EventLoop; friend struct IntrusiveListBaseHookTraits; diff --git a/src/input/cache/Lease.hxx b/src/input/cache/Lease.hxx index 833d7b601..55cfa1080 100644 --- a/src/input/cache/Lease.hxx +++ b/src/input/cache/Lease.hxx @@ -29,7 +29,7 @@ * A lease for an #InputCacheItem. */ class InputCacheLease - : public IntrusiveListHook + : public IntrusiveListHook<> { InputCacheItem *item = nullptr; diff --git a/src/io/uring/CancellableOperation.hxx b/src/io/uring/CancellableOperation.hxx index e495fefe1..5cd600e2e 100644 --- a/src/io/uring/CancellableOperation.hxx +++ b/src/io/uring/CancellableOperation.hxx @@ -40,7 +40,8 @@ namespace Uring { -class CancellableOperation : public IntrusiveListHook +class CancellableOperation + : public IntrusiveListHook { Operation *operation; diff --git a/src/lib/nfs/Cancellable.hxx b/src/lib/nfs/Cancellable.hxx index 3aa71bf4d..55276c8a4 100644 --- a/src/lib/nfs/Cancellable.hxx +++ b/src/lib/nfs/Cancellable.hxx @@ -27,7 +27,7 @@ template class CancellablePointer - : public IntrusiveListHook + : public IntrusiveListHook<> { public: typedef T *pointer; diff --git a/src/lib/upnp/Discovery.hxx b/src/lib/upnp/Discovery.hxx index c0cecadb2..02255f57d 100644 --- a/src/lib/upnp/Discovery.hxx +++ b/src/lib/upnp/Discovery.hxx @@ -79,7 +79,7 @@ class UPnPDeviceDirectory final : UpnpCallback { }; class Downloader final - : public IntrusiveListHook, CurlResponseHandler + : public IntrusiveListHook<>, CurlResponseHandler { InjectEvent defer_start_event; diff --git a/src/output/plugins/httpd/HttpdClient.hxx b/src/output/plugins/httpd/HttpdClient.hxx index 4ec01033a..95914211c 100644 --- a/src/output/plugins/httpd/HttpdClient.hxx +++ b/src/output/plugins/httpd/HttpdClient.hxx @@ -34,7 +34,7 @@ class HttpdOutput; class HttpdClient final : BufferedSocket, - public IntrusiveListHook + public IntrusiveListHook<> { /** * The httpd output object this client is connected to. diff --git a/src/output/plugins/snapcast/Client.hxx b/src/output/plugins/snapcast/Client.hxx index f06ea88f3..fef64eb31 100644 --- a/src/output/plugins/snapcast/Client.hxx +++ b/src/output/plugins/snapcast/Client.hxx @@ -33,7 +33,7 @@ struct SnapcastTime; class SnapcastOutput; class UniqueSocketDescriptor; -class SnapcastClient final : BufferedSocket, public IntrusiveListHook +class SnapcastClient final : BufferedSocket, public IntrusiveListHook<> { SnapcastOutput &output; diff --git a/src/util/IntrusiveHookMode.hxx b/src/util/IntrusiveHookMode.hxx new file mode 100644 index 000000000..238c68055 --- /dev/null +++ b/src/util/IntrusiveHookMode.hxx @@ -0,0 +1,56 @@ +/* + * Copyright 2022 Max Kellermann + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#pragma once + +/** + * Specifies the mode in which a hook for intrusive containers + * operates. This is meant to be used as a template argument to the + * hook class (e.g. #IntrusiveListHook). + */ +enum class IntrusiveHookMode { + /** + * No implicit initialization. + */ + NORMAL, + + /** + * Keep track of whether the item is currently linked, allows + * using method is_linked(). This requires implicit + * initialization and requires iterating all items when + * deleting them which adds a considerable amount of overhead. + */ + TRACK, + + /** + * Automatically unlinks the item in the destructor. This + * implies #TRACK and adds code to the destructor. + */ + AUTO_UNLINK, +}; diff --git a/src/util/IntrusiveList.hxx b/src/util/IntrusiveList.hxx index 570440b38..75c6cd0fa 100644 --- a/src/util/IntrusiveList.hxx +++ b/src/util/IntrusiveList.hxx @@ -30,6 +30,7 @@ #pragma once #include "Cast.hxx" +#include "IntrusiveHookMode.hxx" #include "MemberPointer.hxx" #include "OptionalCounter.hxx" @@ -47,6 +48,7 @@ struct IntrusiveListNode { } }; +template class IntrusiveListHook { template friend struct IntrusiveListBaseHookTraits; template friend struct IntrusiveListMemberHookTraits; @@ -56,13 +58,33 @@ protected: IntrusiveListNode siblings; public: - IntrusiveListHook() noexcept = default; + static constexpr IntrusiveHookMode mode = _mode; + + IntrusiveListHook() noexcept { + if constexpr (mode >= IntrusiveHookMode::TRACK) + siblings.next = nullptr; + } + + ~IntrusiveListHook() noexcept { + if constexpr (mode >= IntrusiveHookMode::AUTO_UNLINK) + if (is_linked()) + unlink(); + } IntrusiveListHook(const IntrusiveListHook &) = delete; IntrusiveListHook &operator=(const IntrusiveListHook &) = delete; void unlink() noexcept { IntrusiveListNode::Connect(*siblings.prev, *siblings.next); + + if constexpr (mode >= IntrusiveHookMode::TRACK) + siblings.next = nullptr; + } + + bool is_linked() const noexcept { + static_assert(mode >= IntrusiveHookMode::TRACK); + + return siblings.next != nullptr; } private: @@ -75,52 +97,27 @@ private: } }; -/** - * A variant of #IntrusiveListHook which keeps track of whether it is - * currently in a list. - */ -class SafeLinkIntrusiveListHook : public IntrusiveListHook { -public: - SafeLinkIntrusiveListHook() noexcept { - siblings.next = nullptr; - } - - void unlink() noexcept { - IntrusiveListHook::unlink(); - siblings.next = nullptr; - } - - bool is_linked() const noexcept { - return siblings.next != nullptr; - } -}; +using SafeLinkIntrusiveListHook = + IntrusiveListHook; +using AutoUnlinkIntrusiveListHook = + IntrusiveListHook; /** - * A variant of #IntrusiveListHook which auto-unlinks itself from the - * list upon destruction. As a side effect, it has an is_linked() - * method. - */ -class AutoUnlinkIntrusiveListHook : public SafeLinkIntrusiveListHook { -public: - ~AutoUnlinkIntrusiveListHook() noexcept { - if (is_linked()) - unlink(); - } -}; - -/** - * Detect the hook type; this is important because - * SafeLinkIntrusiveListHook::unlink() needs to clear the "next" - * pointer. This is a template to postpone the type checks, to allow + * Detect the hook type which is embedded in the given type as a base + * class. This is a template to postpone the type checks, to allow * forward-declared types. */ template struct IntrusiveListHookDetection { - static_assert(std::is_base_of_v); - - using type = std::conditional_t, - SafeLinkIntrusiveListHook, - IntrusiveListHook>; + /* TODO can this be simplified somehow, without checking for + all possible enum values? */ + using type = std::conditional_t, U>, + IntrusiveListHook, + std::conditional_t, U>, + IntrusiveListHook, + std::conditional_t, U>, + IntrusiveListHook, + void>>>; }; /** @@ -131,10 +128,6 @@ struct IntrusiveListBaseHookTraits { template using Hook = typename IntrusiveListHookDetection::type; - static constexpr bool IsAutoUnlink() noexcept { - return std::is_base_of_v; - } - static constexpr T *Cast(IntrusiveListNode *node) noexcept { auto *hook = &Hook::Cast(*node); return static_cast(hook); @@ -152,14 +145,12 @@ template struct IntrusiveListMemberHookTraits { using T = MemberPointerContainerType; using _Hook = MemberPointerType; - using Hook = typename IntrusiveListHookDetection<_Hook>::type; - static constexpr bool IsAutoUnlink() noexcept { - return std::is_base_of_v; - } + template + using Hook = _Hook; static constexpr T *Cast(IntrusiveListNode *node) noexcept { - auto &hook = Hook::Cast(*node); + auto &hook = Hook::Cast(*node); return &ContainerCast(hook, member); } @@ -176,14 +167,15 @@ template, bool constant_time_size=false> class IntrusiveList { - template - using Hook = typename IntrusiveListHookDetection::type; - IntrusiveListNode head{&head, &head}; [[no_unique_address]] OptionalCounter counter; + static constexpr auto GetHookMode() noexcept { + return HookTraits::template Hook::mode; + } + static constexpr T *Cast(IntrusiveListNode *node) noexcept { return HookTraits::Cast(node); } @@ -234,7 +226,7 @@ public: } ~IntrusiveList() noexcept { - if constexpr (std::is_base_of_v) + if constexpr (GetHookMode() >= IntrusiveHookMode::TRACK) clear(); } @@ -283,7 +275,7 @@ public: } void clear() noexcept { - if constexpr (std::is_base_of_v) { + if constexpr (GetHookMode() >= IntrusiveHookMode::TRACK) { /* for SafeLinkIntrusiveListHook, we need to remove each item manually, or else its is_linked() method will not work */ @@ -504,7 +496,7 @@ public: void insert(iterator p, reference t) noexcept { static_assert(!constant_time_size || - !HookTraits::IsAutoUnlink(), + GetHookMode() < IntrusiveHookMode::AUTO_UNLINK, "Can't use auto-unlink hooks with constant_time_size"); auto &existing_node = ToNode(*p); diff --git a/test/util/TestIntrusiveList.cxx b/test/util/TestIntrusiveList.cxx index 1039324e6..a553c1979 100644 --- a/test/util/TestIntrusiveList.cxx +++ b/test/util/TestIntrusiveList.cxx @@ -36,17 +36,17 @@ namespace { -template -struct CharItem final : Hook { +template +struct CharItem final : IntrusiveListHook { char ch; constexpr CharItem(char _ch) noexcept:ch(_ch) {} }; -template +template static std::string -ToString(const IntrusiveList> &list, - typename IntrusiveList>::const_iterator it, +ToString(const IntrusiveList> &list, + typename IntrusiveList>::const_iterator it, std::size_t n) noexcept { std::string result; @@ -55,10 +55,10 @@ ToString(const IntrusiveList> &list, return result; } -template +template static std::string -ToStringReverse(const IntrusiveList> &list, - typename IntrusiveList>::const_iterator it, +ToStringReverse(const IntrusiveList> &list, + typename IntrusiveList>::const_iterator it, std::size_t n) noexcept { std::string result; @@ -71,7 +71,7 @@ ToStringReverse(const IntrusiveList> &list, TEST(IntrusiveList, Basic) { - using Item = CharItem; + using Item = CharItem; Item items[]{'a', 'b', 'c'}; @@ -103,9 +103,9 @@ TEST(IntrusiveList, Basic) ASSERT_EQ(ToStringReverse(list, list.begin(), 6), "a_cfea"); } -TEST(IntrusiveList, SafeLink) +TEST(IntrusiveList, Track) { - using Item = CharItem; + using Item = CharItem; Item items[]{'a', 'b', 'c'}; @@ -162,7 +162,7 @@ TEST(IntrusiveList, SafeLink) TEST(IntrusiveList, AutoUnlink) { - using Item = CharItem; + using Item = CharItem; Item a{'a'}; ASSERT_FALSE(a.is_linked()); @@ -194,7 +194,7 @@ TEST(IntrusiveList, AutoUnlink) TEST(IntrusiveList, Merge) { - using Item = CharItem; + using Item = CharItem; const auto predicate = [](const Item &a, const Item &b){ return a.ch < b.ch; @@ -229,7 +229,7 @@ TEST(IntrusiveList, Merge) TEST(IntrusiveList, Sort) { - using Item = CharItem; + using Item = CharItem; const auto predicate = [](const Item &a, const Item &b){ return a.ch < b.ch;