Skip to content

Commit

Permalink
Pointer Packing (#553)
Browse files Browse the repository at this point in the history
* Using PackedPointer for OptionalPointer

* Trying to improve AbstractTree

* Apply clang-format

* Comments and CI cleanup

* Fixing MSVC warning

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
jatinchowdhury18 and github-actions[bot] authored Sep 17, 2024
1 parent 3f5ff48 commit 10ca93d
Show file tree
Hide file tree
Showing 19 changed files with 478 additions and 173 deletions.
29 changes: 14 additions & 15 deletions bench/AbstractTreeBench.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ struct DataTree : chowdsp::AbstractTree<FakeData, DataTree>

static Node& insertOneElement (FakeData&& element, Node& parent, AbstractTree& tree)
{
auto* new_node = tree.createEmptyNode();
new_node->leaf = std::move (element);
auto* new_node = tree.createLeafNode (std::move (element));
new_node->prev_sibling = parent.last_child;
if (parent.last_child != nullptr)
parent.last_child->next_sibling = new_node;
Expand All @@ -27,8 +26,7 @@ struct DataTree : chowdsp::AbstractTree<FakeData, DataTree>
AbstractTree& tree,
std::string_view tag)
{
auto* new_sub_tree_node = tree.createEmptyNode();
new_sub_tree_node->tag = tag;
auto* new_sub_tree_node = tree.createTagNode (tag);
new_sub_tree_node->prev_sibling = parent.last_child;
if (parent.last_child != nullptr)
parent.last_child->next_sibling = new_sub_tree_node;
Expand All @@ -38,25 +36,26 @@ struct DataTree : chowdsp::AbstractTree<FakeData, DataTree>

static FakeData& insertElementInternal (AbstractTree& self, FakeData&& element, Node& root)
{
// since we know all the tags ahead of time, we only compare the first letter.
for (auto* iter = root.first_child; iter != nullptr; iter = iter->next_sibling)
{
if (iter->tag == positiveTag && element[0] > 0)
return *insertOneElement (std::move (element), *iter, self).leaf;
if (iter->value.tag()[0] == positiveTag[0] && element[0] > 0)
return insertOneElement (std::move (element), *iter, self).value.leaf();

if (iter->tag == negativeTag && element[0] < 0)
return *insertOneElement (std::move (element), *iter, self).leaf;
if (iter->value.tag()[0] == negativeTag[0] && element[0] < 0)
return insertOneElement (std::move (element), *iter, self).value.leaf();

if (iter->tag == zeroTag && element[0] == 0)
return *insertOneElement (std::move (element), *iter, self).leaf;
if (iter->value.tag()[0] == zeroTag[0] && element[0] == 0)
return insertOneElement (std::move (element), *iter, self).value.leaf();
}

if (element[0] > 0)
return *insertElementIntoNewSubtree (std::move (element), root, self, positiveTag).leaf;
return insertElementIntoNewSubtree (std::move (element), root, self, positiveTag).value.leaf();

if (element[0] < 0)
return *insertElementIntoNewSubtree (std::move (element), root, self, negativeTag).leaf;
return insertElementIntoNewSubtree (std::move (element), root, self, negativeTag).value.leaf();

return *insertElementIntoNewSubtree (std::move (element), root, self, zeroTag).leaf;
return insertElementIntoNewSubtree (std::move (element), root, self, zeroTag).value.leaf();
}
};

Expand Down Expand Up @@ -223,8 +222,8 @@ static void accessTree (benchmark::State& state)
{
if (iter_count == 25)
{
(*iter->leaf)[0]++;
benchmark::DoNotOptimize ((*iter->leaf)[0]);
iter->value.leaf()[0]++;
benchmark::DoNotOptimize (iter->value.leaf()[0]);
break;
}
iter_count++;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
namespace chowdsp
{
template <typename ElementType, typename DerivedType>
AbstractTree<ElementType, DerivedType>::AbstractTree()
AbstractTree<ElementType, DerivedType>::AbstractTree (size_t num_nodes_reserved)
{
reserve (num_nodes_reserved);
clear();
}

template <typename ElementType, typename DerivedType>
AbstractTree<ElementType, DerivedType>::~AbstractTree()
{
doForAllNodes ([] (Node& node)
{ node.leaf.reset(); });
{ node.value.destroy(); });
}

template <typename ElementType, typename DerivedType>
Expand Down Expand Up @@ -79,20 +80,24 @@ void AbstractTree<ElementType, DerivedType>::removeNode (Node& node)

onDelete (node);

if (node.leaf.has_value())
if (node.value.has_value())
count--;

for (auto* iter = &root_node; iter != nullptr; iter = iter->next_linear)
{
if (iter->next_linear == &node)
{
iter->next_linear = node.next_linear;
if (last_node == &node)
last_node = iter;
break;
}
}

if (node.prev_sibling != nullptr)
node.prev_sibling->next_sibling = node.next_sibling;
if (node.next_sibling != nullptr)
node.next_sibling->prev_sibling = node.prev_sibling;
if (node.next_linear != nullptr)
node.next_linear->prev_linear = node.prev_linear;
// we don't need to check if node.prev_linear is nullptr, because the root node is never deleted!
node.prev_linear->next_linear = node.next_linear;

if (last_node == &node)
last_node = node.prev_linear;

if (node.parent->first_child == node.parent->last_child)
{
Expand All @@ -108,15 +113,15 @@ void AbstractTree<ElementType, DerivedType>::removeNode (Node& node)
node.parent->last_child = node.prev_sibling;
}

node.leaf.reset();
node.value.destroy();
}

template <typename ElementType, typename DerivedType>
void AbstractTree<ElementType, DerivedType>::removeElement (const ElementType& element)
{
for (auto* node = &root_node; node != nullptr; node = node->next_linear)
{
if (node->leaf.has_value() && node->leaf == element)
if (node->value.has_value() && node->value.leaf() == element)
{
removeNode (*node);
break;
Expand All @@ -133,7 +138,7 @@ void AbstractTree<ElementType, DerivedType>::removeElements (const Callable& ele
// need to change.
for (auto* node = &root_node; node != nullptr;)
{
if (node->leaf.has_value() && elementsToRemove (*node->leaf))
if (node->value.has_value() && elementsToRemove (node->value.leaf()))
{
auto* next_node = node->next_linear;
removeNode (*node);
Expand All @@ -150,8 +155,8 @@ template <typename ElementType, typename DerivedType>
void AbstractTree<ElementType, DerivedType>::clear()
{
doForAllNodes ([] (Node& node)
{ node.leaf.reset(); });
allocator.reset (64 * sizeof (Node));
{ node.value.destroy(); });
allocator.clear();
count = 0;
root_node = {};
}
Expand Down Expand Up @@ -205,8 +210,8 @@ void AbstractTree<ElementType, DerivedType>::doForAllElements (Callable&& callab
doForAllNodes (
[c = std::forward<Callable> (callable)] (Node& node)
{
if (node.leaf.has_value())
c (*node.leaf);
if (node.value.has_value())
c (node.value.leaf());
});
}

Expand All @@ -217,28 +222,50 @@ void AbstractTree<ElementType, DerivedType>::doForAllElements (Callable&& callab
doForAllNodes (
[c = std::forward<Callable> (callable)] (const Node& node)
{
if (node.leaf.has_value())
c (*node.leaf);
if (node.value.has_value())
c (node.value.leaf());
});
}

template <typename ElementType, typename DerivedType>
typename AbstractTree<ElementType, DerivedType>::Node* AbstractTree<ElementType, DerivedType>::createEmptyNode()
typename AbstractTree<ElementType, DerivedType>::Node* AbstractTree<ElementType, DerivedType>::createTagNode (std::string_view str)
{
auto* new_node = new (allocator.allocate<Node> (1)) Node {};
auto* bytes = (std::byte*) allocator.allocate_bytes (sizeof (Node) + sizeof (std::string_view) + alignof (std::string_view) + str.size(), alignof (Node));

auto* new_node = new (bytes) Node {};
last_node->next_linear = new_node;
new_node->prev_linear = last_node;
last_node = new_node;

bytes = juce::snapPointerToAlignment (bytes + sizeof (Node), alignof (std::string_view));

auto* str_data = (char*) (bytes + sizeof (std::string_view));
std::copy (str.begin(), str.end(), str_data);

auto tag_str_view = new (bytes) std::string_view { str_data, str.size() };
new_node->value.set_tag (tag_str_view);

return new_node;
}

template <typename ElementType, typename DerivedType>
std::string_view AbstractTree<ElementType, DerivedType>::allocateTag (std::string_view str)
void AbstractTree<ElementType, DerivedType>::reserve (size_t num_nodes)
{
auto* str_data = allocator.allocate<char> (str.size());
std::copy (str.begin(), str.end(), str_data);
return { str_data, str.size() }; // NOLINT NOSONAR
if (count > 0)
{
jassertfalse;
return;
}
allocator.reset (num_nodes * (sizeof (Node) + sizeof (ElementType) + alignof (ElementType)));
}

template <typename ElementType, typename DerivedType>
void AbstractTree<ElementType, DerivedType>::shrinkArena()
{
if (count > 0)
{
jassertfalse;
return;
}
allocator.reset (allocator.get_current_arena().get_total_num_bytes());
}
} // namespace chowdsp
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,85 @@ template <typename ElementType, typename DerivedType>
class AbstractTree
{
public:
struct ValuePtr
{
JUCE_BEGIN_IGNORE_WARNINGS_MSVC (4324) // structure was padded due to alignment specifier
struct alignas (8) Void
{
};
PackedPointer<Void> ptr { nullptr, Empty };
JUCE_END_IGNORE_WARNINGS_MSVC

ValuePtr() = default;

enum : uint8_t
{
Empty = 0,
Leaf = 1,
Tag = 2,
};

void set (ElementType* new_leaf)
{
jassert (! has_value());
ptr.set (reinterpret_cast<Void*> (new_leaf), Leaf); // NOSONAR
}

void set_tag (std::string_view* new_tag)
{
jassert (! has_value());
ptr.set (reinterpret_cast<Void*> (new_tag), Tag); // NOSONAR
}

[[nodiscard]] ElementType& leaf()
{
jassert (has_value());
return *(reinterpret_cast<ElementType*> (ptr.get_ptr())); // NOSONAR
}

[[nodiscard]] const ElementType& leaf() const
{
jassert (has_value());
return *(reinterpret_cast<const ElementType*> (ptr.get_ptr())); // NOSONAR
}

[[nodiscard]] std::string_view tag() const
{
jassert (is_tag());
return *(reinterpret_cast<const std::string_view*> (ptr.get_ptr())); // NOSONAR
}

void destroy()
{
if (has_value())
leaf().~ElementType(); // NOSONAR
ptr.set (nullptr, Empty);
}

[[nodiscard]] bool has_value() const noexcept
{
return ptr.get_flags() == Leaf;
}

[[nodiscard]] bool is_tag() const noexcept
{
return ptr.get_flags() == Tag;
}
};

struct Node
{
std::optional<ElementType> leaf { std::nullopt };
std::string_view tag {};
ValuePtr value {};

Node* parent {}; // slot for parent in hierarchy
Node* first_child {}; // slot for first child in hierarchy
Node* last_child {}; // slot for last child in hierarchy
Node* next_sibling {}; // slot for next sibling in hierarchy
Node* prev_sibling {}; // slot for previous sibling in hierarchy
Node* next_linear {}; // slot for linked list through all nodes
Node* prev_linear {}; // slot for linked list through all nodes
};

AbstractTree();
explicit AbstractTree (size_t num_nodes_reserved = 64);
virtual ~AbstractTree();

AbstractTree (const AbstractTree&) = delete;
Expand Down Expand Up @@ -77,19 +141,43 @@ class AbstractTree
template <typename Callable>
void doForAllElements (Callable&& callable) const;

/** Creates a new empty node in the tree's memory arena. */
Node* createEmptyNode();
/** Creates a new tag node in the tree's memory arena. */
Node* createTagNode (std::string_view str);

/** Creates a new leaf node in the tree's memory arena. */
template <typename C = ElementType, typename... Args>
Node* createLeafNode (Args&&... args)
{
auto* bytes = (std::byte*) allocator.allocate_bytes (sizeof (Node) + sizeof (C) + alignof (C), alignof (Node));

auto* new_node = new (bytes) Node {};
last_node->next_linear = new_node;
last_node = new_node;

/** Allocates a new tag in the tree's memory arena. */
std::string_view allocateTag (std::string_view str);
auto* new_obj = new (juce::snapPointerToAlignment (bytes + sizeof (Node), alignof (C))) C (std::forward<Args> (args)...);
new_node->value.set (new_obj);
return new_node;
}

[[nodiscard]] Node& getRootNode() noexcept { return root_node; }
[[nodiscard]] const Node& getRootNode() const noexcept { return root_node; }

/**
* Reserves memory for some number of nodes.
* NOTE: this must be called while the tree is empty.
*/
void reserve (size_t num_nodes);

/**
* Shrinks the tree's memory arena down to the reserved size.
* NOTE: this must be called while the tree is empty.
*/
void shrinkArena();

protected:
virtual void onDelete (const Node& /*nodeBeingDeleted*/) {}

ChainedArenaAllocator allocator { 64 * sizeof (Node) };
ChainedArenaAllocator allocator {};

private:
Node root_node {};
Expand Down
Loading

0 comments on commit 10ca93d

Please sign in to comment.