From b3504c942a6a8aa3159553c5c401ab0c3bf5c809 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com>
Date: Wed, 4 Sep 2024 20:15:14 +0300
Subject: [PATCH 1/5] Make the Node a lightweight type holding pointers rather
 than the variant

---
 tests/src/tests/tests.assembler.cpp         |  4 +-
 zasm/include/zasm/program/node.hpp          | 51 ++++++++++++++++-----
 zasm/src/zasm/src/program/program.cpp       | 28 +++++++++--
 zasm/src/zasm/src/program/program.node.hpp  |  4 +-
 zasm/src/zasm/src/program/program.state.hpp | 15 ++++++
 5 files changed, 82 insertions(+), 20 deletions(-)

diff --git a/tests/src/tests/tests.assembler.cpp b/tests/src/tests/tests.assembler.cpp
index 29f60c11..ec6ea18b 100644
--- a/tests/src/tests/tests.assembler.cpp
+++ b/tests/src/tests/tests.assembler.cpp
@@ -51,14 +51,14 @@ namespace zasm::tests
         Program program(MachineMode::AMD64);
 
         x86::Assembler assembler(program);
-        
+
         ASSERT_EQ(assembler.mov(x86::rax, x86::rax), ErrorCode::None);
         auto* nodeA = assembler.getCursor();
         ASSERT_EQ(assembler.mov(x86::rdx, x86::rdx), ErrorCode::None);
         auto* nodeB = assembler.getCursor();
         ASSERT_EQ(assembler.mov(x86::rbx, x86::rbx), ErrorCode::None);
         auto* nodeC = assembler.getCursor();
-        
+
         program.destroy(nodeC);
         ASSERT_EQ(assembler.getCursor(), nodeB);
 
diff --git a/zasm/include/zasm/program/node.hpp b/zasm/include/zasm/program/node.hpp
index 3f16c771..ead1c7ea 100644
--- a/zasm/include/zasm/program/node.hpp
+++ b/zasm/include/zasm/program/node.hpp
@@ -22,6 +22,27 @@ namespace zasm
     };
     ZASM_ENABLE_ENUM_OPERATORS(NodeFlags);
 
+    template<typename T> constexpr std::size_t TypeHash()
+    {
+        std::size_t result{ 14695981039346656037 };
+
+#ifdef _MSC_VER
+#    define F __FUNCSIG__
+#else
+#    define F __PRETTY_FUNCTION__
+#endif
+
+        for (const auto& c : F)
+        {
+            result ^= c;
+            result *= 1099511628211;
+        }
+
+        return result;
+    }
+
+    template<typename T> constexpr std::size_t constexpr_hash = TypeHash<std::decay_t<T>>();
+
     /// <summary>
     /// A type to hold data such as Instruction, Label, Data etc. within a doubly
     /// linked list managed by the Program. The data is internally stored as a variant
@@ -40,7 +61,7 @@ namespace zasm
         NodeFlags _flags{};
         Node* _prev{};
         Node* _next{};
