diff --git a/src/core/agent/agent.h b/src/core/agent/agent.h index 991e23c35..a5d780f2d 100644 --- a/src/core/agent/agent.h +++ b/src/core/agent/agent.h @@ -151,11 +151,23 @@ class Agent { /// If the thread-safety mechanism is set to user-specified this function /// will be called before the operations are executed for this agent.\n - /// Subclasses define the critical region by adding the uids of all + /// Subclasses define the critical region by adding the AgentPointers of all /// agents that must not be processed in parallel. \n - /// Don't forget to add the uid of the current agent.\n + /// Don't forget to add the current agent.\n + /// Here an example from NeuronSoma.\n + /// + /// void NeuronSoma::CriticalRegion(std::vector>* aptrs) { + /// aptrs->reserve(daughters_.size() + 1); + /// aptrs->push_back(Agent::GetAgentPtr<>()); + /// for (auto& daughter : daughters_) { + /// aptrs->push_back(daughter); + /// } + /// } + /// /// \see `Param::thread_safety_mechanism` - virtual void CriticalRegion(std::vector* uids) {} + /// \see `AgentPointer` + /// \see `NeuronSoma::CriticalRegion` + virtual void CriticalRegion(std::vector>* aptrs) {} uint32_t GetBoxIdx() const; diff --git a/src/core/agent/agent_pointer.h b/src/core/agent/agent_pointer.h index 1347e76d1..a3c40174f 100644 --- a/src/core/agent/agent_pointer.h +++ b/src/core/agent/agent_pointer.h @@ -21,7 +21,7 @@ #include #include "core/agent/agent_uid.h" -#include "core/execution_context/in_place_exec_ctxt.h" +#include "core/execution_context/execution_context.h" #include "core/simulation.h" #include "core/util/root.h" @@ -37,7 +37,7 @@ class Agent; /// the type returned by `Get` and can therefore inline the code from the callee /// and perform optimizations. /// @tparam TAgent agent type -template +template class AgentPointer { public: explicit AgentPointer(const AgentUid& uid) : uid_(uid) {} @@ -106,8 +106,12 @@ class AgentPointer { const TAgent& operator*() const { return *(this->operator->()); } + bool operator<(const AgentPointer& other) const { return uid_ < other.uid_; } + operator bool() const { return *this != nullptr; } // NOLINT + operator AgentPointer() const { return AgentPointer(uid_); } + TAgent* Get() { return this->operator->(); } const TAgent* Get() const { return this->operator->(); } diff --git a/src/core/execution_context/execution_context.h b/src/core/execution_context/execution_context.h index e4c728639..8c0860cb6 100644 --- a/src/core/execution_context/execution_context.h +++ b/src/core/execution_context/execution_context.h @@ -18,7 +18,9 @@ #include #include +#include "core/agent/agent_handle.h" #include "core/agent/agent_uid.h" +#include "core/container/math_array.h" #include "core/functor.h" #include "core/operation/operation.h" diff --git a/src/core/execution_context/in_place_exec_ctxt.cc b/src/core/execution_context/in_place_exec_ctxt.cc index a1a241cb7..83ba1143e 100644 --- a/src/core/execution_context/in_place_exec_ctxt.cc +++ b/src/core/execution_context/in_place_exec_ctxt.cc @@ -157,27 +157,40 @@ void InPlaceExecutionContext::Execute( critical_region_2_.clear(); locks_.clear(); agent->CriticalRegion(&critical_region_); + // Sort such that the locks further down are acquired in a sorted order + // This technique avoids deadlocks. std::sort(critical_region_.begin(), critical_region_.end()); - for (auto uid : critical_region_) { - locks_.push_back(GetAgent(uid)->GetLock()); + // Remove all AgentPointers which correspond to a nullptr. + while (critical_region_.size() && critical_region_.back() == nullptr) { + critical_region_.pop_back(); + } + // Remove all duplicate entries + critical_region_.erase( + std::unique(critical_region_.begin(), critical_region_.end()), + critical_region_.end()); + for (auto aptr : critical_region_) { + locks_.push_back(aptr->GetLock()); } for (auto* l : locks_) { l->lock(); } agent->CriticalRegion(&critical_region_2_); + // Sort such that the locks further down are acquired in a sorted order + // This technique avoids deadlocks. std::sort(critical_region_2_.begin(), critical_region_2_.end()); - bool same = true; - if (critical_region_.size() == critical_region_2_.size()) { - for (size_t i = 0; i < critical_region_.size(); ++i) { - if (critical_region_[i] != critical_region_2_[i]) { - same = false; - break; - } - } - } else { - same = false; + // Remove all AgentPointers which correspond to a nullptr. + while (critical_region_2_.size() && + critical_region_2_.back() == nullptr) { + critical_region_2_.pop_back(); } - if (same) { + // Remove all duplicate entries + critical_region_2_.erase( + std::unique(critical_region_2_.begin(), critical_region_2_.end()), + critical_region_2_.end()); + // if the critical regions are not the same, then another thread + // changed it before the locks were acquired. In this case we have to + // try again. Otherwise we can leave the while loop. + if (critical_region_ == critical_region_2_) { break; } for (auto* l : locks_) { diff --git a/src/core/execution_context/in_place_exec_ctxt.h b/src/core/execution_context/in_place_exec_ctxt.h index de33dbbc8..e1a0baf12 100644 --- a/src/core/execution_context/in_place_exec_ctxt.h +++ b/src/core/execution_context/in_place_exec_ctxt.h @@ -22,6 +22,7 @@ #include #include "core/agent/agent_handle.h" +#include "core/agent/agent_pointer.h" #include "core/agent/agent_uid.h" #include "core/container/agent_uid_map.h" #include "core/container/math_array.h" @@ -140,13 +141,6 @@ class InPlaceExecutionContext : public ExecutionContext { ThreadInfo* tinfo_; - /// Contains unique ids of agents that will be removed at the end of each - /// iteration. AgentUids are separated by numa node. - std::vector remove_; - std::vector critical_region_; - std::vector critical_region_2_; - std::vector locks_; - /// Pointer to new agents std::vector new_agents_; @@ -169,6 +163,17 @@ class InPlaceExecutionContext : public ExecutionContext { virtual void RemoveAgentsFromRm( const std::vector& all_exec_ctxts); + + private: + /// Contains unique ids of agents that will be removed at the end of each + /// iteration. AgentUids are separated by numa node. + std::vector remove_; + /// Used to determine which agents must not be updated from different threads. + std::vector> critical_region_; + /// Used to determine which agents must not be updated from different threads. + std::vector> critical_region_2_; + + std::vector locks_; }; } // namespace bdm diff --git a/src/core/type_index.h b/src/core/type_index.h index bef968986..5c611a1ce 100644 --- a/src/core/type_index.h +++ b/src/core/type_index.h @@ -17,6 +17,7 @@ #include #include "core/agent/agent.h" +#include "core/container/agent_uid_map.h" #include "core/container/flatmap.h" class TClass; diff --git a/src/neuroscience/neurite_element.cc b/src/neuroscience/neurite_element.cc index 8b983db98..cf3c65ab4 100644 --- a/src/neuroscience/neurite_element.cc +++ b/src/neuroscience/neurite_element.cc @@ -119,15 +119,15 @@ void NeuriteElement::Update(const NewAgentEvent& event) { } } -void NeuriteElement::CriticalRegion(std::vector* uids) { - uids->reserve(4); - uids->push_back(GetUid()); - uids->push_back(mother_.GetUid()); - if (daughter_left_ != nullptr) { - uids->push_back(daughter_left_.GetUid()); +void NeuriteElement::CriticalRegion(std::vector>* aptrs) { + aptrs->reserve(4); + aptrs->push_back(GetAgentPtr<>()); + aptrs->push_back(mother_); + if (daughter_left_) { + aptrs->push_back(daughter_left_); } - if (daughter_right_ != nullptr) { - uids->push_back(daughter_right_.GetUid()); + if (daughter_right_) { + aptrs->push_back(daughter_right_); } } diff --git a/src/neuroscience/neurite_element.h b/src/neuroscience/neurite_element.h index 400409f57..5e1c4fb2a 100644 --- a/src/neuroscience/neurite_element.h +++ b/src/neuroscience/neurite_element.h @@ -67,7 +67,7 @@ class NeuriteElement : public Agent, public NeuronOrNeurite { Spinlock* GetLock() override { return Base::GetLock(); } - void CriticalRegion(std::vector* uids) override; + void CriticalRegion(std::vector>* aptrs) override; Shape GetShape() const override { return Shape::kCylinder; } diff --git a/src/neuroscience/neuron_soma.cc b/src/neuroscience/neuron_soma.cc index 72d50cd91..8311865ad 100644 --- a/src/neuroscience/neuron_soma.cc +++ b/src/neuroscience/neuron_soma.cc @@ -66,11 +66,11 @@ void NeuronSoma::Update(const NewAgentEvent& event) { // do nothing for CellDivisionEvent or others } -void NeuronSoma::CriticalRegion(std::vector* uids) { - uids->reserve(daughters_.size() + 1); - uids->push_back(Agent::GetUid()); +void NeuronSoma::CriticalRegion(std::vector>* aptrs) { + aptrs->reserve(daughters_.size() + 1); + aptrs->push_back(Agent::GetAgentPtr<>()); for (auto& daughter : daughters_) { - uids->push_back(daughter.GetUid()); + aptrs->push_back(daughter); } } diff --git a/src/neuroscience/neuron_soma.h b/src/neuroscience/neuron_soma.h index f61c51058..edfc5fa2c 100644 --- a/src/neuroscience/neuron_soma.h +++ b/src/neuroscience/neuron_soma.h @@ -61,7 +61,7 @@ class NeuronSoma : public Cell, public NeuronOrNeurite { Spinlock* GetLock() override { return Base::GetLock(); } - void CriticalRegion(std::vector* uids) override; + void CriticalRegion(std::vector>* aptrs) override; // *************************************************************************** // METHODS FOR NEURON TREE STRUCTURE *