From afd601940ccc3b9de53a550b3034ab98a6f644a9 Mon Sep 17 00:00:00 2001 From: Danny Robson Date: Thu, 23 May 2019 12:36:52 +1000 Subject: [PATCH] pool: initial attempts at clear --- pool.hpp | 80 ++++++++++++++++++++++++++++++++++++++++++++++++--- test/pool.cpp | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 4 deletions(-) diff --git a/pool.hpp b/pool.hpp index 7dbfb48a..d7fd7131 100644 --- a/pool.hpp +++ b/pool.hpp @@ -29,6 +29,7 @@ namespace cruft { union node { alignas(node*) std::atomic next; + alignas(node*) node* raw; alignas(T) char data[sizeof(T)]; }; @@ -77,16 +78,19 @@ namespace cruft { // allocate the memory and note the base address for deletion in destructor m_next = m_head = new node[m_capacity]; - clear (); + relink (); } ~pool () { + clear (); + // don't check if everything's been returned as pools are often used // for PODs which don't need to be destructed via calling release. delete [] m_head; } + // Data management [[nodiscard]] T* allocate [[gnu::malloc]] [[gnu::returns_nonnull]] (void) @@ -158,18 +162,86 @@ namespace cruft { bool full (void) const { return size () == capacity (); } + /// Destroys all objects that have been allocated, frees the + /// associated memory, and then rebuilds the free node list ready for + /// allocations again. + /// + /// NOTE: All bets are off if any object throws an exception out of + /// their destructor. We provide no exception guarantees. + /// + /// This call is NOT thread safe. No users should be accessing this + /// object for the duration of this call. void clear (void) { + // Create a fake root so that we can always point to the parent + // of every node in the system. Hopefully this isn't too large for + // the stack. + node container; + container.next.store (m_next.load ()); + + // Sort the node list. We walk the list, and at each step reparent + // the child at the lowest memory address to the cursor. + for (node* start = container.raw; start; start = start->raw) { + node* parent = start; + + // Find the node whose child is the lowest pointer + int count = 0; + for (auto cursor = parent; cursor->raw; cursor = cursor->raw) { + ++count; + CHECK_NEQ (cursor->raw, start); + if (cursor->raw < parent) + parent = cursor; + } + + // Parent the lowest child to the start of the sorted list + auto tmp = start->raw; + start->raw = parent->raw; + + // Remove the lowest child from their old parent + auto parent_next = parent->raw; + parent->raw = parent_next ? parent_next->raw : nullptr; + + // Parent the old successor of the start to the lowest child + start->raw = tmp; + } + + // Now that we've ordered the nodes we can walk the list from + // start to finish and find nodes that aren't in the free list. + // Call the destructors on the data contained in these. + auto node_cursor = m_next.load (std::memory_order_relaxed); + auto data_cursor = m_head; + + while (node_cursor) { + while (data_cursor < node_cursor) { + cruft::cast::alignment (data_cursor->data)->~T (); + ++data_cursor; + } + + node_cursor = node_cursor->raw; + ++data_cursor; + } + + while (data_cursor < m_head + m_capacity) { + cruft::cast::alignment (data_cursor->data)->~T (); + ++data_cursor; + } + + relink (); + } + + private: + void relink (void) + { + // Reset the allocation cursor to point to the start of the region m_next = m_head; - // build out a complete singly linked list from all the nodes. + // build out the linked list from all the nodes. for (size_t i = 0; i < m_capacity - 1; ++i) m_next[i].next = m_next + i + 1; - m_next[m_capacity - 1].next = nullptr; } - + public: // Indexing size_t index (T const *ptr) const { diff --git a/test/pool.cpp b/test/pool.cpp index b972edf3..78c73485 100644 --- a/test/pool.cpp +++ b/test/pool.cpp @@ -1,6 +1,7 @@ #include "../pool.hpp" #include "../tap.hpp" +#include "../random.hpp" #include #include @@ -138,6 +139,84 @@ check_size_queries (cruft::TAP::logger &tap) } +//----------------------------------------------------------------------------- +static void +check_destructors (cruft::TAP::logger &tap) +{ + struct counter { + counter (int *_target): target (_target) { ; } + + counter (counter const &) = delete; + counter& operator= (counter const &) = delete; + + counter (counter &&) = delete; + counter& operator= (counter &&) = delete; + + ~counter () { if (target) ++*target; } + + int *target; + }; + + int count = 0; + int expected = 0; + + { + cruft::pool data (8); + auto *first = data.construct (&count); + auto const *second = data.construct (&count); + (void)second; + + CHECK_EQ (count, 0); + + data.destroy (first); + ++expected; + tap.expect_eq (count, expected, "destructors run on destroy"); + } + + expected++; + tap.expect_eq (count, expected, "single destructor run on pool destructor"); + + // Make an pool without allocations and destroy it. This isn't reported + // via TAP, but is used as a sanity check that the destructor doesn't run + // into infinite loops or other such problems. + { + cruft::pool data (8); + } + + // Destroy a full pool + { + cruft::pool data (8); + for (int i = 0; i < 8; ++i) { + data.construct (&count); + ++expected; + } + } + + tap.expect_eq (count, expected, "prestine full pool destructor triggers all data desctructors"); + + { + cruft::pool data (8); + counter* items[8]; + for (int i = 0; i < 8; ++i) { + items[i] = data.construct (&count); + ++expected; + } + + for (int round = 0; round < 128; ++round) { + auto idx = cruft::random::uniform (std::size (items) - 1); + data.destroy (items[idx]); + items[idx] = data.construct (&count); + ++expected; + } + } + + tap.expect_eq (count, expected, "randomised full pool destructor triggers all data desctructors"); + + cruft::pool data (100*1024); + (void)data; +} + + //----------------------------------------------------------------------------- int main (int, char **) @@ -148,5 +227,6 @@ main (int, char **) check_keep_value (tap); check_keep_variadic_value (tap); check_size_queries (tap); + check_destructors (tap); }); }