-        std::variant<Sentinel, Instruction, Label, EmbeddedLabel, Data, Section, Align> _data{};
+        std::variant<Sentinel*, Instruction*, Label*, EmbeddedLabel*, Data*, Section*, Align*> _data2{};
 
         union
         {
@@ -51,9 +72,9 @@ namespace zasm
     protected:
         // Internal use only.
         template<typename T>
-        constexpr Node(Id nodeId, T&& val) noexcept
+        constexpr Node(Id nodeId, T* val) noexcept
             : _id{ nodeId }
-            , _data{ std::forward<T>(val) }
+            , _data2{ val }
         {
         }
 
@@ -97,7 +118,7 @@ namespace zasm
         /// <returns>True if the T is the current type</returns>
         template<typename T> constexpr bool holds() const noexcept
         {
-            return std::holds_alternative<T>(_data);
+            return std::holds_alternative<T*>(_data2);
         }
 
         /// <summary>
@@ -108,13 +129,13 @@ namespace zasm
         /// <returns>Returns a reference to the data with the type of T</returns>
         template<typename T> constexpr const T& get() const
         {
-            return std::get<T>(_data);
+            return *std::get<T*>(_data2);
         }
 
         /// <see cref="get"/>
         template<typename T> constexpr T& get()
         {
-            return std::get<T>(_data);
+            return *std::get<T*>(_data2);
         }
 
         /// <summary>
@@ -125,13 +146,19 @@ namespace zasm
         /// <returns>Pointer of type T</returns>
         template<typename T> constexpr const T* getIf() const noexcept
         {
-            return std::get_if<T>(&_data);
+            auto r = std::get_if<T*>(&_data2);
+            if (r == nullptr)
+                return nullptr;
+            return *r;
         }
 
         /// <see cref="getIf"/>
         template<typename T> constexpr T* getIf() noexcept
         {
-            return std::get_if<T>(&_data);
+            auto r = std::get_if<T*>(&_data2);
+            if (r == nullptr)
+                return nullptr;
+            return *r;
         }
 
         /// <summary>
@@ -141,15 +168,15 @@ namespace zasm
         /// <typeparam name="F">Function type</typeparam>
         /// <param name="func">Visitor function</param>
         /// <returns>The result of the visitor function</returns>
-        template<typename F> constexpr auto visit(F&& func) const
+        template<typename TPred> constexpr auto visit(TPred&& func) const
         {
-            return std::visit(std::forward<F>(func), _data);
+            return std::visit([&](auto&& obj) { return func(*obj); }, _data2);
         }
 
         /// <see cref="visit"/>
-        template<typename F> constexpr auto visit(F&& func)
+        template<typename TPred> constexpr auto visit(TPred&& func)
         {
-            return std::visit(std::forward<F>(func), _data);
+            return std::visit([&](auto&& obj) { return func(*obj); }, _data2);
         }
 
         /// <summary>
diff --git a/zasm/src/zasm/src/program/program.cpp b/zasm/src/zasm/src/program/program.cpp
index bb50ad51..6574e81f 100644
--- a/zasm/src/zasm/src/program/program.cpp
+++ b/zasm/src/zasm/src/program/program.cpp
@@ -127,8 +127,8 @@ namespace zasm
         return entry.node;
     }
 
-    template<bool TNotify, typename F, typename... TArgs>
-    static void notifyObservers(const F&& func, const std::vector<Observer*>& observers, TArgs&&... args) noexcept
+    template<bool TNotify, typename TPred, typename... TArgs>
+    static void notifyObservers(const TPred&& func, const std::vector<Observer*>& observers, TArgs&&... args) noexcept
     {
         if constexpr (TNotify)
         {
@@ -374,6 +374,20 @@ namespace zasm
 
         // Release.
         auto* nodeToDestroy = detail::toInternal(node);
+
+        node->visit([&](auto& ptr) {
+
+            using T = std::decay_t<decltype(ptr)>;
+
+            auto& objectPool = state.nodePools.getPool<T>();
+            objectPool.destroy(&ptr);
+
+            if (!quickDestroy)
+            {
+                objectPool.deallocate(&ptr, 1);
+            }
+        });
+        
         state.nodePool.destroy(nodeToDestroy);
 
         if (!quickDestroy)
@@ -437,7 +451,7 @@ namespace zasm
         return _state->entryPoint;
     }
 
-    template<typename... TArgs> Node* createNode_(detail::ProgramState& state, TArgs&&... args)
+    template<typename T> Node* createNode_(detail::ProgramState& state, T&& object)
     {
         const auto nextId = state.nextNodeId;
         state.nextNodeId = static_cast<Node::Id>(static_cast<std::underlying_type_t<Node::Id>>(nextId) + 1U);
@@ -449,7 +463,13 @@ namespace zasm
             return nullptr;
         }
 
-        ::new ((void*)node) detail::Node(nextId, std::forward<TArgs&&>(args)...);
+        using ObjectType = std::decay_t<T>;
+        auto& objectPool = state.nodePools.getPool<ObjectType>();
+
+        auto* obj = objectPool.allocate(1);
+        ::new ((void*)obj) ObjectType(std::move(object));
+
+        ::new ((void*)node) detail::Node(nextId, obj);
 
         notifyObservers<true>(&Observer::onNodeCreated, state.observer, node);
 
diff --git a/zasm/src/zasm/src/program/program.node.hpp b/zasm/src/zasm/src/program/program.node.hpp
index 00476013..43577d6d 100644
--- a/zasm/src/zasm/src/program/program.node.hpp
+++ b/zasm/src/zasm/src/program/program.node.hpp
@@ -11,8 +11,8 @@ namespace zasm
         {
         public:
             template<typename T>
-            constexpr Node(zasm::Node::Id id, T&& val) noexcept
-                : ::zasm::Node(id, std::forward<T>(val))
+            constexpr Node(zasm::Node::Id id, T* val) noexcept
+                : ::zasm::Node(id, val)
             {
             }
             void setPrev(::zasm::Node* node) noexcept
diff --git a/zasm/src/zasm/src/program/program.state.hpp b/zasm/src/zasm/src/program/program.state.hpp
index 1496adad..b80f2fd4 100644
--- a/zasm/src/zasm/src/program/program.state.hpp
+++ b/zasm/src/zasm/src/program/program.state.hpp
@@ -11,6 +11,7 @@
 
 #include <Zydis/Zydis.h>
 #include <cstddef>
+#include <tuple>
 #include <vector>
 #include <zasm/base/label.hpp>
 #include <zasm/base/mode.hpp>
@@ -46,10 +47,24 @@ namespace zasm::detail
         zasm::Node* node{};
     };
 
+    template<typename... TTypes> struct ObjectPools
+    {
+        std::tuple<ObjectPool<TTypes, PoolSize>...> pools;
+
+        template<typename T> auto& getPool()
+        {
+            return std::get<ObjectPool<T, PoolSize>>(pools);
+        }
+    };
+
+    using NodePools = ObjectPools<Sentinel, Instruction, Label, EmbeddedLabel, Data, Section, Align>;
+
     struct NodeStorage
     {
         ObjectPool<Node, PoolSize> nodePool;
         Node::Id nextNodeId{};
+
+        NodePools nodePools;
     };
 
     struct NodeList

From 73b9e9c3beed18996d2852db150a8568c59dbcd4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com>
Date: Thu, 5 Sep 2024 07:56:01 +0300
Subject: [PATCH 2/5] Unify the object pools, specialization for pool types

---
 .../src/benchmarks/benchmark.assembler.cpp    |  9 ++--
 zasm/src/zasm/src/program/program.cpp         | 20 +++++----
 zasm/src/zasm/src/program/program.state.hpp   | 44 ++++++++++++-------
 3 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/benchmark/src/benchmarks/benchmark.assembler.cpp b/benchmark/src/benchmarks/benchmark.assembler.cpp
index 9d9fd15b..e9ba5a9f 100644
--- a/benchmark/src/benchmarks/benchmark.assembler.cpp
+++ b/benchmark/src/benchmarks/benchmark.assembler.cpp
@@ -120,10 +120,13 @@ namespace zasm::benchmarks
             assembler.setCursor(nullptr);
             state.ResumeTiming();
 
-            for (const auto& instr : zasm::tests::data::Instructions)
+            for (int i = 0; i < 100; i++)
             {
-                instr.emitter(assembler);
-                numInstructions++;
+                for (const auto& instr : zasm::tests::data::Instructions)
+                {
+                    instr.emitter(assembler);
+                    numInstructions++;
+                }
             }
         }
 
