From 564cc0b3058ada6d960ea7458b6a80ff05437861 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Thu, 14 Nov 2024 13:22:19 -0500 Subject: [PATCH 01/35] Initial commit --- .vscode/settings.json | 73 ++++++++++++++++++- .../evaluablenode/EvaluableNodeManagement.cpp | 45 ++++++++---- .../evaluablenode/EvaluableNodeManagement.h | 35 +++++++++ src/Amalgam/interpreter/Interpreter.h | 3 + .../InterpreterOpcodesEntityAccess.cpp | 3 + .../InterpreterOpcodesEntityControl.cpp | 1 + 6 files changed, 146 insertions(+), 14 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 60d2427e..e73b83d0 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,4 +1,75 @@ { "cSpell.words": ["amlg", "cwrap", "KullbackLeibler"], - "files.trimTrailingWhitespace": false + "files.trimTrailingWhitespace": false, + "files.associations": { + "__bit_reference": "cpp", + "__hash_table": "cpp", + "__locale": "cpp", + "__node_handle": "cpp", + "__split_buffer": "cpp", + "__threading_support": "cpp", + "__tree": "cpp", + "__verbose_abort": "cpp", + "array": "cpp", + "bitset": "cpp", + "cctype": "cpp", + "cfenv": "cpp", + "charconv": "cpp", + "cinttypes": "cpp", + "clocale": "cpp", + "cmath": "cpp", + "codecvt": "cpp", + "complex": "cpp", + "condition_variable": "cpp", + "cstdarg": "cpp", + "cstddef": "cpp", + "cstdint": "cpp", + "cstdio": "cpp", + "cstdlib": "cpp", + "cstring": "cpp", + "ctime": "cpp", + "cwchar": "cpp", + "cwctype": "cpp", + "deque": "cpp", + "execution": "cpp", + "memory": "cpp", + "forward_list": "cpp", + "fstream": "cpp", + "future": "cpp", + "initializer_list": "cpp", + "iomanip": "cpp", + "ios": "cpp", + "iosfwd": "cpp", + "iostream": "cpp", + "istream": "cpp", + "limits": "cpp", + "list": "cpp", + "locale": "cpp", + "map": "cpp", + "mutex": "cpp", + "new": "cpp", + "numbers": "cpp", + "optional": "cpp", + "ostream": "cpp", + "print": "cpp", + "queue": "cpp", + "ratio": "cpp", + "regex": "cpp", + "set": "cpp", + "shared_mutex": "cpp", + "span": "cpp", + "sstream": "cpp", + "stack": "cpp", + "stdexcept": "cpp", + "streambuf": "cpp", + "string": "cpp", + "string_view": "cpp", + "tuple": "cpp", + "typeinfo": "cpp", + "unordered_map": "cpp", + "unordered_set": "cpp", + "variant": "cpp", + "vector": "cpp", + "algorithm": "cpp" + } } diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index f03070de..2cf29593 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #ifdef MULTITHREAD_SUPPORT Concurrency::ReadWriteMutex EvaluableNodeManager::memoryModificationMutex; @@ -192,6 +193,8 @@ void EvaluableNodeManager::CollectGarbage() #ifdef MULTITHREAD_SUPPORT + ClearThreadLocalAllocationBuffer(); + //free lock so can attempt to enter write lock to collect garbage if(memory_modification_lock != nullptr) memory_modification_lock->unlock(); @@ -256,24 +259,40 @@ void EvaluableNodeManager::FreeAllNodes() } EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() -{ - size_t allocated_index = 0; +{ #ifdef MULTITHREAD_SUPPORT { - //attempt to allocate using an atomic without write locking + EvaluableNode* tlabNode = getNextNodeFromTLab(); + + std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; + //Fast Path; get node from thread local buffer + if (tlabNode) { + assert(nodes.size() > 0); + return tlabNode; + } + + //slow path allocation; attempt to allocate using an atomic without write locking Concurrency::ReadLock lock(managerAttributesMutex); - //attempt to allocate a node and make sure it's valid - allocated_index = firstUnusedNodeIndex++; - if(allocated_index < nodes.size()) + //attempt to allocate enough nodes to refill thread local buffer + size_t first_index_to_allocate = firstUnusedNodeIndex.fetch_add(tlabSize); + size_t last_index_to_allocate = first_index_to_allocate + tlabSize; + + if(last_index_to_allocate < nodes.size()) { - if(nodes[allocated_index] == nullptr) - nodes[allocated_index] = new EvaluableNode(); + for (int i = first_index_to_allocate; i < last_index_to_allocate; i++) + { + if(nodes[i] == nullptr) + nodes[i] = new EvaluableNode(); + + threadLocalAllocationBuffer.push_back(nodes[i]); + } - return nodes[allocated_index]; + return getNextNodeFromTLab(); } - //the node wasn't valid; put it back and do a write lock to allocate more - --firstUnusedNodeIndex; + + //couldn't allocate enough valid nodes; reset index and allocate more + firstUnusedNodeIndex -= tlabSize; } //don't have enough nodes, so need to attempt a write lock to allocate more Concurrency::WriteLock write_lock(managerAttributesMutex); @@ -282,9 +301,9 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex //use the cached value for firstUnusedNodeIndex, allocated_index, to check if another thread has performed the allocation //as other threads may have reduced firstUnusedNodeIndex, incurring more unnecessary write locks when a memory expansion is needed -#else - allocated_index = firstUnusedNodeIndex; + #endif + size_t allocated_index = firstUnusedNodeIndex; size_t num_nodes = nodes.size(); if(allocated_index < num_nodes && firstUnusedNodeIndex < num_nodes) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index ba41e66e..71859df9 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -6,6 +6,7 @@ //system headers: #include +#include //if the macro PEDANTIC_GARBAGE_COLLECTION is defined, then garbage collection will be performed //after every opcode, to help find and debug memory issues @@ -1090,6 +1091,16 @@ class EvaluableNodeManager // EvaluableNodeManager and so garbage collection should not happening while memory is being modified static Concurrency::ReadWriteMutex memoryModificationMutex; + + // TODO: Make this private when done debugging + typedef std::vector TLab; + inline static thread_local TLab threadLocalAllocationBuffer; + + static void ClearThreadLocalAllocationBuffer() + { + threadLocalAllocationBuffer.clear(); + } + protected: #else @@ -1114,4 +1125,28 @@ class EvaluableNodeManager //extra space to allocate when allocating static const double allocExpansionFactor; + +private: + + #ifdef MULTITHREAD_SUPPORT + static EvaluableNode* getNextNodeFromTLab() + { + if(threadLocalAllocationBuffer.size() > 0) + { + EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; + threadLocalAllocationBuffer.pop_back(); + return end; + } + else + { + return NULL; + } + + } + + static const int tlabSize = 20; + + #endif // MULTITHREAD_SUPPORT + + }; diff --git a/src/Amalgam/interpreter/Interpreter.h b/src/Amalgam/interpreter/Interpreter.h index 30ef8724..c74727c3 100644 --- a/src/Amalgam/interpreter/Interpreter.h +++ b/src/Amalgam/interpreter/Interpreter.h @@ -753,6 +753,7 @@ class Interpreter result = result_ref; resultsSaver.SetStackLocation(results_saver_location, result); + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -821,6 +822,7 @@ class Interpreter resultsSaver.SetStackLocation(results_saver_location, *result); } + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -831,6 +833,7 @@ class Interpreter inline void EndConcurrency() { //allow other threads to perform garbage collection + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); parentInterpreter->memoryModificationLock.unlock(); taskSet.WaitForTasks(taskEnqueueLock); parentInterpreter->memoryModificationLock.lock(); diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp index 123c1119..11f515e1 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp @@ -489,6 +489,8 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_ENTITY_and_CALL_ENTIT #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing + + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif @@ -608,6 +610,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_CONTAINER(EvaluableNo #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp index 8e12d235..5a4b73b0 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp @@ -664,6 +664,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_LOAD_ENTITY(EvaluableNode #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif From bba20796ac4adbc9bbf5b74b1857a0399204de8f Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Fri, 15 Nov 2024 09:57:14 -0500 Subject: [PATCH 02/35] commented out debug prints. --- src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 6 +++--- src/Amalgam/evaluablenode/EvaluableNodeManagement.h | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 2cf29593..c8968c26 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -262,9 +262,9 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() { #ifdef MULTITHREAD_SUPPORT { - EvaluableNode* tlabNode = getNextNodeFromTLab(); + EvaluableNode* tlabNode = getNextNodeFromTLab(this); - std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; + // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; //Fast Path; get node from thread local buffer if (tlabNode) { assert(nodes.size() > 0); @@ -288,7 +288,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() threadLocalAllocationBuffer.push_back(nodes[i]); } - return getNextNodeFromTLab(); + return getNextNodeFromTLab(this); } //couldn't allocate enough valid nodes; reset index and allocate more diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 71859df9..ff0e029f 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1129,9 +1129,11 @@ class EvaluableNodeManager private: #ifdef MULTITHREAD_SUPPORT - static EvaluableNode* getNextNodeFromTLab() + + static inline thread_local EvaluableNodeManager* lastEvaluableNode; + static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNode) { - if(threadLocalAllocationBuffer.size() > 0) + if(threadLocalAllocationBuffer.size() > 0 && thisEvaluableNode == lastEvaluableNode) { EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; threadLocalAllocationBuffer.pop_back(); @@ -1139,6 +1141,10 @@ class EvaluableNodeManager } else { + if (lastEvaluableNode != thisEvaluableNode) + ClearThreadLocalAllocationBuffer(); + + lastEvaluableNode = thisEvaluableNode; return NULL; } From d8ea43445f66716edc4c84b6647140516118f601 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Fri, 15 Nov 2024 16:21:08 -0500 Subject: [PATCH 03/35] Initilize tlab nodes to state ENT_NULL --- src/Amalgam/evaluablenode/EvaluableNode.h | 2 ++ src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNode.h b/src/Amalgam/evaluablenode/EvaluableNode.h index 850f6031..2824f26a 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.h +++ b/src/Amalgam/evaluablenode/EvaluableNode.h @@ -12,6 +12,8 @@ #include #include +#define AMALGAM_MEMORY_INTEGRITY + //if the macro AMALGAM_MEMORY_INTEGRITY is defined, then it will continuously verify memory, at a high cost of performance //this is useful for diagnosing and debugging memory issues //if the macro AMALGAM_FAST_MEMORY_INTEGRITY is defined, then only the checks that are fast will be made diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index c8968c26..40425c28 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -8,6 +8,9 @@ #include #include + +#define PEDANTIC_GARBAGE_COLLECTION + #ifdef MULTITHREAD_SUPPORT Concurrency::ReadWriteMutex EvaluableNodeManager::memoryModificationMutex; #endif @@ -283,7 +286,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() for (int i = first_index_to_allocate; i < last_index_to_allocate; i++) { if(nodes[i] == nullptr) - nodes[i] = new EvaluableNode(); + nodes[i] = new EvaluableNode(ENT_NULL); threadLocalAllocationBuffer.push_back(nodes[i]); } @@ -293,6 +296,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() //couldn't allocate enough valid nodes; reset index and allocate more firstUnusedNodeIndex -= tlabSize; + ClearThreadLocalAllocationBuffer(); } //don't have enough nodes, so need to attempt a write lock to allocate more Concurrency::WriteLock write_lock(managerAttributesMutex); From 28294d0d3505feca59582eb343532cfe8f70d870 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 18 Nov 2024 16:22:56 -0500 Subject: [PATCH 04/35] Put deallocated nodes back into tlab. --- src/Amalgam/evaluablenode/EvaluableNode.h | 4 ++-- .../evaluablenode/EvaluableNodeManagement.cpp | 16 +++++++++++-- .../evaluablenode/EvaluableNodeManagement.h | 24 +++++++++++++++---- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNode.h b/src/Amalgam/evaluablenode/EvaluableNode.h index 2824f26a..7f78178d 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.h +++ b/src/Amalgam/evaluablenode/EvaluableNode.h @@ -12,8 +12,6 @@ #include #include -#define AMALGAM_MEMORY_INTEGRITY - //if the macro AMALGAM_MEMORY_INTEGRITY is defined, then it will continuously verify memory, at a high cost of performance //this is useful for diagnosing and debugging memory issues //if the macro AMALGAM_FAST_MEMORY_INTEGRITY is defined, then only the checks that are fast will be made @@ -21,6 +19,8 @@ #define AMALGAM_FAST_MEMORY_INTEGRITY #endif +#define AMALGAM_FAST_MEMORY_INTEGRITY + //forward declarations: class EvaluableNodeManager; diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 40425c28..e7f7843e 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -9,7 +9,7 @@ #include -#define PEDANTIC_GARBAGE_COLLECTION +// #define PEDANTIC_GARBAGE_COLLECTION #ifdef MULTITHREAD_SUPPORT Concurrency::ReadWriteMutex EvaluableNodeManager::memoryModificationMutex; @@ -288,7 +288,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() if(nodes[i] == nullptr) nodes[i] = new EvaluableNode(ENT_NULL); - threadLocalAllocationBuffer.push_back(nodes[i]); + addToTLab(this, nodes[i]); } return getNextNodeFromTLab(this); @@ -464,6 +464,11 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree) } tree->Invalidate(); + +#ifdef MULTITHREAD_SUPPORT + tree->InitializeType(ENT_NULL); + addToTLab(this, tree); +#endif } void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) @@ -503,12 +508,19 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) std::swap(ocn, tree_ocn); tree->Invalidate(); + + for(auto &e : ocn) { if(e != nullptr && !e->IsNodeDeallocated()) FreeNodeTreeWithCyclesRecurse(e); } } + + #ifdef MULTITHREAD_SUPPORT + tree->InitializeType(ENT_NULL); + addToTLab(this, tree); + #endif } void EvaluableNodeManager::ModifyLabels(EvaluableNode *n, EvaluableNodeMetadataModifier metadata_modifier) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index ff0e029f..0288ae9b 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1101,6 +1101,7 @@ class EvaluableNodeManager threadLocalAllocationBuffer.clear(); } + protected: #else @@ -1130,10 +1131,10 @@ class EvaluableNodeManager #ifdef MULTITHREAD_SUPPORT - static inline thread_local EvaluableNodeManager* lastEvaluableNode; - static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNode) + static inline thread_local EvaluableNodeManager* lastEvaluableNodeManager; + static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNodeManager) { - if(threadLocalAllocationBuffer.size() > 0 && thisEvaluableNode == lastEvaluableNode) + if(threadLocalAllocationBuffer.size() > 0 && thisEvaluableNodeManager == lastEvaluableNodeManager) { EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; threadLocalAllocationBuffer.pop_back(); @@ -1141,15 +1142,28 @@ class EvaluableNodeManager } else { - if (lastEvaluableNode != thisEvaluableNode) + if (lastEvaluableNodeManager != thisEvaluableNodeManager) ClearThreadLocalAllocationBuffer(); - lastEvaluableNode = thisEvaluableNode; + lastEvaluableNodeManager = thisEvaluableNodeManager; return NULL; } } + static void addToTLab(EvaluableNodeManager* thisEvaulableNodeManager, EvaluableNode* evaluableNode) + { + std::cout << "!!!naricc_debug!!! Addeding evaluabelNode to Tlab: " << evaluableNode << std::endl; + + if(thisEvaulableNodeManager != lastEvaluableNodeManager) + { + threadLocalAllocationBuffer.clear(); + lastEvaluableNodeManager = thisEvaulableNodeManager; + } + + threadLocalAllocationBuffer.push_back(evaluableNode); + } + static const int tlabSize = 20; #endif // MULTITHREAD_SUPPORT From 9d33fedf13675bffdf01d05d342a7f74073e1dc2 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 18 Nov 2024 21:20:08 -0500 Subject: [PATCH 05/35] Moving around some code. --- .../evaluablenode/EvaluableNodeManagement.cpp | 26 +++++++++---------- .../evaluablenode/EvaluableNodeManagement.h | 13 +++------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index e7f7843e..a3686acb 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -263,16 +263,16 @@ void EvaluableNodeManager::FreeAllNodes() EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() { + EvaluableNode* tlabNode = getNextNodeFromTLab(this); + // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; + //Fast Path; get node from thread local buffer + if (tlabNode) { + assert(nodes.size() > 0); + return tlabNode; + } + #ifdef MULTITHREAD_SUPPORT { - EvaluableNode* tlabNode = getNextNodeFromTLab(this); - - // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; - //Fast Path; get node from thread local buffer - if (tlabNode) { - assert(nodes.size() > 0); - return tlabNode; - } //slow path allocation; attempt to allocate using an atomic without write locking Concurrency::ReadLock lock(managerAttributesMutex); @@ -465,10 +465,8 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree) tree->Invalidate(); -#ifdef MULTITHREAD_SUPPORT tree->InitializeType(ENT_NULL); addToTLab(this, tree); -#endif } void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) @@ -498,6 +496,9 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) else if(tree->IsImmediate()) { tree->Invalidate(); + + tree->InitializeType(ENT_NULL); + addToTLab(this, tree); } else //ordered { @@ -517,10 +518,7 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) } } - #ifdef MULTITHREAD_SUPPORT - tree->InitializeType(ENT_NULL); - addToTLab(this, tree); - #endif + } void EvaluableNodeManager::ModifyLabels(EvaluableNode *n, EvaluableNodeMetadataModifier metadata_modifier) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 0288ae9b..e7e51b33 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1080,6 +1080,10 @@ class EvaluableNodeManager Concurrency::ReadWriteMutex managerAttributesMutex; std::atomic firstUnusedNodeIndex; +#else + size_t firstUnusedNodeIndex; +#endif + public: //global mutex to manage whether memory nodes are being modified @@ -1104,10 +1108,6 @@ class EvaluableNodeManager protected: -#else - size_t firstUnusedNodeIndex; -#endif - //nodes that have been allocated and may be in use // all nodes in use are below firstUnusedNodeIndex, such that all above that index are free for use // nodes cannot be nullptr for lower indices than firstUnusedNodeIndex @@ -1129,8 +1129,6 @@ class EvaluableNodeManager private: - #ifdef MULTITHREAD_SUPPORT - static inline thread_local EvaluableNodeManager* lastEvaluableNodeManager; static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNodeManager) { @@ -1166,7 +1164,4 @@ class EvaluableNodeManager static const int tlabSize = 20; - #endif // MULTITHREAD_SUPPORT - - }; From 70c55e0b1f8df7488b7480825524052f0d946b17 Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Tue, 19 Nov 2024 08:36:15 -0500 Subject: [PATCH 06/35] 22156: Cleanup, progress, identified issue --- src/Amalgam/Parser.cpp | 4 +- src/Amalgam/evaluablenode/EvaluableNode.h | 13 +++- .../evaluablenode/EvaluableNodeManagement.cpp | 37 ++++++------ .../evaluablenode/EvaluableNodeManagement.h | 60 ++++++++++--------- src/Amalgam/interpreter/Interpreter.h | 3 - .../InterpreterOpcodesEntityAccess.cpp | 3 - .../InterpreterOpcodesEntityControl.cpp | 1 - 7 files changed, 66 insertions(+), 55 deletions(-) diff --git a/src/Amalgam/Parser.cpp b/src/Amalgam/Parser.cpp index 8cdb5a05..c4ff6f7c 100644 --- a/src/Amalgam/Parser.cpp +++ b/src/Amalgam/Parser.cpp @@ -197,6 +197,8 @@ EvaluableNode *Parser::GetCodeForPathToSharedNodeFromParentAToParentB(UnparseDat if(shared_node == nullptr || a_parent == nullptr || b_parent == nullptr) return nullptr; + //TODO 22156: allocate from enm in a way that does not invalidate tlab (but does not increase the size of EvaluableNodeManager) + //find all parent nodes of a to find collision with parent node of b, along with depth counts EvaluableNode::ReferenceCountType a_parent_nodes; size_t a_ancestor_depth = 1; @@ -989,7 +991,7 @@ void Parser::Unparse(UnparseData &upd, EvaluableNode *tree, EvaluableNode *paren std::swap(upd.parentNodes, references); Unparse(upd, code_to_print, nullptr, expanded_whitespace, indentation_depth, need_initial_indent); std::swap(upd.parentNodes, references); //put the old parentNodes back - enm.FreeNodeTree(code_to_print); + //don't free code_to_print, but let enm's destructor clean it up return; } diff --git a/src/Amalgam/evaluablenode/EvaluableNode.h b/src/Amalgam/evaluablenode/EvaluableNode.h index 7f78178d..16bf8052 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.h +++ b/src/Amalgam/evaluablenode/EvaluableNode.h @@ -147,7 +147,7 @@ class EvaluableNode inline void InitializeType(EvaluableNodeType _type) { #ifdef AMALGAM_FAST_MEMORY_INTEGRITY - assert(IsEvaluableNodeTypeValid(_type)); + assert(IsEvaluableNodeTypeValid(_type) || _type == ENT_DEALLOCATED); #endif type = _type; @@ -173,6 +173,17 @@ class EvaluableNode attributes.individualAttribs.isIdempotent = true; value.ConstructMappedChildNodes(); } + else if(_type == ENT_DEALLOCATED) + { + #ifdef AMALGAM_FAST_MEMORY_INTEGRITY + //use a value that is more apparent that something went wrong + value.numberValueContainer.numberValue = std::numeric_limits::quiet_NaN(); + #else + value.numberValueContainer.numberValue = 0; + #endif + + value.numberValueContainer.labelStringID = StringInternPool::NOT_A_STRING_ID; + } else { value.ConstructOrderedChildNodes(); diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index a3686acb..e2c7c347 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -263,13 +263,11 @@ void EvaluableNodeManager::FreeAllNodes() EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() { - EvaluableNode* tlabNode = getNextNodeFromTLab(this); + EvaluableNode *tlab_node = GetNextNodeFromTLab(); // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; //Fast Path; get node from thread local buffer - if (tlabNode) { - assert(nodes.size() > 0); - return tlabNode; - } + if(tlab_node != nullptr) + return tlab_node; #ifdef MULTITHREAD_SUPPORT { @@ -283,15 +281,15 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() if(last_index_to_allocate < nodes.size()) { - for (int i = first_index_to_allocate; i < last_index_to_allocate; i++) + for(size_t i = first_index_to_allocate; i < last_index_to_allocate; i++) { if(nodes[i] == nullptr) - nodes[i] = new EvaluableNode(ENT_NULL); + nodes[i] = new EvaluableNode(ENT_DEALLOCATED); - addToTLab(this, nodes[i]); + AddNodeToTLab(nodes[i]); } - return getNextNodeFromTLab(this); + return GetNextNodeFromTLab(); } //couldn't allocate enough valid nodes; reset index and allocate more @@ -307,15 +305,16 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() //as other threads may have reduced firstUnusedNodeIndex, incurring more unnecessary write locks when a memory expansion is needed #endif - size_t allocated_index = firstUnusedNodeIndex; + //reduce accesses to the atomic variable for performance + size_t allocated_index = firstUnusedNodeIndex++; size_t num_nodes = nodes.size(); - if(allocated_index < num_nodes && firstUnusedNodeIndex < num_nodes) + if(allocated_index < num_nodes) { - if(nodes[firstUnusedNodeIndex] == nullptr) - nodes[firstUnusedNodeIndex] = new EvaluableNode(); + if(nodes[allocated_index] == nullptr) + nodes[allocated_index] = new EvaluableNode(); - return nodes[firstUnusedNodeIndex++]; + return nodes[allocated_index]; } //ran out, so need another node; push a bunch on the heap so don't need to reallocate as often and slow down garbage collection @@ -324,10 +323,10 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() //fill new EvaluableNode slots with nullptr nodes.resize(new_num_nodes, nullptr); - if(nodes[firstUnusedNodeIndex] == nullptr) - nodes[firstUnusedNodeIndex] = new EvaluableNode(); + if(nodes[allocated_index] == nullptr) + nodes[allocated_index] = new EvaluableNode(); - return nodes[firstUnusedNodeIndex++]; + return nodes[allocated_index]; } void EvaluableNodeManager::FreeAllNodesExceptReferencedNodes(size_t cur_first_unused_node_index) @@ -466,7 +465,7 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree) tree->Invalidate(); tree->InitializeType(ENT_NULL); - addToTLab(this, tree); + AddNodeToTLab(tree); } void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) @@ -498,7 +497,7 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) tree->Invalidate(); tree->InitializeType(ENT_NULL); - addToTLab(this, tree); + AddNodeToTLab(tree); } else //ordered { diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index e7e51b33..df05af37 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1080,10 +1080,6 @@ class EvaluableNodeManager Concurrency::ReadWriteMutex managerAttributesMutex; std::atomic firstUnusedNodeIndex; -#else - size_t firstUnusedNodeIndex; -#endif - public: //global mutex to manage whether memory nodes are being modified @@ -1095,19 +1091,12 @@ class EvaluableNodeManager // EvaluableNodeManager and so garbage collection should not happening while memory is being modified static Concurrency::ReadWriteMutex memoryModificationMutex; - - // TODO: Make this private when done debugging - typedef std::vector TLab; - inline static thread_local TLab threadLocalAllocationBuffer; - - static void ClearThreadLocalAllocationBuffer() - { - threadLocalAllocationBuffer.clear(); - } - - protected: +#else + size_t firstUnusedNodeIndex; +#endif + //nodes that have been allocated and may be in use // all nodes in use are below firstUnusedNodeIndex, such that all above that index are free for use // nodes cannot be nullptr for lower indices than firstUnusedNodeIndex @@ -1117,6 +1106,21 @@ class EvaluableNodeManager //only allocated if needed std::unique_ptr nodesCurrentlyReferenced; +public: + // TODO: Make this private when done debugging + typedef std::vector TLab; + +#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) + thread_local +#endif + inline static TLab threadLocalAllocationBuffer; + + static void ClearThreadLocalAllocationBuffer() + { + threadLocalAllocationBuffer.clear(); + } +protected: + //buffer used for updating EvaluableNodeFlags, particularly UpdateFlagsForNodeTree #if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) thread_local @@ -1127,12 +1131,14 @@ class EvaluableNodeManager //extra space to allocate when allocating static const double allocExpansionFactor; -private: +#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) + thread_local +#endif + static inline EvaluableNodeManager *lastEvaluableNodeManager; - static inline thread_local EvaluableNodeManager* lastEvaluableNodeManager; - static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNodeManager) + inline EvaluableNode *GetNextNodeFromTLab() { - if(threadLocalAllocationBuffer.size() > 0 && thisEvaluableNodeManager == lastEvaluableNodeManager) + if(threadLocalAllocationBuffer.size() > 0 && this == lastEvaluableNodeManager) { EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; threadLocalAllocationBuffer.pop_back(); @@ -1140,26 +1146,26 @@ class EvaluableNodeManager } else { - if (lastEvaluableNodeManager != thisEvaluableNodeManager) + if(lastEvaluableNodeManager != this) ClearThreadLocalAllocationBuffer(); - lastEvaluableNodeManager = thisEvaluableNodeManager; - return NULL; + lastEvaluableNodeManager = this; + return nullptr; } } - static void addToTLab(EvaluableNodeManager* thisEvaulableNodeManager, EvaluableNode* evaluableNode) + inline void AddNodeToTLab(EvaluableNode *en) { - std::cout << "!!!naricc_debug!!! Addeding evaluabelNode to Tlab: " << evaluableNode << std::endl; + //std::cout << "!!!naricc_debug!!! Addeding evaluabelNode to Tlab: " << en << std::endl; - if(thisEvaulableNodeManager != lastEvaluableNodeManager) + if(this != lastEvaluableNodeManager) { threadLocalAllocationBuffer.clear(); - lastEvaluableNodeManager = thisEvaulableNodeManager; + lastEvaluableNodeManager = this; } - threadLocalAllocationBuffer.push_back(evaluableNode); + threadLocalAllocationBuffer.push_back(en); } static const int tlabSize = 20; diff --git a/src/Amalgam/interpreter/Interpreter.h b/src/Amalgam/interpreter/Interpreter.h index c74727c3..30ef8724 100644 --- a/src/Amalgam/interpreter/Interpreter.h +++ b/src/Amalgam/interpreter/Interpreter.h @@ -753,7 +753,6 @@ class Interpreter result = result_ref; resultsSaver.SetStackLocation(results_saver_location, result); - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -822,7 +821,6 @@ class Interpreter resultsSaver.SetStackLocation(results_saver_location, *result); } - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -833,7 +831,6 @@ class Interpreter inline void EndConcurrency() { //allow other threads to perform garbage collection - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); parentInterpreter->memoryModificationLock.unlock(); taskSet.WaitForTasks(taskEnqueueLock); parentInterpreter->memoryModificationLock.lock(); diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp index 11f515e1..123c1119 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp @@ -489,8 +489,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_ENTITY_and_CALL_ENTIT #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing - - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif @@ -610,7 +608,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_CONTAINER(EvaluableNo #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp index 5a4b73b0..8e12d235 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp @@ -664,7 +664,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_LOAD_ENTITY(EvaluableNode #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif From 371fbdaa0aa1bc33d81b34d11907bd2c4d2a4c5d Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:15:23 -0500 Subject: [PATCH 07/35] 22156: Re-adds tlab clears in concurrency --- src/Amalgam/interpreter/Interpreter.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Amalgam/interpreter/Interpreter.h b/src/Amalgam/interpreter/Interpreter.h index 30ef8724..c74727c3 100644 --- a/src/Amalgam/interpreter/Interpreter.h +++ b/src/Amalgam/interpreter/Interpreter.h @@ -753,6 +753,7 @@ class Interpreter result = result_ref; resultsSaver.SetStackLocation(results_saver_location, result); + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -821,6 +822,7 @@ class Interpreter resultsSaver.SetStackLocation(results_saver_location, *result); } + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -831,6 +833,7 @@ class Interpreter inline void EndConcurrency() { //allow other threads to perform garbage collection + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); parentInterpreter->memoryModificationLock.unlock(); taskSet.WaitForTasks(taskEnqueueLock); parentInterpreter->memoryModificationLock.lock(); From 22d2a78ca6522084fc97067ae77513ec53382d25 Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:31:50 -0500 Subject: [PATCH 08/35] 22156: Collects more to tlab from frees --- .../evaluablenode/EvaluableNodeManagement.cpp | 8 ++--- .../evaluablenode/EvaluableNodeManagement.h | 29 +++---------------- 2 files changed, 6 insertions(+), 31 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index e2c7c347..43e6e64f 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -482,6 +482,7 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) auto &tree_mcn = tree->GetMappedChildNodesReference(); std::swap(mcn, tree_mcn); tree->Invalidate(); + AddNodeToTLab(tree); for(auto &[_, e] : mcn) { @@ -495,8 +496,6 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) else if(tree->IsImmediate()) { tree->Invalidate(); - - tree->InitializeType(ENT_NULL); AddNodeToTLab(tree); } else //ordered @@ -507,8 +506,7 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) auto &tree_ocn = tree->GetOrderedChildNodesReference(); std::swap(ocn, tree_ocn); tree->Invalidate(); - - + AddNodeToTLab(tree); for(auto &e : ocn) { @@ -516,8 +514,6 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) FreeNodeTreeWithCyclesRecurse(e); } } - - } void EvaluableNodeManager::ModifyLabels(EvaluableNode *n, EvaluableNodeMetadataModifier metadata_modifier) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index df05af37..3b49e197 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -743,7 +743,7 @@ class EvaluableNodeManager #endif en->Invalidate(); - ReclaimFreedNodesAtEnd(); + AddNodeToTLab(en); } //attempts to free the node reference @@ -759,7 +759,7 @@ class EvaluableNodeManager if(enr.unique && enr != nullptr && !enr->GetNeedCycleCheck()) { enr->Invalidate(); - ReclaimFreedNodesAtEnd(); + AddNodeToTLab(enr); } } @@ -779,6 +779,7 @@ class EvaluableNodeManager if(IsEvaluableNodeTypeImmediate(en->GetType())) { en->Invalidate(); + AddNodeToTLab(en); } else if(!en->GetNeedCycleCheck()) { @@ -794,8 +795,6 @@ class EvaluableNodeManager #endif FreeNodeTreeWithCyclesRecurse(en); } - - ReclaimFreedNodesAtEnd(); } //attempts to free the node reference @@ -826,8 +825,6 @@ class EvaluableNodeManager FreeNodeTreeRecurse(e); } } - - ReclaimFreedNodesAtEnd(); } //retuns the nodes currently referenced, allocating if they don't exist @@ -896,24 +893,6 @@ class EvaluableNodeManager // and can improve reuse without calling the more expensive FreeAllNodesExceptReferencedNodes void CompactAllocatedNodes(); - //allows freed nodes at the end of nodes to be reallocated - inline void ReclaimFreedNodesAtEnd() - { - #ifndef MULTITHREAD_SUPPORT - //this cannot be used with multithreading because each thread will be using RecommendGarbageCollection - //to determine whether it should stay in garbage collection, and this can break the logic - //an alternative implementation would be to have a separate variable to indicate that everything should - //go into garbage collection, regardless of the current state of firstUnusedNodeIndex, but the extra - //overhead of that logic called for each opcode is not worth the gains of acquiring a write lock here - //and occasionally freeing a small bit of memory - - //if any group of nodes on the top are ready to be cleaned up cheaply, do so - while(firstUnusedNodeIndex > 0 && nodes[firstUnusedNodeIndex - 1] != nullptr - && nodes[firstUnusedNodeIndex - 1]->IsNodeDeallocated()) - firstUnusedNodeIndex--; - #endif - } - //returns the number of nodes currently being used that have not been freed yet __forceinline size_t GetNumberOfUsedNodes() { return firstUnusedNodeIndex; } @@ -1107,7 +1086,7 @@ class EvaluableNodeManager std::unique_ptr nodesCurrentlyReferenced; public: - // TODO: Make this private when done debugging + // TODO 22156: Make this private when done debugging typedef std::vector TLab; #if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) From bb85f4c82d7540d8e8fa28703f07e5d7af0666fa Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Thu, 21 Nov 2024 18:16:21 -0500 Subject: [PATCH 09/35] Implemented AllocListNodeWithOrderedChildNodes --- .../evaluablenode/EvaluableNodeManagement.cpp | 129 ++++++++++-------- 1 file changed, 74 insertions(+), 55 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 43e6e64f..ffb23e3e 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -69,63 +69,79 @@ EvaluableNode *EvaluableNodeManager::AllocNode(EvaluableNode *original, Evaluabl return n; } + +void InitializeListNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType childNodeType, int nodeIndex, std::vector *ocn_buffer) +{ + if(nodeIndex == 0) + { + // parent + node->InitializeType(ENT_LIST); + std::vector *ocn_ptr = &node->GetOrderedChildNodesReference(); + std::swap(*ocn_buffer, *ocn_ptr); + } + else + { + std::vector *ocn_ptr = &parent->GetOrderedChildNodesReference(); + (*ocn_ptr)[nodeIndex-1] = node; + node->InitializeType(childNodeType); + } +} + EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(EvaluableNodeType child_node_type, size_t num_child_nodes) { if(num_child_nodes == 0) return AllocNode(ENT_LIST); + EvaluableNode* parent = nullptr; + // Allocate from TLab + + size_t num_to_alloc = 1 + num_child_nodes; size_t num_allocated = 0; - size_t num_to_alloc = num_child_nodes + 1; size_t num_total_nodes_needed = 0; - EvaluableNode *retval = nullptr; - - //start off allocating the parent node, then switch to child_node_type - EvaluableNodeType cur_type = ENT_LIST; - //ordered child nodes destination; preallocate outside of the lock (for performance) and swap in - std::vector *ocn_ptr = nullptr; std::vector ocn_buffer; ocn_buffer.resize(num_child_nodes); - //outer loop needed for multithreading, but doesn't hurt anything for single threading + while(num_allocated < num_to_alloc) { + EvaluableNode* newNode = nullptr; + + while ((newNode = GetNextNodeFromTLab()) && num_allocated < num_to_alloc) { - #ifdef MULTITHREAD_SUPPORT - //attempt to allocate as many as possible using an atomic without write locking - Concurrency::ReadLock lock(managerAttributesMutex); - #endif + if(parent == nullptr) + { + parent = newNode; + } - for(; num_allocated < num_to_alloc; num_allocated++) + InitializeListNode(newNode, parent, child_node_type, num_allocated, &ocn_buffer); + num_allocated++; + } + + if (num_allocated >= num_to_alloc) + { + //we got enough nodes out of the tlab + return parent; + } + + + { + #ifdef MULTITHREAD_SUPPORT + Concurrency::ReadLock lock(managerAttributesMutex); + #endif + + for(int num_added_to_tlab = 0; num_allocated + num_added_to_tlab < num_to_alloc; num_added_to_tlab++) { - //attempt to allocate a node and make sure it's valid size_t allocated_index = firstUnusedNodeIndex++; if(allocated_index < nodes.size()) { - if(nodes[allocated_index] != nullptr) - nodes[allocated_index]->InitializeType(cur_type); - else - nodes[allocated_index] = new EvaluableNode(cur_type); - - //if first node, populate the parent node - if(num_allocated == 0) - { - //prep parent node - retval = nodes[allocated_index]; - - //get the pointer to place child elements, - // but swap out the preallocated ordered child nodes - ocn_ptr = &retval->GetOrderedChildNodesReference(); - std::swap(ocn_buffer, *ocn_ptr); - - //advance type to child node type - cur_type = child_node_type; - } - else //set the appropriate child node + if(nodes[allocated_index] == nullptr) { - (*ocn_ptr)[num_allocated - 1] = nodes[allocated_index]; + nodes[allocated_index] = new EvaluableNode(); } + + AddNodeToTLab(nodes[allocated_index]); } else { @@ -133,36 +149,39 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl --firstUnusedNodeIndex; break; } + } - - //if have allocated enough, just return - if(num_allocated == num_to_alloc) - return retval; - - num_total_nodes_needed = firstUnusedNodeIndex + (num_to_alloc - num_allocated); } - #ifdef MULTITHREAD_SUPPORT + if(num_allocated == num_to_alloc) + { + assert(parent); + return parent; + } - //don't have enough nodes, so need to attempt a write lock to allocate more - Concurrency::WriteLock write_lock(managerAttributesMutex); + { + #ifdef MULTITHREAD_SUPPORT + //don't have enough nodes, so need to attempt a write lock to allocate more + Concurrency::WriteLock write_lock(managerAttributesMutex); - //try again after write lock to allocate a node in case another thread has performed the allocation - //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex - #endif + //try again after write lock to allocate a node in case another thread has performed the allocation + //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex + #endif - //if don't currently have enough free nodes to meet the needs, then expand the allocation - if(nodes.size() <= num_total_nodes_needed) - { - size_t new_num_nodes = static_cast(allocExpansionFactor * num_total_nodes_needed) + 1; + //if don't currently have enough free nodes to meet the needs, then expand the allocation + if(nodes.size() <= num_total_nodes_needed) + { + size_t new_num_nodes = static_cast(allocExpansionFactor * num_total_nodes_needed) + 1; - //fill new EvaluableNode slots with nullptr - nodes.resize(new_num_nodes, nullptr); + //fill new EvaluableNode slots with nullptr + nodes.resize(new_num_nodes, nullptr); + } } } - //shouldn't make it here - return retval; + // unreachable + assert(false); + return nullptr; } void EvaluableNodeManager::UpdateGarbageCollectionTrigger(size_t previous_num_nodes) From d06458ce764720e3ac1ba9ad9e8d6b49834d02d1 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Fri, 22 Nov 2024 12:29:52 -0500 Subject: [PATCH 10/35] Added asserts, marked nodes as unallocated. --- src/Amalgam/evaluablenode/EvaluableNode.cpp | 1 + .../evaluablenode/EvaluableNodeManagement.cpp | 20 ++++++++++--------- .../evaluablenode/EvaluableNodeManagement.h | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNode.cpp b/src/Amalgam/evaluablenode/EvaluableNode.cpp index b3c5256f..c508bb85 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNode.cpp @@ -1646,6 +1646,7 @@ void EvaluableNode::Invalidate() break; case ENT_STRING: case ENT_SYMBOL: + std::cout << "!!!naricc_debug!!!: Invalidate: " << value.stringValueContainer.stringID << " label: " << value.stringValueContainer.labelStringID << std::endl; string_intern_pool.DestroyStringReferences(value.stringValueContainer.stringID, value.stringValueContainer.labelStringID); break; case ENT_ASSOC: diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index ffb23e3e..0430eec7 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -70,8 +70,9 @@ EvaluableNode *EvaluableNodeManager::AllocNode(EvaluableNode *original, Evaluabl } -void InitializeListNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType childNodeType, int nodeIndex, std::vector *ocn_buffer) +void InitializeListHeadOrNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType child_node_type, int nodeIndex, std::vector *ocn_buffer) { + if(nodeIndex == 0) { // parent @@ -81,9 +82,10 @@ void InitializeListNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNod } else { + // child node; initialize it and add it to the list items std::vector *ocn_ptr = &parent->GetOrderedChildNodesReference(); (*ocn_ptr)[nodeIndex-1] = node; - node->InitializeType(childNodeType); + node->InitializeType(child_node_type); } } @@ -115,18 +117,19 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl parent = newNode; } - InitializeListNode(newNode, parent, child_node_type, num_allocated, &ocn_buffer); + InitializeListHeadOrNode(newNode, parent, child_node_type, num_allocated, &ocn_buffer); num_allocated++; } if (num_allocated >= num_to_alloc) { - //we got enough nodes out of the tlab + //we got enough nodes out of the tlab; tryutn return parent; } - - { + + + { // Not enough nodes in TLab; add some. #ifdef MULTITHREAD_SUPPORT Concurrency::ReadLock lock(managerAttributesMutex); #endif @@ -138,7 +141,7 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl { if(nodes[allocated_index] == nullptr) { - nodes[allocated_index] = new EvaluableNode(); + nodes[allocated_index] = new EvaluableNode(ENT_DEALLOCATED); } AddNodeToTLab(nodes[allocated_index]); @@ -283,7 +286,6 @@ void EvaluableNodeManager::FreeAllNodes() EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() { EvaluableNode *tlab_node = GetNextNodeFromTLab(); - // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; //Fast Path; get node from thread local buffer if(tlab_node != nullptr) return tlab_node; @@ -483,7 +485,7 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree) tree->Invalidate(); - tree->InitializeType(ENT_NULL); + tree->InitializeType(ENT_DEALLOCATED); AddNodeToTLab(tree); } diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 3b49e197..4a214e15 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1136,7 +1136,7 @@ class EvaluableNodeManager inline void AddNodeToTLab(EvaluableNode *en) { - //std::cout << "!!!naricc_debug!!! Addeding evaluabelNode to Tlab: " << en << std::endl; + assert(en->IsNodeDeallocated()); if(this != lastEvaluableNodeManager) { From 72528d2ed20b1198d9644113ed4b20d522d15438 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Thu, 14 Nov 2024 13:22:19 -0500 Subject: [PATCH 11/35] Initial commit --- .vscode/settings.json | 73 ++++++++++++++++++- .../evaluablenode/EvaluableNodeManagement.cpp | 45 ++++++++---- .../evaluablenode/EvaluableNodeManagement.h | 35 +++++++++ src/Amalgam/interpreter/Interpreter.h | 3 + .../InterpreterOpcodesEntityAccess.cpp | 3 + .../InterpreterOpcodesEntityControl.cpp | 1 + 6 files changed, 146 insertions(+), 14 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 60d2427e..e73b83d0 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,4 +1,75 @@ { "cSpell.words": ["amlg", "cwrap", "KullbackLeibler"], - "files.trimTrailingWhitespace": false + "files.trimTrailingWhitespace": false, + "files.associations": { + "__bit_reference": "cpp", + "__hash_table": "cpp", + "__locale": "cpp", + "__node_handle": "cpp", + "__split_buffer": "cpp", + "__threading_support": "cpp", + "__tree": "cpp", + "__verbose_abort": "cpp", + "array": "cpp", + "bitset": "cpp", + "cctype": "cpp", + "cfenv": "cpp", + "charconv": "cpp", + "cinttypes": "cpp", + "clocale": "cpp", + "cmath": "cpp", + "codecvt": "cpp", + "complex": "cpp", + "condition_variable": "cpp", + "cstdarg": "cpp", + "cstddef": "cpp", + "cstdint": "cpp", + "cstdio": "cpp", + "cstdlib": "cpp", + "cstring": "cpp", + "ctime": "cpp", + "cwchar": "cpp", + "cwctype": "cpp", + "deque": "cpp", + "execution": "cpp", + "memory": "cpp", + "forward_list": "cpp", + "fstream": "cpp", + "future": "cpp", + "initializer_list": "cpp", + "iomanip": "cpp", + "ios": "cpp", + "iosfwd": "cpp", + "iostream": "cpp", + "istream": "cpp", + "limits": "cpp", + "list": "cpp", + "locale": "cpp", + "map": "cpp", + "mutex": "cpp", + "new": "cpp", + "numbers": "cpp", + "optional": "cpp", + "ostream": "cpp", + "print": "cpp", + "queue": "cpp", + "ratio": "cpp", + "regex": "cpp", + "set": "cpp", + "shared_mutex": "cpp", + "span": "cpp", + "sstream": "cpp", + "stack": "cpp", + "stdexcept": "cpp", + "streambuf": "cpp", + "string": "cpp", + "string_view": "cpp", + "tuple": "cpp", + "typeinfo": "cpp", + "unordered_map": "cpp", + "unordered_set": "cpp", + "variant": "cpp", + "vector": "cpp", + "algorithm": "cpp" + } } diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 81721a75..e3eb7d16 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #ifdef MULTITHREAD_SUPPORT Concurrency::ReadWriteMutex EvaluableNodeManager::memoryModificationMutex; @@ -186,6 +187,8 @@ void EvaluableNodeManager::CollectGarbage() #ifdef MULTITHREAD_SUPPORT + ClearThreadLocalAllocationBuffer(); + //free lock so can attempt to enter write lock to collect garbage if(memory_modification_lock != nullptr) memory_modification_lock->unlock(); @@ -250,24 +253,40 @@ void EvaluableNodeManager::FreeAllNodes() } EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() -{ - size_t allocated_index = 0; +{ #ifdef MULTITHREAD_SUPPORT { - //attempt to allocate using an atomic without write locking + EvaluableNode* tlabNode = getNextNodeFromTLab(); + + std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; + //Fast Path; get node from thread local buffer + if (tlabNode) { + assert(nodes.size() > 0); + return tlabNode; + } + + //slow path allocation; attempt to allocate using an atomic without write locking Concurrency::ReadLock lock(managerAttributesMutex); - //attempt to allocate a node and make sure it's valid - allocated_index = firstUnusedNodeIndex++; - if(allocated_index < nodes.size()) + //attempt to allocate enough nodes to refill thread local buffer + size_t first_index_to_allocate = firstUnusedNodeIndex.fetch_add(tlabSize); + size_t last_index_to_allocate = first_index_to_allocate + tlabSize; + + if(last_index_to_allocate < nodes.size()) { - if(nodes[allocated_index] == nullptr) - nodes[allocated_index] = new EvaluableNode(); + for (int i = first_index_to_allocate; i < last_index_to_allocate; i++) + { + if(nodes[i] == nullptr) + nodes[i] = new EvaluableNode(); + + threadLocalAllocationBuffer.push_back(nodes[i]); + } - return nodes[allocated_index]; + return getNextNodeFromTLab(); } - //the node wasn't valid; put it back and do a write lock to allocate more - --firstUnusedNodeIndex; + + //couldn't allocate enough valid nodes; reset index and allocate more + firstUnusedNodeIndex -= tlabSize; } //don't have enough nodes, so need to attempt a write lock to allocate more Concurrency::WriteLock write_lock(managerAttributesMutex); @@ -276,9 +295,9 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex //use the cached value for firstUnusedNodeIndex, allocated_index, to check if another thread has performed the allocation //as other threads may have reduced firstUnusedNodeIndex, incurring more unnecessary write locks when a memory expansion is needed -#else - allocated_index = firstUnusedNodeIndex; + #endif + size_t allocated_index = firstUnusedNodeIndex; size_t num_nodes = nodes.size(); if(allocated_index < num_nodes && firstUnusedNodeIndex < num_nodes) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 8ba1c950..96eec95a 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -6,6 +6,7 @@ //system headers: #include +#include //if the macro PEDANTIC_GARBAGE_COLLECTION is defined, then garbage collection will be performed //after every opcode, to help find and debug memory issues @@ -1090,6 +1091,16 @@ class EvaluableNodeManager // EvaluableNodeManager and so garbage collection should not happening while memory is being modified static Concurrency::ReadWriteMutex memoryModificationMutex; + + // TODO: Make this private when done debugging + typedef std::vector TLab; + inline static thread_local TLab threadLocalAllocationBuffer; + + static void ClearThreadLocalAllocationBuffer() + { + threadLocalAllocationBuffer.clear(); + } + protected: #else @@ -1107,4 +1118,28 @@ class EvaluableNodeManager //extra space to allocate when allocating static const double allocExpansionFactor; + +private: + + #ifdef MULTITHREAD_SUPPORT + static EvaluableNode* getNextNodeFromTLab() + { + if(threadLocalAllocationBuffer.size() > 0) + { + EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; + threadLocalAllocationBuffer.pop_back(); + return end; + } + else + { + return NULL; + } + + } + + static const int tlabSize = 20; + + #endif // MULTITHREAD_SUPPORT + + }; diff --git a/src/Amalgam/interpreter/Interpreter.h b/src/Amalgam/interpreter/Interpreter.h index 30ef8724..c74727c3 100644 --- a/src/Amalgam/interpreter/Interpreter.h +++ b/src/Amalgam/interpreter/Interpreter.h @@ -753,6 +753,7 @@ class Interpreter result = result_ref; resultsSaver.SetStackLocation(results_saver_location, result); + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -821,6 +822,7 @@ class Interpreter resultsSaver.SetStackLocation(results_saver_location, *result); } + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -831,6 +833,7 @@ class Interpreter inline void EndConcurrency() { //allow other threads to perform garbage collection + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); parentInterpreter->memoryModificationLock.unlock(); taskSet.WaitForTasks(taskEnqueueLock); parentInterpreter->memoryModificationLock.lock(); diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp index 123c1119..11f515e1 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp @@ -489,6 +489,8 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_ENTITY_and_CALL_ENTIT #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing + + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif @@ -608,6 +610,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_CONTAINER(EvaluableNo #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp index 8e12d235..5a4b73b0 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp @@ -664,6 +664,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_LOAD_ENTITY(EvaluableNode #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif From 21d70be5940144a3a2ecf18ee10d6e81d19d9f30 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Fri, 15 Nov 2024 09:57:14 -0500 Subject: [PATCH 12/35] commented out debug prints. --- src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 6 +++--- src/Amalgam/evaluablenode/EvaluableNodeManagement.h | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index e3eb7d16..88f09113 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -256,9 +256,9 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() { #ifdef MULTITHREAD_SUPPORT { - EvaluableNode* tlabNode = getNextNodeFromTLab(); + EvaluableNode* tlabNode = getNextNodeFromTLab(this); - std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; + // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; //Fast Path; get node from thread local buffer if (tlabNode) { assert(nodes.size() > 0); @@ -282,7 +282,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() threadLocalAllocationBuffer.push_back(nodes[i]); } - return getNextNodeFromTLab(); + return getNextNodeFromTLab(this); } //couldn't allocate enough valid nodes; reset index and allocate more diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 96eec95a..52c26f04 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1122,9 +1122,11 @@ class EvaluableNodeManager private: #ifdef MULTITHREAD_SUPPORT - static EvaluableNode* getNextNodeFromTLab() + + static inline thread_local EvaluableNodeManager* lastEvaluableNode; + static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNode) { - if(threadLocalAllocationBuffer.size() > 0) + if(threadLocalAllocationBuffer.size() > 0 && thisEvaluableNode == lastEvaluableNode) { EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; threadLocalAllocationBuffer.pop_back(); @@ -1132,6 +1134,10 @@ class EvaluableNodeManager } else { + if (lastEvaluableNode != thisEvaluableNode) + ClearThreadLocalAllocationBuffer(); + + lastEvaluableNode = thisEvaluableNode; return NULL; } From 7d1643f89cc418898c7030241c06c84d2e1fb5f8 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Fri, 15 Nov 2024 16:21:08 -0500 Subject: [PATCH 13/35] Initilize tlab nodes to state ENT_NULL --- src/Amalgam/evaluablenode/EvaluableNode.h | 2 ++ src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNode.h b/src/Amalgam/evaluablenode/EvaluableNode.h index 850f6031..2824f26a 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.h +++ b/src/Amalgam/evaluablenode/EvaluableNode.h @@ -12,6 +12,8 @@ #include #include +#define AMALGAM_MEMORY_INTEGRITY + //if the macro AMALGAM_MEMORY_INTEGRITY is defined, then it will continuously verify memory, at a high cost of performance //this is useful for diagnosing and debugging memory issues //if the macro AMALGAM_FAST_MEMORY_INTEGRITY is defined, then only the checks that are fast will be made diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 88f09113..e650c778 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -8,6 +8,9 @@ #include #include + +#define PEDANTIC_GARBAGE_COLLECTION + #ifdef MULTITHREAD_SUPPORT Concurrency::ReadWriteMutex EvaluableNodeManager::memoryModificationMutex; #endif @@ -277,7 +280,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() for (int i = first_index_to_allocate; i < last_index_to_allocate; i++) { if(nodes[i] == nullptr) - nodes[i] = new EvaluableNode(); + nodes[i] = new EvaluableNode(ENT_NULL); threadLocalAllocationBuffer.push_back(nodes[i]); } @@ -287,6 +290,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() //couldn't allocate enough valid nodes; reset index and allocate more firstUnusedNodeIndex -= tlabSize; + ClearThreadLocalAllocationBuffer(); } //don't have enough nodes, so need to attempt a write lock to allocate more Concurrency::WriteLock write_lock(managerAttributesMutex); From 55f90434c10757e2c54ca2be6fbb1726e9fcceb8 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 18 Nov 2024 16:22:56 -0500 Subject: [PATCH 14/35] Put deallocated nodes back into tlab. --- src/Amalgam/evaluablenode/EvaluableNode.h | 4 ++-- .../evaluablenode/EvaluableNodeManagement.cpp | 16 +++++++++++-- .../evaluablenode/EvaluableNodeManagement.h | 24 +++++++++++++++---- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNode.h b/src/Amalgam/evaluablenode/EvaluableNode.h index 2824f26a..7f78178d 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.h +++ b/src/Amalgam/evaluablenode/EvaluableNode.h @@ -12,8 +12,6 @@ #include #include -#define AMALGAM_MEMORY_INTEGRITY - //if the macro AMALGAM_MEMORY_INTEGRITY is defined, then it will continuously verify memory, at a high cost of performance //this is useful for diagnosing and debugging memory issues //if the macro AMALGAM_FAST_MEMORY_INTEGRITY is defined, then only the checks that are fast will be made @@ -21,6 +19,8 @@ #define AMALGAM_FAST_MEMORY_INTEGRITY #endif +#define AMALGAM_FAST_MEMORY_INTEGRITY + //forward declarations: class EvaluableNodeManager; diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index e650c778..bd1a7c69 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -9,7 +9,7 @@ #include -#define PEDANTIC_GARBAGE_COLLECTION +// #define PEDANTIC_GARBAGE_COLLECTION #ifdef MULTITHREAD_SUPPORT Concurrency::ReadWriteMutex EvaluableNodeManager::memoryModificationMutex; @@ -282,7 +282,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() if(nodes[i] == nullptr) nodes[i] = new EvaluableNode(ENT_NULL); - threadLocalAllocationBuffer.push_back(nodes[i]); + addToTLab(this, nodes[i]); } return getNextNodeFromTLab(this); @@ -458,6 +458,11 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree) } tree->Invalidate(); + +#ifdef MULTITHREAD_SUPPORT + tree->InitializeType(ENT_NULL); + addToTLab(this, tree); +#endif } void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) @@ -497,12 +502,19 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) std::swap(ocn, tree_ocn); tree->Invalidate(); + + for(auto &e : ocn) { if(e != nullptr && !e->IsNodeDeallocated()) FreeNodeTreeWithCyclesRecurse(e); } } + + #ifdef MULTITHREAD_SUPPORT + tree->InitializeType(ENT_NULL); + addToTLab(this, tree); + #endif } void EvaluableNodeManager::ModifyLabels(EvaluableNode *n, EvaluableNodeMetadataModifier metadata_modifier) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 52c26f04..0ac2e939 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1101,6 +1101,7 @@ class EvaluableNodeManager threadLocalAllocationBuffer.clear(); } + protected: #else @@ -1123,10 +1124,10 @@ class EvaluableNodeManager #ifdef MULTITHREAD_SUPPORT - static inline thread_local EvaluableNodeManager* lastEvaluableNode; - static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNode) + static inline thread_local EvaluableNodeManager* lastEvaluableNodeManager; + static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNodeManager) { - if(threadLocalAllocationBuffer.size() > 0 && thisEvaluableNode == lastEvaluableNode) + if(threadLocalAllocationBuffer.size() > 0 && thisEvaluableNodeManager == lastEvaluableNodeManager) { EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; threadLocalAllocationBuffer.pop_back(); @@ -1134,15 +1135,28 @@ class EvaluableNodeManager } else { - if (lastEvaluableNode != thisEvaluableNode) + if (lastEvaluableNodeManager != thisEvaluableNodeManager) ClearThreadLocalAllocationBuffer(); - lastEvaluableNode = thisEvaluableNode; + lastEvaluableNodeManager = thisEvaluableNodeManager; return NULL; } } + static void addToTLab(EvaluableNodeManager* thisEvaulableNodeManager, EvaluableNode* evaluableNode) + { + std::cout << "!!!naricc_debug!!! Addeding evaluabelNode to Tlab: " << evaluableNode << std::endl; + + if(thisEvaulableNodeManager != lastEvaluableNodeManager) + { + threadLocalAllocationBuffer.clear(); + lastEvaluableNodeManager = thisEvaulableNodeManager; + } + + threadLocalAllocationBuffer.push_back(evaluableNode); + } + static const int tlabSize = 20; #endif // MULTITHREAD_SUPPORT From 5ff973f97a34082ca4d5de322395f8b59ca633d6 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 18 Nov 2024 21:20:08 -0500 Subject: [PATCH 15/35] Moving around some code. --- .../evaluablenode/EvaluableNodeManagement.cpp | 26 +++++++++---------- .../evaluablenode/EvaluableNodeManagement.h | 13 +++------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index bd1a7c69..e1d88a50 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -257,16 +257,16 @@ void EvaluableNodeManager::FreeAllNodes() EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() { + EvaluableNode* tlabNode = getNextNodeFromTLab(this); + // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; + //Fast Path; get node from thread local buffer + if (tlabNode) { + assert(nodes.size() > 0); + return tlabNode; + } + #ifdef MULTITHREAD_SUPPORT { - EvaluableNode* tlabNode = getNextNodeFromTLab(this); - - // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; - //Fast Path; get node from thread local buffer - if (tlabNode) { - assert(nodes.size() > 0); - return tlabNode; - } //slow path allocation; attempt to allocate using an atomic without write locking Concurrency::ReadLock lock(managerAttributesMutex); @@ -459,10 +459,8 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree) tree->Invalidate(); -#ifdef MULTITHREAD_SUPPORT tree->InitializeType(ENT_NULL); addToTLab(this, tree); -#endif } void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) @@ -492,6 +490,9 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) else if(tree->IsImmediate()) { tree->Invalidate(); + + tree->InitializeType(ENT_NULL); + addToTLab(this, tree); } else //ordered { @@ -511,10 +512,7 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) } } - #ifdef MULTITHREAD_SUPPORT - tree->InitializeType(ENT_NULL); - addToTLab(this, tree); - #endif + } void EvaluableNodeManager::ModifyLabels(EvaluableNode *n, EvaluableNodeMetadataModifier metadata_modifier) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 0ac2e939..ab7ea3ba 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1080,6 +1080,10 @@ class EvaluableNodeManager Concurrency::ReadWriteMutex managerAttributesMutex; std::atomic firstUnusedNodeIndex; +#else + size_t firstUnusedNodeIndex; +#endif + public: //global mutex to manage whether memory nodes are being modified @@ -1104,10 +1108,6 @@ class EvaluableNodeManager protected: -#else - size_t firstUnusedNodeIndex; -#endif - //nodes that have been allocated and may be in use // all nodes in use are below firstUnusedNodeIndex, such that all above that index are free for use // nodes cannot be nullptr for lower indices than firstUnusedNodeIndex @@ -1122,8 +1122,6 @@ class EvaluableNodeManager private: - #ifdef MULTITHREAD_SUPPORT - static inline thread_local EvaluableNodeManager* lastEvaluableNodeManager; static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNodeManager) { @@ -1159,7 +1157,4 @@ class EvaluableNodeManager static const int tlabSize = 20; - #endif // MULTITHREAD_SUPPORT - - }; From c8fd7eb85f4a662c7841b8da9dbf4926295979ca Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Tue, 19 Nov 2024 08:36:15 -0500 Subject: [PATCH 16/35] 22156: Cleanup, progress, identified issue --- src/Amalgam/Parser.cpp | 4 +- src/Amalgam/evaluablenode/EvaluableNode.h | 13 +++- .../evaluablenode/EvaluableNodeManagement.cpp | 37 +++++----- .../evaluablenode/EvaluableNodeManagement.h | 67 +++++++++++-------- src/Amalgam/interpreter/Interpreter.h | 3 - .../InterpreterOpcodesEntityAccess.cpp | 3 - .../InterpreterOpcodesEntityControl.cpp | 1 - 7 files changed, 73 insertions(+), 55 deletions(-) diff --git a/src/Amalgam/Parser.cpp b/src/Amalgam/Parser.cpp index 31f57ac6..2f27d0aa 100644 --- a/src/Amalgam/Parser.cpp +++ b/src/Amalgam/Parser.cpp @@ -199,6 +199,8 @@ EvaluableNode *Parser::GetCodeForPathToSharedNodeFromParentAToParentB(UnparseDat if(shared_node == nullptr || a_parent == nullptr || b_parent == nullptr) return nullptr; + //TODO 22156: allocate from enm in a way that does not invalidate tlab (but does not increase the size of EvaluableNodeManager) + //find all parent nodes of a to find collision with parent node of b, along with depth counts EvaluableNode::ReferenceCountType a_parent_nodes; size_t a_ancestor_depth = 1; @@ -991,7 +993,7 @@ void Parser::Unparse(UnparseData &upd, EvaluableNode *tree, EvaluableNode *paren std::swap(upd.parentNodes, references); Unparse(upd, code_to_print, nullptr, expanded_whitespace, indentation_depth, need_initial_indent); std::swap(upd.parentNodes, references); //put the old parentNodes back - enm.FreeNodeTree(code_to_print); + //don't free code_to_print, but let enm's destructor clean it up return; } diff --git a/src/Amalgam/evaluablenode/EvaluableNode.h b/src/Amalgam/evaluablenode/EvaluableNode.h index 7f78178d..16bf8052 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.h +++ b/src/Amalgam/evaluablenode/EvaluableNode.h @@ -147,7 +147,7 @@ class EvaluableNode inline void InitializeType(EvaluableNodeType _type) { #ifdef AMALGAM_FAST_MEMORY_INTEGRITY - assert(IsEvaluableNodeTypeValid(_type)); + assert(IsEvaluableNodeTypeValid(_type) || _type == ENT_DEALLOCATED); #endif type = _type; @@ -173,6 +173,17 @@ class EvaluableNode attributes.individualAttribs.isIdempotent = true; value.ConstructMappedChildNodes(); } + else if(_type == ENT_DEALLOCATED) + { + #ifdef AMALGAM_FAST_MEMORY_INTEGRITY + //use a value that is more apparent that something went wrong + value.numberValueContainer.numberValue = std::numeric_limits::quiet_NaN(); + #else + value.numberValueContainer.numberValue = 0; + #endif + + value.numberValueContainer.labelStringID = StringInternPool::NOT_A_STRING_ID; + } else { value.ConstructOrderedChildNodes(); diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index e1d88a50..146a0e2f 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -257,13 +257,11 @@ void EvaluableNodeManager::FreeAllNodes() EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() { - EvaluableNode* tlabNode = getNextNodeFromTLab(this); + EvaluableNode *tlab_node = GetNextNodeFromTLab(); // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; //Fast Path; get node from thread local buffer - if (tlabNode) { - assert(nodes.size() > 0); - return tlabNode; - } + if(tlab_node != nullptr) + return tlab_node; #ifdef MULTITHREAD_SUPPORT { @@ -277,15 +275,15 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() if(last_index_to_allocate < nodes.size()) { - for (int i = first_index_to_allocate; i < last_index_to_allocate; i++) + for(size_t i = first_index_to_allocate; i < last_index_to_allocate; i++) { if(nodes[i] == nullptr) - nodes[i] = new EvaluableNode(ENT_NULL); + nodes[i] = new EvaluableNode(ENT_DEALLOCATED); - addToTLab(this, nodes[i]); + AddNodeToTLab(nodes[i]); } - return getNextNodeFromTLab(this); + return GetNextNodeFromTLab(); } //couldn't allocate enough valid nodes; reset index and allocate more @@ -301,15 +299,16 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() //as other threads may have reduced firstUnusedNodeIndex, incurring more unnecessary write locks when a memory expansion is needed #endif - size_t allocated_index = firstUnusedNodeIndex; + //reduce accesses to the atomic variable for performance + size_t allocated_index = firstUnusedNodeIndex++; size_t num_nodes = nodes.size(); - if(allocated_index < num_nodes && firstUnusedNodeIndex < num_nodes) + if(allocated_index < num_nodes) { - if(nodes[firstUnusedNodeIndex] == nullptr) - nodes[firstUnusedNodeIndex] = new EvaluableNode(); + if(nodes[allocated_index] == nullptr) + nodes[allocated_index] = new EvaluableNode(); - return nodes[firstUnusedNodeIndex++]; + return nodes[allocated_index]; } //ran out, so need another node; push a bunch on the heap so don't need to reallocate as often and slow down garbage collection @@ -318,10 +317,10 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() //fill new EvaluableNode slots with nullptr nodes.resize(new_num_nodes, nullptr); - if(nodes[firstUnusedNodeIndex] == nullptr) - nodes[firstUnusedNodeIndex] = new EvaluableNode(); + if(nodes[allocated_index] == nullptr) + nodes[allocated_index] = new EvaluableNode(); - return nodes[firstUnusedNodeIndex++]; + return nodes[allocated_index]; } void EvaluableNodeManager::FreeAllNodesExceptReferencedNodes(size_t cur_first_unused_node_index) @@ -460,7 +459,7 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree) tree->Invalidate(); tree->InitializeType(ENT_NULL); - addToTLab(this, tree); + AddNodeToTLab(tree); } void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) @@ -492,7 +491,7 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) tree->Invalidate(); tree->InitializeType(ENT_NULL); - addToTLab(this, tree); + AddNodeToTLab(tree); } else //ordered { diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index ab7ea3ba..57746114 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1080,10 +1080,6 @@ class EvaluableNodeManager Concurrency::ReadWriteMutex managerAttributesMutex; std::atomic firstUnusedNodeIndex; -#else - size_t firstUnusedNodeIndex; -#endif - public: //global mutex to manage whether memory nodes are being modified @@ -1095,19 +1091,12 @@ class EvaluableNodeManager // EvaluableNodeManager and so garbage collection should not happening while memory is being modified static Concurrency::ReadWriteMutex memoryModificationMutex; - - // TODO: Make this private when done debugging - typedef std::vector TLab; - inline static thread_local TLab threadLocalAllocationBuffer; - - static void ClearThreadLocalAllocationBuffer() - { - threadLocalAllocationBuffer.clear(); - } - - protected: +#else + size_t firstUnusedNodeIndex; +#endif + //nodes that have been allocated and may be in use // all nodes in use are below firstUnusedNodeIndex, such that all above that index are free for use // nodes cannot be nullptr for lower indices than firstUnusedNodeIndex @@ -1117,15 +1106,39 @@ class EvaluableNodeManager //only allocated if needed std::unique_ptr nodesCurrentlyReferenced; +public: + // TODO: Make this private when done debugging + typedef std::vector TLab; + +#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) + thread_local +#endif + inline static TLab threadLocalAllocationBuffer; + + static void ClearThreadLocalAllocationBuffer() + { + threadLocalAllocationBuffer.clear(); + } +protected: + + //buffer used for updating EvaluableNodeFlags, particularly UpdateFlagsForNodeTree +#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) + thread_local +#endif + static EvaluableNode::ReferenceAssocType nodeToParentNodeCache; + + //extra space to allocate when allocating static const double allocExpansionFactor; -private: +#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) + thread_local +#endif + static inline EvaluableNodeManager *lastEvaluableNodeManager; - static inline thread_local EvaluableNodeManager* lastEvaluableNodeManager; - static EvaluableNode* getNextNodeFromTLab(EvaluableNodeManager* thisEvaluableNodeManager) + inline EvaluableNode *GetNextNodeFromTLab() { - if(threadLocalAllocationBuffer.size() > 0 && thisEvaluableNodeManager == lastEvaluableNodeManager) + if(threadLocalAllocationBuffer.size() > 0 && this == lastEvaluableNodeManager) { EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; threadLocalAllocationBuffer.pop_back(); @@ -1133,26 +1146,26 @@ class EvaluableNodeManager } else { - if (lastEvaluableNodeManager != thisEvaluableNodeManager) + if(lastEvaluableNodeManager != this) ClearThreadLocalAllocationBuffer(); - lastEvaluableNodeManager = thisEvaluableNodeManager; - return NULL; + lastEvaluableNodeManager = this; + return nullptr; } } - static void addToTLab(EvaluableNodeManager* thisEvaulableNodeManager, EvaluableNode* evaluableNode) + inline void AddNodeToTLab(EvaluableNode *en) { - std::cout << "!!!naricc_debug!!! Addeding evaluabelNode to Tlab: " << evaluableNode << std::endl; + //std::cout << "!!!naricc_debug!!! Addeding evaluabelNode to Tlab: " << en << std::endl; - if(thisEvaulableNodeManager != lastEvaluableNodeManager) + if(this != lastEvaluableNodeManager) { threadLocalAllocationBuffer.clear(); - lastEvaluableNodeManager = thisEvaulableNodeManager; + lastEvaluableNodeManager = this; } - threadLocalAllocationBuffer.push_back(evaluableNode); + threadLocalAllocationBuffer.push_back(en); } static const int tlabSize = 20; diff --git a/src/Amalgam/interpreter/Interpreter.h b/src/Amalgam/interpreter/Interpreter.h index c74727c3..30ef8724 100644 --- a/src/Amalgam/interpreter/Interpreter.h +++ b/src/Amalgam/interpreter/Interpreter.h @@ -753,7 +753,6 @@ class Interpreter result = result_ref; resultsSaver.SetStackLocation(results_saver_location, result); - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -822,7 +821,6 @@ class Interpreter resultsSaver.SetStackLocation(results_saver_location, *result); } - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -833,7 +831,6 @@ class Interpreter inline void EndConcurrency() { //allow other threads to perform garbage collection - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); parentInterpreter->memoryModificationLock.unlock(); taskSet.WaitForTasks(taskEnqueueLock); parentInterpreter->memoryModificationLock.lock(); diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp index 11f515e1..123c1119 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp @@ -489,8 +489,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_ENTITY_and_CALL_ENTIT #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing - - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif @@ -610,7 +608,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_CONTAINER(EvaluableNo #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp index 5a4b73b0..8e12d235 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp @@ -664,7 +664,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_LOAD_ENTITY(EvaluableNode #ifdef MULTITHREAD_SUPPORT //this interpreter is no longer executing - EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); memoryModificationLock.unlock(); #endif From 5f24c659cb49357a2303767b4528bbe4b0b12af9 Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:15:23 -0500 Subject: [PATCH 17/35] 22156: Re-adds tlab clears in concurrency --- src/Amalgam/interpreter/Interpreter.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Amalgam/interpreter/Interpreter.h b/src/Amalgam/interpreter/Interpreter.h index 30ef8724..c74727c3 100644 --- a/src/Amalgam/interpreter/Interpreter.h +++ b/src/Amalgam/interpreter/Interpreter.h @@ -753,6 +753,7 @@ class Interpreter result = result_ref; resultsSaver.SetStackLocation(results_saver_location, result); + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -821,6 +822,7 @@ class Interpreter resultsSaver.SetStackLocation(results_saver_location, *result); } + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); interpreter.memoryModificationLock.unlock(); taskSet.MarkTaskCompleted(); } @@ -831,6 +833,7 @@ class Interpreter inline void EndConcurrency() { //allow other threads to perform garbage collection + EvaluableNodeManager::ClearThreadLocalAllocationBuffer(); parentInterpreter->memoryModificationLock.unlock(); taskSet.WaitForTasks(taskEnqueueLock); parentInterpreter->memoryModificationLock.lock(); From 86aa6a3fb79dc399ae600fcc32245ce6e5fba922 Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:31:50 -0500 Subject: [PATCH 18/35] 22156: Collects more to tlab from frees --- .../evaluablenode/EvaluableNodeManagement.cpp | 8 ++--- .../evaluablenode/EvaluableNodeManagement.h | 29 +++---------------- 2 files changed, 6 insertions(+), 31 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 146a0e2f..a332d4f0 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -476,6 +476,7 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) auto &tree_mcn = tree->GetMappedChildNodesReference(); std::swap(mcn, tree_mcn); tree->Invalidate(); + AddNodeToTLab(tree); for(auto &[_, e] : mcn) { @@ -489,8 +490,6 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) else if(tree->IsImmediate()) { tree->Invalidate(); - - tree->InitializeType(ENT_NULL); AddNodeToTLab(tree); } else //ordered @@ -501,8 +500,7 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) auto &tree_ocn = tree->GetOrderedChildNodesReference(); std::swap(ocn, tree_ocn); tree->Invalidate(); - - + AddNodeToTLab(tree); for(auto &e : ocn) { @@ -510,8 +508,6 @@ void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree) FreeNodeTreeWithCyclesRecurse(e); } } - - } void EvaluableNodeManager::ModifyLabels(EvaluableNode *n, EvaluableNodeMetadataModifier metadata_modifier) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 57746114..0ba9f34b 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -743,7 +743,7 @@ class EvaluableNodeManager #endif en->Invalidate(); - ReclaimFreedNodesAtEnd(); + AddNodeToTLab(en); } //attempts to free the node reference @@ -759,7 +759,7 @@ class EvaluableNodeManager if(enr.unique && enr != nullptr && !enr->GetNeedCycleCheck()) { enr->Invalidate(); - ReclaimFreedNodesAtEnd(); + AddNodeToTLab(enr); } } @@ -779,6 +779,7 @@ class EvaluableNodeManager if(IsEvaluableNodeTypeImmediate(en->GetType())) { en->Invalidate(); + AddNodeToTLab(en); } else if(!en->GetNeedCycleCheck()) { @@ -794,8 +795,6 @@ class EvaluableNodeManager #endif FreeNodeTreeWithCyclesRecurse(en); } - - ReclaimFreedNodesAtEnd(); } //attempts to free the node reference @@ -826,8 +825,6 @@ class EvaluableNodeManager FreeNodeTreeRecurse(e); } } - - ReclaimFreedNodesAtEnd(); } //retuns the nodes currently referenced, allocating if they don't exist @@ -896,24 +893,6 @@ class EvaluableNodeManager // and can improve reuse without calling the more expensive FreeAllNodesExceptReferencedNodes void CompactAllocatedNodes(); - //allows freed nodes at the end of nodes to be reallocated - inline void ReclaimFreedNodesAtEnd() - { - #ifndef MULTITHREAD_SUPPORT - //this cannot be used with multithreading because each thread will be using RecommendGarbageCollection - //to determine whether it should stay in garbage collection, and this can break the logic - //an alternative implementation would be to have a separate variable to indicate that everything should - //go into garbage collection, regardless of the current state of firstUnusedNodeIndex, but the extra - //overhead of that logic called for each opcode is not worth the gains of acquiring a write lock here - //and occasionally freeing a small bit of memory - - //if any group of nodes on the top are ready to be cleaned up cheaply, do so - while(firstUnusedNodeIndex > 0 && nodes[firstUnusedNodeIndex - 1] != nullptr - && nodes[firstUnusedNodeIndex - 1]->IsNodeDeallocated()) - firstUnusedNodeIndex--; - #endif - } - //returns the number of nodes currently being used that have not been freed yet __forceinline size_t GetNumberOfUsedNodes() { return firstUnusedNodeIndex; } @@ -1107,7 +1086,7 @@ class EvaluableNodeManager std::unique_ptr nodesCurrentlyReferenced; public: - // TODO: Make this private when done debugging + // TODO 22156: Make this private when done debugging typedef std::vector TLab; #if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) From e54dd4d8b37a3ff7a892c891e4dea574f004a70e Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Thu, 21 Nov 2024 18:16:21 -0500 Subject: [PATCH 19/35] Implemented AllocListNodeWithOrderedChildNodes --- .../evaluablenode/EvaluableNodeManagement.cpp | 129 ++++++++++-------- 1 file changed, 74 insertions(+), 55 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index a332d4f0..1b12367c 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -63,63 +63,79 @@ EvaluableNode *EvaluableNodeManager::AllocNode(EvaluableNode *original, Evaluabl return n; } + +void InitializeListNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType childNodeType, int nodeIndex, std::vector *ocn_buffer) +{ + if(nodeIndex == 0) + { + // parent + node->InitializeType(ENT_LIST); + std::vector *ocn_ptr = &node->GetOrderedChildNodesReference(); + std::swap(*ocn_buffer, *ocn_ptr); + } + else + { + std::vector *ocn_ptr = &parent->GetOrderedChildNodesReference(); + (*ocn_ptr)[nodeIndex-1] = node; + node->InitializeType(childNodeType); + } +} + EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(EvaluableNodeType child_node_type, size_t num_child_nodes) { if(num_child_nodes == 0) return AllocNode(ENT_LIST); + EvaluableNode* parent = nullptr; + // Allocate from TLab + + size_t num_to_alloc = 1 + num_child_nodes; size_t num_allocated = 0; - size_t num_to_alloc = num_child_nodes + 1; size_t num_total_nodes_needed = 0; - EvaluableNode *retval = nullptr; - - //start off allocating the parent node, then switch to child_node_type - EvaluableNodeType cur_type = ENT_LIST; - //ordered child nodes destination; preallocate outside of the lock (for performance) and swap in - std::vector *ocn_ptr = nullptr; std::vector ocn_buffer; ocn_buffer.resize(num_child_nodes); - //outer loop needed for multithreading, but doesn't hurt anything for single threading + while(num_allocated < num_to_alloc) { + EvaluableNode* newNode = nullptr; + + while ((newNode = GetNextNodeFromTLab()) && num_allocated < num_to_alloc) { - #ifdef MULTITHREAD_SUPPORT - //attempt to allocate as many as possible using an atomic without write locking - Concurrency::ReadLock lock(managerAttributesMutex); - #endif + if(parent == nullptr) + { + parent = newNode; + } - for(; num_allocated < num_to_alloc; num_allocated++) + InitializeListNode(newNode, parent, child_node_type, num_allocated, &ocn_buffer); + num_allocated++; + } + + if (num_allocated >= num_to_alloc) + { + //we got enough nodes out of the tlab + return parent; + } + + + { + #ifdef MULTITHREAD_SUPPORT + Concurrency::ReadLock lock(managerAttributesMutex); + #endif + + for(int num_added_to_tlab = 0; num_allocated + num_added_to_tlab < num_to_alloc; num_added_to_tlab++) { - //attempt to allocate a node and make sure it's valid size_t allocated_index = firstUnusedNodeIndex++; if(allocated_index < nodes.size()) { - if(nodes[allocated_index] != nullptr) - nodes[allocated_index]->InitializeType(cur_type); - else - nodes[allocated_index] = new EvaluableNode(cur_type); - - //if first node, populate the parent node - if(num_allocated == 0) - { - //prep parent node - retval = nodes[allocated_index]; - - //get the pointer to place child elements, - // but swap out the preallocated ordered child nodes - ocn_ptr = &retval->GetOrderedChildNodesReference(); - std::swap(ocn_buffer, *ocn_ptr); - - //advance type to child node type - cur_type = child_node_type; - } - else //set the appropriate child node + if(nodes[allocated_index] == nullptr) { - (*ocn_ptr)[num_allocated - 1] = nodes[allocated_index]; + nodes[allocated_index] = new EvaluableNode(); } + + AddNodeToTLab(nodes[allocated_index]); } else { @@ -127,36 +143,39 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl --firstUnusedNodeIndex; break; } + } - - //if have allocated enough, just return - if(num_allocated == num_to_alloc) - return retval; - - num_total_nodes_needed = firstUnusedNodeIndex + (num_to_alloc - num_allocated); } - #ifdef MULTITHREAD_SUPPORT + if(num_allocated == num_to_alloc) + { + assert(parent); + return parent; + } - //don't have enough nodes, so need to attempt a write lock to allocate more - Concurrency::WriteLock write_lock(managerAttributesMutex); + { + #ifdef MULTITHREAD_SUPPORT + //don't have enough nodes, so need to attempt a write lock to allocate more + Concurrency::WriteLock write_lock(managerAttributesMutex); - //try again after write lock to allocate a node in case another thread has performed the allocation - //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex - #endif + //try again after write lock to allocate a node in case another thread has performed the allocation + //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex + #endif - //if don't currently have enough free nodes to meet the needs, then expand the allocation - if(nodes.size() <= num_total_nodes_needed) - { - size_t new_num_nodes = static_cast(allocExpansionFactor * num_total_nodes_needed) + 1; + //if don't currently have enough free nodes to meet the needs, then expand the allocation + if(nodes.size() <= num_total_nodes_needed) + { + size_t new_num_nodes = static_cast(allocExpansionFactor * num_total_nodes_needed) + 1; - //fill new EvaluableNode slots with nullptr - nodes.resize(new_num_nodes, nullptr); + //fill new EvaluableNode slots with nullptr + nodes.resize(new_num_nodes, nullptr); + } } } - //shouldn't make it here - return retval; + // unreachable + assert(false); + return nullptr; } void EvaluableNodeManager::UpdateGarbageCollectionTrigger(size_t previous_num_nodes) From c7222755dad8eaf638a5d16c29e9ab0ef1859e63 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Fri, 22 Nov 2024 12:29:52 -0500 Subject: [PATCH 20/35] Added asserts, marked nodes as unallocated. --- src/Amalgam/evaluablenode/EvaluableNode.cpp | 1 + .../evaluablenode/EvaluableNodeManagement.cpp | 20 ++++++++++--------- .../evaluablenode/EvaluableNodeManagement.h | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNode.cpp b/src/Amalgam/evaluablenode/EvaluableNode.cpp index b3c5256f..c508bb85 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNode.cpp @@ -1646,6 +1646,7 @@ void EvaluableNode::Invalidate() break; case ENT_STRING: case ENT_SYMBOL: + std::cout << "!!!naricc_debug!!!: Invalidate: " << value.stringValueContainer.stringID << " label: " << value.stringValueContainer.labelStringID << std::endl; string_intern_pool.DestroyStringReferences(value.stringValueContainer.stringID, value.stringValueContainer.labelStringID); break; case ENT_ASSOC: diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 1b12367c..ed2f80f7 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -64,8 +64,9 @@ EvaluableNode *EvaluableNodeManager::AllocNode(EvaluableNode *original, Evaluabl } -void InitializeListNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType childNodeType, int nodeIndex, std::vector *ocn_buffer) +void InitializeListHeadOrNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType child_node_type, int nodeIndex, std::vector *ocn_buffer) { + if(nodeIndex == 0) { // parent @@ -75,9 +76,10 @@ void InitializeListNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNod } else { + // child node; initialize it and add it to the list items std::vector *ocn_ptr = &parent->GetOrderedChildNodesReference(); (*ocn_ptr)[nodeIndex-1] = node; - node->InitializeType(childNodeType); + node->InitializeType(child_node_type); } } @@ -109,18 +111,19 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl parent = newNode; } - InitializeListNode(newNode, parent, child_node_type, num_allocated, &ocn_buffer); + InitializeListHeadOrNode(newNode, parent, child_node_type, num_allocated, &ocn_buffer); num_allocated++; } if (num_allocated >= num_to_alloc) { - //we got enough nodes out of the tlab + //we got enough nodes out of the tlab; tryutn return parent; } - - { + + + { // Not enough nodes in TLab; add some. #ifdef MULTITHREAD_SUPPORT Concurrency::ReadLock lock(managerAttributesMutex); #endif @@ -132,7 +135,7 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl { if(nodes[allocated_index] == nullptr) { - nodes[allocated_index] = new EvaluableNode(); + nodes[allocated_index] = new EvaluableNode(ENT_DEALLOCATED); } AddNodeToTLab(nodes[allocated_index]); @@ -277,7 +280,6 @@ void EvaluableNodeManager::FreeAllNodes() EvaluableNode *EvaluableNodeManager::AllocUninitializedNode() { EvaluableNode *tlab_node = GetNextNodeFromTLab(); - // std::cout << "!!!naricc_debug!!! EvaluableNodeManager::AllocUninitializedNode EvaluableNodeManager: " << this << " thread_id: " << std::this_thread::get_id() << " tlab: " << &threadLocalAllocationBuffer << std::endl; //Fast Path; get node from thread local buffer if(tlab_node != nullptr) return tlab_node; @@ -477,7 +479,7 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree) tree->Invalidate(); - tree->InitializeType(ENT_NULL); + tree->InitializeType(ENT_DEALLOCATED); AddNodeToTLab(tree); } diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 0ba9f34b..f97391cf 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1136,7 +1136,7 @@ class EvaluableNodeManager inline void AddNodeToTLab(EvaluableNode *en) { - //std::cout << "!!!naricc_debug!!! Addeding evaluabelNode to Tlab: " << en << std::endl; + assert(en->IsNodeDeallocated()); if(this != lastEvaluableNodeManager) { From ee0e02ada9c056d5655732cce611eb122ce5a89e Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Fri, 22 Nov 2024 13:56:55 -0500 Subject: [PATCH 21/35] 22156: Fixes to compilation --- src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 0430eec7..91fb0f36 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -70,10 +70,9 @@ EvaluableNode *EvaluableNodeManager::AllocNode(EvaluableNode *original, Evaluabl } -void InitializeListHeadOrNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType child_node_type, int nodeIndex, std::vector *ocn_buffer) +void InitializeListHeadOrNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType child_node_type, size_t node_index, std::vector *ocn_buffer) { - - if(nodeIndex == 0) + if(node_index == 0) { // parent node->InitializeType(ENT_LIST); @@ -84,7 +83,7 @@ void InitializeListHeadOrNode(EvaluableNode *node, EvaluableNode *parent, Evalua { // child node; initialize it and add it to the list items std::vector *ocn_ptr = &parent->GetOrderedChildNodesReference(); - (*ocn_ptr)[nodeIndex-1] = node; + (*ocn_ptr)[node_index-1] = node; node->InitializeType(child_node_type); } } From 1702af51825e99f12d7e57531c8feedb6cee0417 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Fri, 22 Nov 2024 16:26:32 -0500 Subject: [PATCH 22/35] Fixed infinite loop by updated num_total_nodes_needed. --- src/Amalgam/evaluablenode/EvaluableNode.cpp | 2 +- src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 3 +++ src/Amalgam/evaluablenode/EvaluableNodeManagement.h | 4 ++++ src/Amalgam/interpreter/Interpreter.cpp | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNode.cpp b/src/Amalgam/evaluablenode/EvaluableNode.cpp index c508bb85..644b52dc 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNode.cpp @@ -1646,7 +1646,7 @@ void EvaluableNode::Invalidate() break; case ENT_STRING: case ENT_SYMBOL: - std::cout << "!!!naricc_debug!!!: Invalidate: " << value.stringValueContainer.stringID << " label: " << value.stringValueContainer.labelStringID << std::endl; + // std::cout << "!!!naricc_debug!!!: Invalidate: " << value.stringValueContainer.stringID << " label: " << value.stringValueContainer.labelStringID << std::endl; string_intern_pool.DestroyStringReferences(value.stringValueContainer.stringID, value.stringValueContainer.labelStringID); break; case ENT_ASSOC: diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index ed2f80f7..e4975608 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -156,11 +156,14 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl return parent; } + { #ifdef MULTITHREAD_SUPPORT //don't have enough nodes, so need to attempt a write lock to allocate more Concurrency::WriteLock write_lock(managerAttributesMutex); + num_total_nodes_needed = firstUnusedNodeIndex + (num_to_alloc - num_allocated); + //try again after write lock to allocate a node in case another thread has performed the allocation //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex #endif diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index f97391cf..8a78710f 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1121,6 +1121,8 @@ class EvaluableNodeManager { EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; threadLocalAllocationBuffer.pop_back(); + + // std::cout << "!!!naricc_debug!!! GetNextNodeFromTLab: " << end << std::endl; return end; } else @@ -1138,6 +1140,8 @@ class EvaluableNodeManager { assert(en->IsNodeDeallocated()); + // std::cout << "!!!naricc_debug!!! AddNodeToTLab " << en << std::endl; + if(this != lastEvaluableNodeManager) { threadLocalAllocationBuffer.clear(); diff --git a/src/Amalgam/interpreter/Interpreter.cpp b/src/Amalgam/interpreter/Interpreter.cpp index b0e6c450..08b0c794 100644 --- a/src/Amalgam/interpreter/Interpreter.cpp +++ b/src/Amalgam/interpreter/Interpreter.cpp @@ -491,6 +491,7 @@ EvaluableNodeReference Interpreter::InterpretNode(EvaluableNode *en, bool immedi EvaluableNodeType ent = en->GetType(); auto oc = _opcodes[ent]; + // std::cout << "!!!naricc_debug!!! InterpretNode: en: " << en << " type: " << ((uint)en->GetType()) << " valid: " << en->IsNodeValid() << std::endl; EvaluableNodeReference retval = (this->*oc)(en, immediate_result); #ifdef AMALGAM_MEMORY_INTEGRITY From f13c02b57a219823c111586264a074ffbc99f232 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Fri, 22 Nov 2024 16:49:04 -0500 Subject: [PATCH 23/35] Put loop fix back in. --- src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index f77c027f..9be859fa 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -164,10 +164,13 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl //don't have enough nodes, so need to attempt a write lock to allocate more Concurrency::WriteLock write_lock(managerAttributesMutex); + num_total_nodes_needed = firstUnusedNodeIndex + (num_to_alloc - num_allocated); + //try again after write lock to allocate a node in case another thread has performed the allocation //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex #endif + //if don't currently have enough free nodes to meet the needs, then expand the allocation if(nodes.size() <= num_total_nodes_needed) { From 5783e8b17cabd21ef8e4fe7a1400ce2f4987209b Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Fri, 22 Nov 2024 17:07:33 -0500 Subject: [PATCH 24/35] 22156: Fixes bug, memory leak remains --- src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 9be859fa..ad7116f3 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -159,13 +159,12 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl return parent; } + num_total_nodes_needed = firstUnusedNodeIndex + (num_to_alloc - num_allocated); { #ifdef MULTITHREAD_SUPPORT //don't have enough nodes, so need to attempt a write lock to allocate more Concurrency::WriteLock write_lock(managerAttributesMutex); - num_total_nodes_needed = firstUnusedNodeIndex + (num_to_alloc - num_allocated); - //try again after write lock to allocate a node in case another thread has performed the allocation //already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex #endif From af42e13f390bc2e64577aa08c95d934a43b7d9a2 Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Fri, 22 Nov 2024 20:50:45 -0500 Subject: [PATCH 25/35] 22156: Fixes issue, minor style changes --- .../evaluablenode/EvaluableNodeManagement.cpp | 15 ++++++--------- .../evaluablenode/EvaluableNodeManagement.h | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index ad7116f3..50eaa39f 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -91,7 +91,7 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl if(num_child_nodes == 0) return AllocNode(ENT_LIST); - EvaluableNode* parent = nullptr; + EvaluableNode *parent = nullptr; // Allocate from TLab size_t num_to_alloc = 1 + num_child_nodes; @@ -102,12 +102,11 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl std::vector ocn_buffer; ocn_buffer.resize(num_child_nodes); - while(num_allocated < num_to_alloc) { - EvaluableNode* newNode = nullptr; + EvaluableNode *newNode = nullptr; - while ((newNode = GetNextNodeFromTLab()) && num_allocated < num_to_alloc) + while((newNode = GetNextNodeFromTLab()) && num_allocated < num_to_alloc) { if(parent == nullptr) { @@ -118,13 +117,11 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl num_allocated++; } - if (num_allocated >= num_to_alloc) + if(num_allocated >= num_to_alloc) { //we got enough nodes out of the tlab; tryutn return parent; } - - { // Not enough nodes in TLab; add some. #ifdef MULTITHREAD_SUPPORT @@ -215,9 +212,9 @@ void EvaluableNodeManager::CollectGarbage() PerformanceProfiler::StartOperation(collect_garbage_string, GetNumberOfUsedNodes()); } -#ifdef MULTITHREAD_SUPPORT - ClearThreadLocalAllocationBuffer(); + +#ifdef MULTITHREAD_SUPPORT //free lock so can attempt to enter write lock to collect garbage if(memory_modification_lock != nullptr) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index f97391cf..fdddeba0 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1119,7 +1119,7 @@ class EvaluableNodeManager { if(threadLocalAllocationBuffer.size() > 0 && this == lastEvaluableNodeManager) { - EvaluableNode* end = threadLocalAllocationBuffer[threadLocalAllocationBuffer.size()-1]; + EvaluableNode *end = threadLocalAllocationBuffer.back(); threadLocalAllocationBuffer.pop_back(); return end; } From 62e8acd382a454fc9102d84a4bfaab0a04dd807c Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 09:25:24 -0500 Subject: [PATCH 26/35] Untracked settings.json --- .vscode/settings.json | 73 +------------------------------------------ 1 file changed, 1 insertion(+), 72 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index e73b83d0..60d2427e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,75 +1,4 @@ { "cSpell.words": ["amlg", "cwrap", "KullbackLeibler"], - "files.trimTrailingWhitespace": false, - "files.associations": { - "__bit_reference": "cpp", - "__hash_table": "cpp", - "__locale": "cpp", - "__node_handle": "cpp", - "__split_buffer": "cpp", - "__threading_support": "cpp", - "__tree": "cpp", - "__verbose_abort": "cpp", - "array": "cpp", - "bitset": "cpp", - "cctype": "cpp", - "cfenv": "cpp", - "charconv": "cpp", - "cinttypes": "cpp", - "clocale": "cpp", - "cmath": "cpp", - "codecvt": "cpp", - "complex": "cpp", - "condition_variable": "cpp", - "cstdarg": "cpp", - "cstddef": "cpp", - "cstdint": "cpp", - "cstdio": "cpp", - "cstdlib": "cpp", - "cstring": "cpp", - "ctime": "cpp", - "cwchar": "cpp", - "cwctype": "cpp", - "deque": "cpp", - "execution": "cpp", - "memory": "cpp", - "forward_list": "cpp", - "fstream": "cpp", - "future": "cpp", - "initializer_list": "cpp", - "iomanip": "cpp", - "ios": "cpp", - "iosfwd": "cpp", - "iostream": "cpp", - "istream": "cpp", - "limits": "cpp", - "list": "cpp", - "locale": "cpp", - "map": "cpp", - "mutex": "cpp", - "new": "cpp", - "numbers": "cpp", - "optional": "cpp", - "ostream": "cpp", - "print": "cpp", - "queue": "cpp", - "ratio": "cpp", - "regex": "cpp", - "set": "cpp", - "shared_mutex": "cpp", - "span": "cpp", - "sstream": "cpp", - "stack": "cpp", - "stdexcept": "cpp", - "streambuf": "cpp", - "string": "cpp", - "string_view": "cpp", - "tuple": "cpp", - "typeinfo": "cpp", - "unordered_map": "cpp", - "unordered_set": "cpp", - "variant": "cpp", - "vector": "cpp", - "algorithm": "cpp" - } + "files.trimTrailingWhitespace": false } From 4738e996a4a8ca6cbfeefc3f368fa2beb83333ac Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 09:33:35 -0500 Subject: [PATCH 27/35] Get rid of debug defines. --- src/Amalgam/evaluablenode/EvaluableNode.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNode.h b/src/Amalgam/evaluablenode/EvaluableNode.h index 16bf8052..8dab3062 100644 --- a/src/Amalgam/evaluablenode/EvaluableNode.h +++ b/src/Amalgam/evaluablenode/EvaluableNode.h @@ -19,7 +19,6 @@ #define AMALGAM_FAST_MEMORY_INTEGRITY #endif -#define AMALGAM_FAST_MEMORY_INTEGRITY //forward declarations: class EvaluableNodeManager; From ac191ded3ecd9bd7e52970f4cd8876518382dc4e Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 09:48:50 -0500 Subject: [PATCH 28/35] Fixed bug when adding nodes to tlab for list. --- .../evaluablenode/EvaluableNodeManagement.cpp | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 50eaa39f..c34421d0 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -8,13 +8,6 @@ #include #include - -// #define PEDANTIC_GARBAGE_COLLECTION -#include - - -// #define PEDANTIC_GARBAGE_COLLECTION - #ifdef MULTITHREAD_SUPPORT Concurrency::ReadWriteMutex EvaluableNodeManager::memoryModificationMutex; #endif @@ -119,7 +112,7 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl if(num_allocated >= num_to_alloc) { - //we got enough nodes out of the tlab; tryutn + //we got enough nodes out of the tlab return parent; } @@ -128,7 +121,8 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl Concurrency::ReadLock lock(managerAttributesMutex); #endif - for(int num_added_to_tlab = 0; num_allocated + num_added_to_tlab < num_to_alloc; num_added_to_tlab++) + int num_added_to_tlab = 0; + for(; num_allocated + num_added_to_tlab < num_to_alloc; num_added_to_tlab++) { size_t allocated_index = firstUnusedNodeIndex++; if(allocated_index < nodes.size()) @@ -146,16 +140,17 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl --firstUnusedNodeIndex; break; } - } - } - if(num_allocated == num_to_alloc) - { - assert(parent); - return parent; + if( num_added_to_tlab + num_allocated >= num_allocated) + { + // We were able to add enough nodes to tlab; use them next time through the loop + continue; + } } + + // There weren't enough free nodes available to fill the tlab; allocate more num_total_nodes_needed = firstUnusedNodeIndex + (num_to_alloc - num_allocated); { #ifdef MULTITHREAD_SUPPORT From a654b779882bcb3804e8eb2ed3008582acda9d24 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 11:23:54 -0500 Subject: [PATCH 29/35] made TLab private. --- .../evaluablenode/EvaluableNodeManagement.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index fdddeba0..42494aed 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1086,13 +1086,6 @@ class EvaluableNodeManager std::unique_ptr nodesCurrentlyReferenced; public: - // TODO 22156: Make this private when done debugging - typedef std::vector TLab; - -#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) - thread_local -#endif - inline static TLab threadLocalAllocationBuffer; static void ClearThreadLocalAllocationBuffer() { @@ -1149,4 +1142,12 @@ class EvaluableNodeManager static const int tlabSize = 20; +private: + typedef std::vector TLab; + + #if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) + thread_local + #endif + inline static TLab threadLocalAllocationBuffer; + }; From 0ecd08ea1fe16b6d7b97feb1aa7d7d9d28d6c9b2 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 11:25:30 -0500 Subject: [PATCH 30/35] Removed unneeded include. --- src/Amalgam/evaluablenode/EvaluableNodeManagement.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 42494aed..abd448e1 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -6,7 +6,6 @@ //system headers: #include -#include //if the macro PEDANTIC_GARBAGE_COLLECTION is defined, then garbage collection will be performed //after every opcode, to help find and debug memory issues From c3401e3fedcdf32ef07e68224e0a0e7e01a2402c Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 11:30:35 -0500 Subject: [PATCH 31/35] Removed unused variable. --- src/Amalgam/evaluablenode/EvaluableNodeManagement.h | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index abd448e1..30ebcac6 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -1090,15 +1090,7 @@ class EvaluableNodeManager { threadLocalAllocationBuffer.clear(); } -protected: - - //buffer used for updating EvaluableNodeFlags, particularly UpdateFlagsForNodeTree -#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) - thread_local -#endif - static EvaluableNode::ReferenceAssocType nodeToParentNodeCache; - - +protected: //extra space to allocate when allocating static const double allocExpansionFactor; From 2b3cae9610f49adb544533949aff807ae47bd46e Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 11:34:26 -0500 Subject: [PATCH 32/35] Remove debug print. --- src/Amalgam/interpreter/Interpreter.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Amalgam/interpreter/Interpreter.cpp b/src/Amalgam/interpreter/Interpreter.cpp index 08b0c794..b0e6c450 100644 --- a/src/Amalgam/interpreter/Interpreter.cpp +++ b/src/Amalgam/interpreter/Interpreter.cpp @@ -491,7 +491,6 @@ EvaluableNodeReference Interpreter::InterpretNode(EvaluableNode *en, bool immedi EvaluableNodeType ent = en->GetType(); auto oc = _opcodes[ent]; - // std::cout << "!!!naricc_debug!!! InterpretNode: en: " << en << " type: " << ((uint)en->GetType()) << " valid: " << en->IsNodeValid() << std::endl; EvaluableNodeReference retval = (this->*oc)(en, immediate_result); #ifdef AMALGAM_MEMORY_INTEGRITY From 557cb03443aede6b914852d8f63585cc2662ae07 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 12:45:25 -0500 Subject: [PATCH 33/35] Fixed bug in AllocListNodeWithOrderedChildNodes --- src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index c34421d0..0a98682a 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -142,7 +142,7 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl } } - if( num_added_to_tlab + num_allocated >= num_allocated) + if( num_added_to_tlab + num_allocated >= num_to_alloc) { // We were able to add enough nodes to tlab; use them next time through the loop continue; From f38924faf3304fb65c8c5032387af719e169c460 Mon Sep 17 00:00:00 2001 From: Nathan Ricci Date: Mon, 25 Nov 2024 16:40:50 -0500 Subject: [PATCH 34/35] Style clean ups; comments. --- src/Amalgam/Parser.cpp | 2 -- .../evaluablenode/EvaluableNodeManagement.cpp | 14 +++------- .../evaluablenode/EvaluableNodeManagement.h | 28 +++++++++++++------ 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/Amalgam/Parser.cpp b/src/Amalgam/Parser.cpp index 2f27d0aa..a48b7dc9 100644 --- a/src/Amalgam/Parser.cpp +++ b/src/Amalgam/Parser.cpp @@ -199,8 +199,6 @@ EvaluableNode *Parser::GetCodeForPathToSharedNodeFromParentAToParentB(UnparseDat if(shared_node == nullptr || a_parent == nullptr || b_parent == nullptr) return nullptr; - //TODO 22156: allocate from enm in a way that does not invalidate tlab (but does not increase the size of EvaluableNodeManager) - //find all parent nodes of a to find collision with parent node of b, along with depth counts EvaluableNode::ReferenceCountType a_parent_nodes; size_t a_ancestor_depth = 1; diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp index 0a98682a..0e9028a8 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #ifdef MULTITHREAD_SUPPORT Concurrency::ReadWriteMutex EvaluableNodeManager::memoryModificationMutex; @@ -73,8 +72,8 @@ void InitializeListHeadOrNode(EvaluableNode *node, EvaluableNode *parent, Evalua else { // child node; initialize it and add it to the list items - std::vector *ocn_ptr = &parent->GetOrderedChildNodesReference(); - (*ocn_ptr)[node_index-1] = node; + auto &ocn = parent->GetOrderedChildNodesReference(); + ocn[node_index-1] = node; node->InitializeType(child_node_type); } } @@ -102,9 +101,7 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl while((newNode = GetNextNodeFromTLab()) && num_allocated < num_to_alloc) { if(parent == nullptr) - { parent = newNode; - } InitializeListHeadOrNode(newNode, parent, child_node_type, num_allocated, &ocn_buffer); num_allocated++; @@ -128,9 +125,8 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl if(allocated_index < nodes.size()) { if(nodes[allocated_index] == nullptr) - { nodes[allocated_index] = new EvaluableNode(ENT_DEALLOCATED); - } + AddNodeToTLab(nodes[allocated_index]); } @@ -142,11 +138,9 @@ EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(Evaluabl } } + // If we added enough nodes to the tlab, use them in the next loop iteration if( num_added_to_tlab + num_allocated >= num_to_alloc) - { - // We were able to add enough nodes to tlab; use them next time through the loop continue; - } } diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 30ebcac6..df9288f9 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -984,6 +984,12 @@ class EvaluableNodeManager //when numNodesToRunGarbageCollection are allocated, then it is time to run garbage collection size_t numNodesToRunGarbageCollection; + // Remove all EvaluableNodes from the thread local allocation buffer, leaving it empty. + inline static void ClearThreadLocalAllocationBuffer() + { + threadLocalAllocationBuffer.clear(); + } + protected: //allocates an EvaluableNode of the respective memory type in the appropriate way // returns an uninitialized EvaluableNode -- care must be taken to set fields properly @@ -1084,21 +1090,21 @@ class EvaluableNodeManager //only allocated if needed std::unique_ptr nodesCurrentlyReferenced; -public: - - static void ClearThreadLocalAllocationBuffer() - { - threadLocalAllocationBuffer.clear(); - } -protected: //extra space to allocate when allocating static const double allocExpansionFactor; #if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) thread_local #endif + + // We need to keep track of the the last EvaluableNodeManager that accessed + // the thread local allocation buffer for a given thread. We do this so + // a given thread local allocation buffer has only nodes associated with one manager. + // If a different manager accesses the buffer, we clear the buffer to maintain this invariant. static inline EvaluableNodeManager *lastEvaluableNodeManager; + // Get a pointer to the next available node from the thread local allocation buffer. + // If the buffer is empty, returns null. inline EvaluableNode *GetNextNodeFromTLab() { if(threadLocalAllocationBuffer.size() > 0 && this == lastEvaluableNodeManager) @@ -1118,6 +1124,10 @@ class EvaluableNodeManager } + // Adds a node to the thread local allocation buffer. + // If this is accessed by a different EvaluableNode manager than + // the last time it was called on this thread, it will clear the buffer + // before adding the node. inline void AddNodeToTLab(EvaluableNode *en) { assert(en->IsNodeDeallocated()); @@ -1131,14 +1141,16 @@ class EvaluableNodeManager threadLocalAllocationBuffer.push_back(en); } +private: + static const int tlabSize = 20; -private: typedef std::vector TLab; #if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE) thread_local #endif + // This buffer holds EvaluableNode*'s reserved for allocation by a specific thread inline static TLab threadLocalAllocationBuffer; }; From 948c0113d7ffd98ee054454d34f5c4cbe417e87d Mon Sep 17 00:00:00 2001 From: "Christopher J. Hazard, PhD" <143410553+howsohazard@users.noreply.github.com> Date: Mon, 25 Nov 2024 17:37:47 -0500 Subject: [PATCH 35/35] 22173: Fixes possible issue when setting an Entity root to null --- src/Amalgam/entity/Entity.cpp | 7 ++----- src/Amalgam/evaluablenode/EvaluableNodeManagement.h | 4 ++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Amalgam/entity/Entity.cpp b/src/Amalgam/entity/Entity.cpp index 558df5dd..e5fc47bd 100644 --- a/src/Amalgam/entity/Entity.cpp +++ b/src/Amalgam/entity/Entity.cpp @@ -859,11 +859,8 @@ void Entity::SetRoot(EvaluableNode *_code, bool allocated_with_entity_enm, Evalu EvaluableNode *cur_root = GetRoot(); bool entity_previously_empty = (cur_root == nullptr || cur_root->GetNumChildNodes() == 0); - if(_code == nullptr) - { - evaluableNodeManager.SetRootNode(evaluableNodeManager.AllocNode(ENT_NULL)); - } - else if(allocated_with_entity_enm && metadata_modifier == EvaluableNodeManager::ENMM_NO_CHANGE) + if(_code == nullptr + || (allocated_with_entity_enm && metadata_modifier == EvaluableNodeManager::ENMM_NO_CHANGE)) { evaluableNodeManager.SetRootNode(_code); } diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index df9288f9..d552a446 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -926,8 +926,12 @@ class EvaluableNodeManager //sets the root node, implicitly defined as the first node in memory, to new_root // note that new_root MUST have been allocated by this EvaluableNodeManager //ensures that the new root node is kept and the old is released + //if new_root is nullptr, then it allocates its own ENT_NULL node inline void SetRootNode(EvaluableNode *new_root) { + if(new_root == nullptr) + new_root = AllocNode(ENT_NULL); + #ifdef MULTITHREAD_SUPPORT //use WriteLock to be safe Concurrency::WriteLock lock(managerAttributesMutex);