From: SUTER Frederic Date: Thu, 2 Sep 2021 10:21:16 +0000 (+0200) Subject: [procasticommit] Do not mix private and public fields X-Git-Tag: v3.29~115 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/f01052cf1a094dd5df8869e4b26431b1efb6abb0 [procasticommit] Do not mix private and public fields --- diff --git a/include/simgrid/s4u/Mailbox.hpp b/include/simgrid/s4u/Mailbox.hpp index 3fabf154d3..9687ef9ea9 100644 --- a/include/simgrid/s4u/Mailbox.hpp +++ b/include/simgrid/s4u/Mailbox.hpp @@ -47,7 +47,7 @@ public: bool empty() const; /* Returns the number of queued communications */ - unsigned int size() const; + size_t size() const; /** Check if there is a communication going on in a mailbox. */ bool listen() const; diff --git a/src/instr/instr_config.cpp b/src/instr/instr_config.cpp index 528548b106..3c22b9b5ce 100644 --- a/src/instr/instr_config.cpp +++ b/src/instr/instr_config.cpp @@ -234,7 +234,7 @@ static void on_container_creation_paje(const Container& c) timestamp); stream << std::fixed << std::setprecision(trace_precision) << PajeEventType::CreateContainer << " "; - stream << timestamp << " " << c.get_id() << " " << c.type_->get_id() << " " << c.parent_->get_id() << " \""; + stream << timestamp << " " << c.get_id() << " " << c.get_type()->get_id() << " " << c.get_parent()->get_id() << " \""; if (c.get_name().find("rank-") != 0) stream << c.get_name() << "\""; else @@ -256,7 +256,7 @@ static void on_container_destruction_paje(const Container& c) timestamp); stream << std::fixed << std::setprecision(trace_precision) << PajeEventType::DestroyContainer << " "; - stream << timestamp << " " << c.type_->get_id() << " " << c.get_id(); + stream << timestamp << " " << c.get_type()->get_id() << " " << c.get_id(); XBT_DEBUG("Dump %s", stream.str().c_str()); tracing_file << stream.str() << std::endl; } @@ -423,7 +423,7 @@ static void on_simulation_end() last_timestamp_to_dump = surf_get_clock(); dump_buffer(true); - const Type* root_type = Container::get_root()->type_; + const Type* root_type = Container::get_root()->get_type(); /* destroy all data structures of tracing (and free) */ delete Container::get_root(); delete root_type; diff --git a/src/instr/instr_interface.cpp b/src/instr/instr_interface.cpp index a25a45c0d1..a1b6da9dac 100644 --- a/src/instr/instr_interface.cpp +++ b/src/instr/instr_interface.cpp @@ -147,7 +147,7 @@ void TRACE_declare_mark(const char *mark_type) } XBT_DEBUG("MARK,declare %s", mark_type); - simgrid::instr::Container::get_root()->type_->by_name_or_create(mark_type); + simgrid::instr::Container::get_root()->get_type()->by_name_or_create(mark_type); declared_marks.emplace(mark_type); } @@ -176,7 +176,7 @@ void TRACE_declare_mark_value_with_color (const char *mark_type, const char *mar xbt_assert(mark_value, "mark_value is nullptr"); auto* type = - static_cast(simgrid::instr::Container::get_root()->type_->by_name(mark_type)); + static_cast(simgrid::instr::Container::get_root()->get_type()->by_name(mark_type)); if (not type) { throw simgrid::TracingError(XBT_THROW_POINT, simgrid::xbt::string_printf("mark_type with name (%s) is not declared", mark_type)); @@ -231,7 +231,7 @@ void TRACE_mark(const char *mark_type, const char *mark_value) //check if mark_type is already declared auto* type = - static_cast(simgrid::instr::Container::get_root()->type_->by_name(mark_type)); + static_cast(simgrid::instr::Container::get_root()->get_type()->by_name(mark_type)); if (not type) { throw simgrid::TracingError(XBT_THROW_POINT, simgrid::xbt::string_printf("mark_type with name (%s) is not declared", mark_type)); diff --git a/src/instr/instr_paje_containers.hpp b/src/instr/instr_paje_containers.hpp index c4befb69a6..8ce3d7f3e9 100644 --- a/src/instr/instr_paje_containers.hpp +++ b/src/instr/instr_paje_containers.hpp @@ -17,11 +17,15 @@ class StateType; class VariableType; class Container { + friend class NetZoneContainer; static Container* root_container_; static std::map> all_containers_; long long int id_; std::string name_; /* Unique name of this container */ + Type* type_; /* Type of this container */ + Container* parent_; + std::map> children_; protected: static void set_root(Container* root) { root_container_ = root; } @@ -35,9 +39,6 @@ public: Container& operator=(const Container&) = delete; virtual ~Container(); - Type* type_; /* Type of this container */ - Container* parent_; - std::map> children_; static Container* by_name_or_null(const std::string& name); static Container* by_name(const std::string& name); @@ -46,10 +47,13 @@ public: long long int get_id() const { return id_; } void remove_from_parent(); + Container* get_parent() const { return parent_; } + Type* get_type() const { return type_; } StateType* get_state(const std::string& name); LinkType* get_link(const std::string& name); VariableType* get_variable(const std::string& name); void create_child(const std::string& name, const std::string& type_name); + Container* get_child_by_name(const std::string& name) const { return children_.at(name); } static Container* get_root() { return root_container_; } }; diff --git a/src/instr/instr_platform.cpp b/src/instr/instr_platform.cpp index f38cb50682..d096c421aa 100644 --- a/src/instr/instr_platform.cpp +++ b/src/instr/instr_platform.cpp @@ -33,17 +33,17 @@ static simgrid::instr::Container* lowestCommonAncestor(const simgrid::instr::Con const simgrid::instr::Container* a2) { // this is only an optimization (since most of a1 and a2 share the same parent) - if (a1->parent_ == a2->parent_) - return a1->parent_; + if (a1->get_parent() == a2->get_parent()) + return a1->get_parent(); // create an array with all ancestors of a1 std::vector ancestors_a1; - for (auto* p = a1->parent_; p != nullptr; p = p->parent_) + for (auto* p = a1->get_parent(); p != nullptr; p = p->get_parent()) ancestors_a1.push_back(p); // create an array with all ancestors of a2 std::vector ancestors_a2; - for (auto* p = a2->parent_; p != nullptr; p = p->parent_) + for (auto* p = a2->get_parent(); p != nullptr; p = p->get_parent()) ancestors_a2.push_back(p); // find the lowest ancestor @@ -94,10 +94,11 @@ static void linkContainers(simgrid::instr::Container* src, simgrid::instr::Conta filter->insert(aux2); // declare type - std::string link_typename = parent->type_->get_name() + "-" + src->type_->get_name() + - std::to_string(src->type_->get_id()) + "-" + dst->type_->get_name() + - std::to_string(dst->type_->get_id()); - simgrid::instr::LinkType* link = parent->type_->by_name_or_create(link_typename, src->type_, dst->type_); + std::string link_typename = parent->get_type()->get_name() + "-" + src->get_type()->get_name() + + std::to_string(src->get_type()->get_id()) + "-" + dst->get_type()->get_name() + + std::to_string(dst->get_type()->get_id()); + simgrid::instr::LinkType* link = + parent->get_type()->by_name_or_create(link_typename, src->get_type(), dst->get_type()); link->set_calling_container(parent); // create the link @@ -123,7 +124,7 @@ static void recursiveGraphExtraction(const simgrid::s4u::NetZone* netzone, simgr // bottom-up recursion for (auto const& nz_son : netzone->get_children()) { - simgrid::instr::Container* child_container = container->children_.at(nz_son->get_name()); + simgrid::instr::Container* child_container = container->get_child_by_name(nz_son->get_name()); recursiveGraphExtraction(nz_son, child_container, filter); } @@ -159,7 +160,7 @@ static void recursiveNewVariableType(const std::string& new_typename, const std: void instr_new_variable_type(const std::string& new_typename, const std::string& color) { - recursiveNewVariableType(new_typename, color, simgrid::instr::Container::get_root()->type_); + recursiveNewVariableType(new_typename, color, simgrid::instr::Container::get_root()->get_type()); } static void recursiveNewUserVariableType(const std::string& parent_type, const std::string& new_typename, @@ -175,7 +176,7 @@ static void recursiveNewUserVariableType(const std::string& parent_type, const s void instr_new_user_variable_type(const std::string& parent_type, const std::string& new_typename, const std::string& color) { - recursiveNewUserVariableType(parent_type, new_typename, color, simgrid::instr::Container::get_root()->type_); + recursiveNewUserVariableType(parent_type, new_typename, color, simgrid::instr::Container::get_root()->get_type()); } static void recursiveNewUserStateType(const std::string& parent_type, const std::string& new_typename, @@ -190,7 +191,7 @@ static void recursiveNewUserStateType(const std::string& parent_type, const std: void instr_new_user_state_type(const std::string& parent_type, const std::string& new_typename) { - recursiveNewUserStateType(parent_type, new_typename, simgrid::instr::Container::get_root()->type_); + recursiveNewUserStateType(parent_type, new_typename, simgrid::instr::Container::get_root()->get_type()); } static void recursiveNewValueForUserStateType(const std::string& type_name, const char* val, const std::string& color, @@ -205,7 +206,7 @@ static void recursiveNewValueForUserStateType(const std::string& type_name, cons void instr_new_value_for_user_state_type(const std::string& type_name, const char* value, const std::string& color) { - recursiveNewValueForUserStateType(type_name, value, color, simgrid::instr::Container::get_root()->type_); + recursiveNewValueForUserStateType(type_name, value, color, simgrid::instr::Container::get_root()->get_type()); } namespace simgrid { @@ -259,11 +260,11 @@ static void on_netzone_creation(s4u::NetZone const& netzone) xbt_assert(Container::get_root() == root); if (TRACE_smpi_is_enabled()) { - auto* mpi = root->type_->by_name_or_create("MPI"); + auto* mpi = root->get_type()->by_name_or_create("MPI"); if (not TRACE_smpi_is_grouped()) mpi->by_name_or_create("MPI_STATE"); - root->type_->by_name_or_create("MPI_LINK", mpi, mpi); - root->type_->by_name_or_create("MIGRATE_LINK", mpi, mpi); + root->get_type()->by_name_or_create("MPI_LINK", mpi, mpi); + root->get_type()->by_name_or_create("MIGRATE_LINK", mpi, mpi); mpi->by_name_or_create("MIGRATE_STATE"); } @@ -288,16 +289,16 @@ static void on_link_creation(s4u::Link const& link) auto* container = new Container(link.get_name(), "LINK", currentContainer.back()); if ((TRACE_categorized() || TRACE_uncategorized() || TRACE_platform()) && (not TRACE_disable_link())) { - VariableType* bandwidth = container->type_->by_name_or_create("bandwidth", ""); + VariableType* bandwidth = container->get_type()->by_name_or_create("bandwidth", ""); bandwidth->set_calling_container(container); bandwidth->set_event(0, link.get_bandwidth()); - VariableType* latency = container->type_->by_name_or_create("latency", ""); + VariableType* latency = container->get_type()->by_name_or_create("latency", ""); latency->set_calling_container(container); latency->set_event(0, link.get_latency()); } if (TRACE_uncategorized()) { - container->type_->by_name_or_create("bandwidth_used", "0.5 0.5 0.5"); + container->get_type()->by_name_or_create("bandwidth_used", "0.5 0.5 0.5"); } } @@ -310,22 +311,22 @@ static void on_host_creation(s4u::Host const& host) const Container* root = Container::get_root(); if ((TRACE_categorized() || TRACE_uncategorized() || TRACE_platform()) && (not TRACE_disable_speed())) { - VariableType* speed = container->type_->by_name_or_create("speed", ""); + VariableType* speed = container->get_type()->by_name_or_create("speed", ""); speed->set_calling_container(container); speed->set_event(0, host.get_speed()); - VariableType* cores = container->type_->by_name_or_create("core_count", ""); + VariableType* cores = container->get_type()->by_name_or_create("core_count", ""); cores->set_calling_container(container); cores->set_event(0, host.get_core_count()); } if (TRACE_uncategorized()) - container->type_->by_name_or_create("speed_used", "0.5 0.5 0.5"); + container->get_type()->by_name_or_create("speed_used", "0.5 0.5 0.5"); if (TRACE_smpi_is_enabled() && TRACE_smpi_is_grouped()) { - auto* mpi = container->type_->by_name_or_create("MPI"); + auto* mpi = container->get_type()->by_name_or_create("MPI"); mpi->by_name_or_create("MPI_STATE"); - root->type_->by_name_or_create("MIGRATE_LINK", mpi, mpi); + root->get_type()->by_name_or_create("MIGRATE_LINK", mpi, mpi); mpi->by_name_or_create("MIGRATE_STATE"); } } @@ -370,14 +371,14 @@ static void on_actor_creation(s4u::Actor const& actor) std::string container_name = instr_pid(actor); container->create_child(container_name, "ACTOR"); - auto* actor_type = container->type_->by_name_or_create("ACTOR"); + auto* actor_type = container->get_type()->by_name_or_create("ACTOR"); auto* state = actor_type->by_name_or_create("ACTOR_STATE"); state->add_entity_value("suspend", "1 0 1"); state->add_entity_value("sleep", "1 1 0"); state->add_entity_value("receive", "1 0 0"); state->add_entity_value("send", "0 0 1"); state->add_entity_value("execute", "0 1 1"); - root->type_->by_name_or_create("ACTOR_LINK", actor_type, actor_type); + root->get_type()->by_name_or_create("ACTOR_LINK", actor_type, actor_type); actor.on_exit([container_name](bool failed) { if (failed) @@ -407,15 +408,15 @@ static void on_vm_creation(s4u::Host const& host) { const Container* container = new HostContainer(host, currentContainer.back()); const Container* root = Container::get_root(); - auto* vm = container->type_->by_name_or_create("VM"); + auto* vm = container->get_type()->by_name_or_create("VM"); auto* state = vm->by_name_or_create("VM_STATE"); state->add_entity_value("suspend", "1 0 1"); state->add_entity_value("sleep", "1 1 0"); state->add_entity_value("receive", "1 0 0"); state->add_entity_value("send", "0 0 1"); state->add_entity_value("execute", "0 1 1"); - root->type_->by_name_or_create("VM_LINK", vm, vm); - root->type_->by_name_or_create("VM_ACTOR_LINK", vm, vm); + root->get_type()->by_name_or_create("VM_LINK", vm, vm); + root->get_type()->by_name_or_create("VM_ACTOR_LINK", vm, vm); } void define_callbacks() diff --git a/src/kernel/activity/CommImpl.cpp b/src/kernel/activity/CommImpl.cpp index 8785fb7bac..b6726fa994 100644 --- a/src/kernel/activity/CommImpl.cpp +++ b/src/kernel/activity/CommImpl.cpp @@ -55,11 +55,11 @@ XBT_PRIVATE simgrid::kernel::activity::ActivityImplPtr simcall_HANDLER_comm_isen if (not other_comm) { other_comm = std::move(this_comm); - if (mbox->permanent_receiver_ != nullptr) { + if (mbox->is_permanent()) { // this mailbox is for small messages, which have to be sent right now other_comm->state_ = simgrid::kernel::activity::State::READY; - other_comm->dst_actor_ = mbox->permanent_receiver_.get(); - mbox->done_comm_queue_.push_back(other_comm); + other_comm->dst_actor_ = mbox->get_permanent_receiver().get(); + mbox->push_done(other_comm); XBT_DEBUG("pushing a message into the permanent receive list %p, comm %p", mbox, other_comm.get()); } else { @@ -119,7 +119,7 @@ simcall_HANDLER_comm_irecv(smx_simcall_t /*simcall*/, smx_actor_t receiver, smx_ simgrid::kernel::activity::CommImplPtr other_comm; // communication already done, get it inside the list of completed comms - if (mbox->permanent_receiver_ != nullptr && not mbox->done_comm_queue_.empty()) { + if (mbox->is_permanent() && mbox->has_some_done_comm()) { XBT_DEBUG("We have a comm that has probably already been received, trying to match it, to skip the communication"); // find a match in the list of already received comms other_comm = mbox->find_matching_comm(simgrid::kernel::activity::CommImpl::Type::SEND, match_fun, data, @@ -150,7 +150,7 @@ simcall_HANDLER_comm_irecv(smx_simcall_t /*simcall*/, smx_actor_t receiver, smx_ /*remove_matching*/ true); if (other_comm == nullptr) { - XBT_DEBUG("Receive pushed first (%zu comm enqueued so far)", mbox->comm_queue_.size()); + XBT_DEBUG("Receive pushed first (%zu comm enqueued so far)", mbox->size()); other_comm = std::move(this_synchro); mbox->push(other_comm); } else { diff --git a/src/kernel/activity/MailboxImpl.hpp b/src/kernel/activity/MailboxImpl.hpp index 14a03d3882..2ae26317a3 100644 --- a/src/kernel/activity/MailboxImpl.hpp +++ b/src/kernel/activity/MailboxImpl.hpp @@ -25,10 +25,14 @@ class MailboxImpl { s4u::Mailbox piface_; xbt::string name_; + actor::ActorImplPtr permanent_receiver_; // actor to which the mailbox is attached + boost::circular_buffer_space_optimized comm_queue_{MAX_MAILBOX_SIZE}; + // messages already received in the permanent receive mode + boost::circular_buffer_space_optimized done_comm_queue_{MAX_MAILBOX_SIZE}; friend s4u::Engine; - friend s4u::Mailbox* s4u::Engine::mailbox_by_name_or_create(const std::string& name) const; friend s4u::Mailbox; + friend s4u::Mailbox* s4u::Engine::mailbox_by_name_or_create(const std::string& name) const; friend s4u::Mailbox* s4u::Mailbox::by_name(const std::string& name); friend mc::CommunicationDeterminismChecker; @@ -43,15 +47,18 @@ public: const char* get_cname() const { return name_.c_str(); } void set_receiver(s4u::ActorPtr actor); void push(CommImplPtr comm); + void push_done(CommImplPtr done_comm) { done_comm_queue_.push_back(done_comm); } void remove(const CommImplPtr& comm); CommImplPtr iprobe(int type, bool (*match_fun)(void*, void*, CommImpl*), void* data); CommImplPtr find_matching_comm(CommImpl::Type type, bool (*match_fun)(void*, void*, CommImpl*), void* this_user_data, const CommImplPtr& my_synchro, bool done, bool remove_matching); - - actor::ActorImplPtr permanent_receiver_; // actor to which the mailbox is attached - boost::circular_buffer_space_optimized comm_queue_{MAX_MAILBOX_SIZE}; - // messages already received in the permanent receive mode - boost::circular_buffer_space_optimized done_comm_queue_{MAX_MAILBOX_SIZE}; + bool is_permanent() const { return permanent_receiver_ != nullptr; } + actor::ActorImplPtr get_permanent_receiver() { return permanent_receiver_; } + bool empty() const { return comm_queue_.empty(); } + size_t size() const { return comm_queue_.size(); } + CommImplPtr front() const { return comm_queue_.front(); } + bool has_some_done_comm() const { return not done_comm_queue_.empty(); } + CommImplPtr done_front() const { return done_comm_queue_.front(); } }; } // namespace activity } // namespace kernel diff --git a/src/s4u/s4u_Mailbox.cpp b/src/s4u/s4u_Mailbox.cpp index 23b140ff78..2fcd87fe55 100644 --- a/src/s4u/s4u_Mailbox.cpp +++ b/src/s4u/s4u_Mailbox.cpp @@ -33,17 +33,17 @@ Mailbox* Mailbox::by_name(const std::string& name) bool Mailbox::empty() const { - return pimpl_->comm_queue_.empty(); + return pimpl_->empty(); } -unsigned int Mailbox::size() const +size_t Mailbox::size() const { - return pimpl_->comm_queue_.size(); + return pimpl_->size(); } bool Mailbox::listen() const { - return not this->empty() || (pimpl_->permanent_receiver_ && not pimpl_->done_comm_queue_.empty()); + return not pimpl_->empty() || (pimpl_->is_permanent() && pimpl_->has_some_done_comm()); } aid_t Mailbox::listen_from() const @@ -58,18 +58,18 @@ aid_t Mailbox::listen_from() const bool Mailbox::ready() const { bool comm_ready = false; - if (not pimpl_->comm_queue_.empty()) { - comm_ready = pimpl_->comm_queue_.front()->state_ == kernel::activity::State::DONE; + if (not pimpl_->empty()) { + comm_ready = pimpl_->front()->state_ == kernel::activity::State::DONE; - } else if (pimpl_->permanent_receiver_ && not pimpl_->done_comm_queue_.empty()) { - comm_ready = pimpl_->done_comm_queue_.front()->state_ == kernel::activity::State::DONE; + } else if (pimpl_->is_permanent() && pimpl_->has_some_done_comm()) { + comm_ready = pimpl_->done_front()->state_ == kernel::activity::State::DONE; } return comm_ready; } kernel::activity::CommImplPtr Mailbox::front() const { - return pimpl_->comm_queue_.empty() ? nullptr : pimpl_->comm_queue_.front(); + return pimpl_->empty() ? nullptr : pimpl_->front(); } void Mailbox::set_receiver(ActorPtr actor) @@ -80,9 +80,9 @@ void Mailbox::set_receiver(ActorPtr actor) /** @brief get the receiver (process associated to the mailbox) */ ActorPtr Mailbox::get_receiver() const { - if (pimpl_->permanent_receiver_ == nullptr) + if (pimpl_->is_permanent()) return ActorPtr(); - return pimpl_->permanent_receiver_->get_iface(); + return pimpl_->get_permanent_receiver()->get_iface(); } CommPtr Mailbox::put_init() diff --git a/src/smpi/colls/smpi_automatic_selector.cpp b/src/smpi/colls/smpi_automatic_selector.cpp index a84cd1e118..34b9abd33b 100644 --- a/src/smpi/colls/smpi_automatic_selector.cpp +++ b/src/smpi/colls/smpi_automatic_selector.cpp @@ -25,7 +25,7 @@ barrier__default(comm); \ if (TRACE_is_enabled()) { \ simgrid::instr::EventType* type = \ - simgrid::instr::Container::get_root()->type_->by_name_or_create( \ + simgrid::instr::Container::get_root()->get_type()->by_name_or_create( \ _XBT_STRINGIFY(cat)); \ \ std::string cont_name = std::string("rank-" + std::to_string(simgrid::s4u::this_actor::get_pid())); \ diff --git a/src/smpi/plugins/ampi/instr_ampi.cpp b/src/smpi/plugins/ampi/instr_ampi.cpp index d1b24a55a6..bfa4240c97 100644 --- a/src/smpi/plugins/ampi/instr_ampi.cpp +++ b/src/smpi/plugins/ampi/instr_ampi.cpp @@ -37,7 +37,7 @@ void TRACE_migration_call(aid_t pid, simgrid::instr::TIData* extra) if(smpi_process()->replaying()) {//When replaying, we register an event. smpi_container(pid)->get_state("MIGRATE_STATE")->add_entity_value(operation); - auto* type = static_cast(smpi_container(pid)->type_->by_name(operation)); + auto* type = static_cast(smpi_container(pid)->get_type()->by_name(operation)); new simgrid::instr::NewEvent(smpi_process()->simulated_elapsed(), smpi_container(pid), type, type->get_entity_value(operation)); } else {