diff --git a/zasm/src/zasm/src/program/program.cpp b/zasm/src/zasm/src/program/program.cpp
index 6574e81f..031cec9c 100644
--- a/zasm/src/zasm/src/program/program.cpp
+++ b/zasm/src/zasm/src/program/program.cpp
@@ -376,10 +376,9 @@ namespace zasm
         auto* nodeToDestroy = detail::toInternal(node);
 
         node->visit([&](auto& ptr) {
-
             using T = std::decay_t<decltype(ptr)>;
 
-            auto& objectPool = state.nodePools.getPool<T>();
+            auto& objectPool = state.objectPools.get<T>();
             objectPool.destroy(&ptr);
 
             if (!quickDestroy)
@@ -387,13 +386,14 @@ namespace zasm
                 objectPool.deallocate(&ptr, 1);
             }
         });
-        
-        state.nodePool.destroy(nodeToDestroy);
+
+        auto& nodePool = state.objectPools.get<Node>();
+        nodePool.destroy(nodeToDestroy);
 
         if (!quickDestroy)
         {
             // Release memory, when quickDestroy is true the entire pool will be cleared at once.
-            state.nodePool.deallocate(nodeToDestroy, 1);
+            nodePool.deallocate(nodeToDestroy, 1);
 
             // Remove mapping.
             auto& nodeMap = state.nodeMap;
@@ -438,7 +438,7 @@ namespace zasm
         _state->sections.clear();
         _state->labels.clear();
         _state->symbolNames.clear();
-        _state->nodePool.reset();
+        _state->objectPools.reset();
     }
 
     void Program::setEntryPoint(const Label& label)
@@ -456,19 +456,21 @@ namespace zasm
         const auto nextId = state.nextNodeId;
         state.nextNodeId = static_cast<Node::Id>(static_cast<std::underlying_type_t<Node::Id>>(nextId) + 1U);
 
-        auto& pool = state.nodePool;
-        auto* node = detail::toInternal(pool.allocate(1));
+        auto& nodePool = state.objectPools.get<Node>();
+        auto* node = detail::toInternal(nodePool.allocate(1));
         if (node == nullptr)
         {
             return nullptr;
         }
 
+        // Construct object.
         using ObjectType = std::decay_t<T>;
-        auto& objectPool = state.nodePools.getPool<ObjectType>();
+        auto& objectPool = state.objectPools.get<ObjectType>();
 
         auto* obj = objectPool.allocate(1);
         ::new ((void*)obj) ObjectType(std::move(object));
 
+        // Construct node.
         ::new ((void*)node) detail::Node(nextId, obj);
 
         notifyObservers<true>(&Observer::onNodeCreated, state.observer, node);
diff --git a/zasm/src/zasm/src/program/program.state.hpp b/zasm/src/zasm/src/program/program.state.hpp
index b80f2fd4..4c2e2a16 100644
--- a/zasm/src/zasm/src/program/program.state.hpp
+++ b/zasm/src/zasm/src/program/program.state.hpp
@@ -23,8 +23,6 @@ namespace zasm
 
 namespace zasm::detail
 {
-    constexpr std::size_t PoolSize = 1U << 10;
-
     struct LabelData
     {
         Label::Id id{ Label::Id::Invalid };
@@ -47,25 +45,36 @@ namespace zasm::detail
         zasm::Node* node{};
     };
 
-    template<typename... TTypes> struct ObjectPools
+    namespace detail
     {
-        std::tuple<ObjectPool<TTypes, PoolSize>...> pools;
+        template<typename T>
+        struct PoolSize
+        {
+            static constexpr std::size_t kSize = 50'000;
+        };
 
-        template<typename T> auto& getPool()
+        template<> struct PoolSize<Instruction>
         {
-            return std::get<ObjectPool<T, PoolSize>>(pools);
-        }
-    };
+            static constexpr std::size_t kSize = 30'000;
+        };
 
-    using NodePools = ObjectPools<Sentinel, Instruction, Label, EmbeddedLabel, Data, Section, Align>;
+        template<typename... TTypes> struct ObjectPools
+        {
+            std::tuple<ObjectPool<TTypes, PoolSize<TTypes>::kSize>...> pools;
 
-    struct NodeStorage
-    {
-        ObjectPool<Node, PoolSize> nodePool;
-        Node::Id nextNodeId{};
+            template<typename T> auto& get()
+            {
+                return std::get<ObjectPool<T, PoolSize<T>::kSize>>(pools);
+            }
 
-        NodePools nodePools;
-    };
+            void reset()
+            {
+                std::apply([](auto&... pool) { (pool.reset(), ...); }, pools);
+            }
+        };
+    } // namespace detail
+
+    using ObjectPools = detail::ObjectPools<zasm::Node, Sentinel, Instruction, Label, EmbeddedLabel, Data, Section, Align>;
 
     struct NodeList
     {
@@ -79,7 +88,7 @@ namespace zasm::detail
         StringPool symbolNames;
     };
 
-    struct ProgramState : NodeStorage, NodeList, Symbols
+    struct ProgramState : NodeList, Symbols
     {
         MachineMode mode{};
 
@@ -94,6 +103,9 @@ namespace zasm::detail
 
         Label entryPoint{ Label::Id::Invalid };
 
+        ObjectPools objectPools;
+        Node::Id nextNodeId{};
+
         ProgramState(MachineMode m)
             : mode(m)
         {

From c7d29026d2ebe46a1c91dd845c748725358ea644 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com>
Date: Thu, 5 Sep 2024 08:02:43 +0300
Subject: [PATCH 3/5] Remove pointless code

---
 zasm/include/zasm/program/node.hpp | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/zasm/include/zasm/program/node.hpp b/zasm/include/zasm/program/node.hpp
index ead1c7ea..a4621e24 100644
--- a/zasm/include/zasm/program/node.hpp
+++ b/zasm/include/zasm/program/node.hpp
@@ -22,27 +22,6 @@ namespace zasm
     };
     ZASM_ENABLE_ENUM_OPERATORS(NodeFlags);
 
-    template<typename T> constexpr std::size_t TypeHash()
-    {
-        std::size_t result{ 14695981039346656037 };
-
-#ifdef _MSC_VER
-#    define F __FUNCSIG__
-#else
-#    define F __PRETTY_FUNCTION__
-#endif
-
-        for (const auto& c : F)
-        {
-            result ^= c;
-            result *= 1099511628211;
-        }
-
-        return result;
-    }
-
-    template<typename T> constexpr std::size_t constexpr_hash = TypeHash<std::decay_t<T>>();
-
     /// <summary>
     /// A type to hold data such as Instruction, Label, Data etc. within a doubly
     /// linked list managed by the Program. The data is internally stored as a variant

From 2d1a21c978a56a9331e00616cd4f8b704c41ca4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com>
Date: Thu, 5 Sep 2024 08:06:01 +0300
Subject: [PATCH 4/5] Rename variable

---
 zasm/include/zasm/program/node.hpp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/zasm/include/zasm/program/node.hpp b/zasm/include/zasm/program/node.hpp
index a4621e24..e89603bd 100644
--- a/zasm/include/zasm/program/node.hpp
+++ b/zasm/include/zasm/program/node.hpp
@@ -40,7 +40,7 @@ namespace zasm
         NodeFlags _flags{};
         Node* _prev{};
         Node* _next{};
-        std::variant<Sentinel*, Instruction*, Label*, EmbeddedLabel*, Data*, Section*, Align*> _data2{};
+        std::variant<Sentinel*, Instruction*, Label*, EmbeddedLabel*, Data*, Section*, Align*> _data{};
 
         union
         {
@@ -53,7 +53,7 @@ namespace zasm
         template<typename T>
         constexpr Node(Id nodeId, T* val) noexcept
             : _id{ nodeId }
-            , _data2{ val }
+            , _data{ val }
         {
         }
 
@@ -97,7 +97,7 @@ namespace zasm
         /// <returns>True if the T is the current type</returns>
         template<typename T> constexpr bool holds() const noexcept
         {
-            return std::holds_alternative<T*>(_data2);
+            return std::holds_alternative<T*>(_data);
         }
 
         /// <summary>
@@ -108,13 +108,13 @@ namespace zasm
         /// <returns>Returns a reference to the data with the type of T</returns>
         template<typename T> constexpr const T& get() const
         {
-            return *std::get<T*>(_data2);
+            return *std::get<T*>(_data);
         }
 
         /// <see cref="get"/>
         template<typename T> constexpr T& get()
         {
-            return *std::get<T*>(_data2);
+            return *std::get<T*>(_data);
         }
 
         /// <summary>
@@ -125,7 +125,7 @@ namespace zasm
         /// <returns>Pointer of type T</returns>
         template<typename T> constexpr const T* getIf() const noexcept
         {
-            auto r = std::get_if<T*>(&_data2);
+            auto r = std::get_if<T*>(&_data);
             if (r == nullptr)
                 return nullptr;
             return *r;
@@ -134,7 +134,7 @@ namespace zasm
         /// <see cref="getIf"/>
         template<typename T> constexpr T* getIf() noexcept
         {
-            auto r = std::get_if<T*>(&_data2);
+            auto r = std::get_if<T*>(&_data);
             if (r == nullptr)
                 return nullptr;
             return *r;
@@ -149,13 +149,13 @@ namespace zasm
         /// <returns>The result of the visitor function</returns>
         template<typename TPred> constexpr auto visit(TPred&& func) const
         {
-            return std::visit([&](auto&& obj) { return func(*obj); }, _data2);
+            return std::visit([&](auto&& obj) { return func(*obj); }, _data);
         }
 
         /// <see cref="visit"/>
         template<typename TPred> constexpr auto visit(TPred&& func)
         {
-            return std::visit([&](auto&& obj) { return func(*obj); }, _data2);
+            return std::visit([&](auto&& obj) { return func(*obj); }, _data);
         }
 
         /// <summary>

From 74d5cbb458c5d85718f4522db8ec677b36957932 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com>
Date: Thu, 5 Sep 2024 08:12:43 +0300
Subject: [PATCH 5/5] Add test for node type

---
 tests/src/tests/tests.program.cpp | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tests/src/tests/tests.program.cpp b/tests/src/tests/tests.program.cpp
index 019f2903..b206274f 100644
--- a/tests/src/tests/tests.program.cpp
+++ b/tests/src/tests/tests.program.cpp
@@ -347,4 +347,26 @@ namespace zasm::tests
         ASSERT_EQ(imm.value<int>(), 3);
     }
 
+    TEST(ProgramTests, TestNodeType)
+    {
+        Program program(MachineMode::AMD64);
+
+        auto testIns = Instruction{}.setMnemonic(x86::Mnemonic::Add);
+
+        auto* node = program.createNode(testIns);
+        ASSERT_NE(node, nullptr);
+
+        ASSERT_TRUE(node->holds<Instruction>());
+
+        ASSERT_FALSE(node->holds<Data>());
+
+        auto* inst = node->getIf<Instruction>();
+        ASSERT_NE(inst, nullptr);
+
+        ASSERT_EQ(testIns, *inst);
+
+        auto* data = node->getIf<Data>();
+        ASSERT_EQ(data, nullptr);
+    }
+
 } // namespace zasm::tests