From 5f830c1d1018751d5f6c764ed83893b3e4d600c1 Mon Sep 17 00:00:00 2001 From: Arnaud Giersch Date: Fri, 3 Jan 2020 15:34:42 +0100 Subject: [PATCH] [sonar] Make member variables "private". --- include/simgrid/smpi/replay.hpp | 16 ++- src/smpi/internals/smpi_replay.cpp | 168 ++++++++++++++---------- src/smpi/plugins/sampi_loadbalancer.cpp | 10 +- 3 files changed, 111 insertions(+), 83 deletions(-) diff --git a/include/simgrid/smpi/replay.hpp b/include/simgrid/smpi/replay.hpp index 9001d5f3cf..4da7a7ee7a 100644 --- a/include/simgrid/smpi/replay.hpp +++ b/include/simgrid/smpi/replay.hpp @@ -181,22 +181,26 @@ public: * In other words: The logic goes here, the setup is done by the ActionArgParser. */ template class ReplayAction { + const std::string name_; + const aid_t my_proc_id_ = s4u::this_actor::get_pid(); + T args_; + protected: - const std::string name; - const aid_t my_proc_id = s4u::this_actor::get_pid(); - T args; + const std::string& get_name() const { return name_; } + aid_t get_pid() const { return my_proc_id_; } + const T& get_args() const { return args_; } public: - explicit ReplayAction(const std::string& name) : name(name) {} + explicit ReplayAction(const std::string& name) : name_(name) {} virtual ~ReplayAction() = default; void execute(xbt::ReplayAction& action) { // Needs to be re-initialized for every action, hence here double start_time = smpi_process()->simulated_elapsed(); - args.parse(action, name); + args_.parse(action, name_); kernel(action); - if (name != "Init") + if (name_ != "Init") log_timed_action(action, start_time); } diff --git a/src/smpi/internals/smpi_replay.cpp b/src/smpi/internals/smpi_replay.cpp index 29e1c7c375..1cd1302092 100644 --- a/src/smpi/internals/smpi_replay.cpp +++ b/src/smpi/internals/smpi_replay.cpp @@ -411,6 +411,7 @@ void WaitAction::kernel(simgrid::xbt::ReplayAction& action) { std::string s = boost::algorithm::join(action, " "); xbt_assert(req_storage.size(), "action wait not preceded by any irecv or isend: %s", s.c_str()); + const WaitTestParser& args = get_args(); MPI_Request request = req_storage.find(args.src, args.dst, args.tag); req_storage.remove(request); @@ -425,7 +426,7 @@ void WaitAction::kernel(simgrid::xbt::ReplayAction& action) // Must be taken before Request::wait() since the request may be set to // MPI_REQUEST_NULL by Request::wait! bool is_wait_for_receive = (request->flags() & MPI_REQ_RECV); - // TODO: Here we take the rank while we normally take the process id (look for my_proc_id) + // TODO: Here we take the rank while we normally take the process id (look for get_pid()) TRACE_smpi_comm_in(rank, __func__, new simgrid::instr::WaitTIData(args.src, args.dst, args.tag)); MPI_Status status; @@ -438,57 +439,63 @@ void WaitAction::kernel(simgrid::xbt::ReplayAction& action) void SendAction::kernel(simgrid::xbt::ReplayAction&) { + const SendRecvParser& args = get_args(); int dst_traced = MPI_COMM_WORLD->group()->actor(args.partner)->get_pid(); - TRACE_smpi_comm_in(my_proc_id, __func__, new simgrid::instr::Pt2PtTIData(name, args.partner, args.size, - args.tag, Datatype::encode(args.datatype1))); + TRACE_smpi_comm_in( + get_pid(), __func__, + new simgrid::instr::Pt2PtTIData(get_name(), args.partner, args.size, args.tag, Datatype::encode(args.datatype1))); if (not TRACE_smpi_view_internals()) - TRACE_smpi_send(my_proc_id, my_proc_id, dst_traced, args.tag, args.size * args.datatype1->size()); + TRACE_smpi_send(get_pid(), get_pid(), dst_traced, args.tag, args.size * args.datatype1->size()); - if (name == "send") { + if (get_name() == "send") { Request::send(nullptr, args.size, args.datatype1, args.partner, args.tag, MPI_COMM_WORLD); - } else if (name == "isend") { + } else if (get_name() == "isend") { MPI_Request request = Request::isend(nullptr, args.size, args.datatype1, args.partner, args.tag, MPI_COMM_WORLD); req_storage.add(request); } else { - xbt_die("Don't know this action, %s", name.c_str()); + xbt_die("Don't know this action, %s", get_name().c_str()); } - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void RecvAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, __func__, new simgrid::instr::Pt2PtTIData(name, args.partner, args.size, - args.tag, Datatype::encode(args.datatype1))); + const SendRecvParser& args = get_args(); + TRACE_smpi_comm_in( + get_pid(), __func__, + new simgrid::instr::Pt2PtTIData(get_name(), args.partner, args.size, args.tag, Datatype::encode(args.datatype1))); MPI_Status status; // unknown size from the receiver point of view - if (args.size <= 0.0) { + double arg_size = args.size; + if (arg_size <= 0.0) { Request::probe(args.partner, args.tag, MPI_COMM_WORLD, &status); - args.size = status.count; + arg_size = status.count; } bool is_recv = false; // Help analyzers understanding that status is not used unintialized - if (name == "recv") { + if (get_name() == "recv") { is_recv = true; - Request::recv(nullptr, args.size, args.datatype1, args.partner, args.tag, MPI_COMM_WORLD, &status); - } else if (name == "irecv") { - MPI_Request request = Request::irecv(nullptr, args.size, args.datatype1, args.partner, args.tag, MPI_COMM_WORLD); + Request::recv(nullptr, arg_size, args.datatype1, args.partner, args.tag, MPI_COMM_WORLD, &status); + } else if (get_name() == "irecv") { + MPI_Request request = Request::irecv(nullptr, arg_size, args.datatype1, args.partner, args.tag, MPI_COMM_WORLD); req_storage.add(request); } else { THROW_IMPOSSIBLE; } - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); if (is_recv && not TRACE_smpi_view_internals()) { int src_traced = MPI_COMM_WORLD->group()->actor(status.MPI_SOURCE)->get_pid(); - TRACE_smpi_recv(src_traced, my_proc_id, args.tag); + TRACE_smpi_recv(src_traced, get_pid(), args.tag); } } void ComputeAction::kernel(simgrid::xbt::ReplayAction&) { + const ComputeParser& args = get_args(); if (smpi_cfg_simulate_computation()) { smpi_execute_flops(args.flops/smpi_adjust_comp_speed()); } @@ -496,6 +503,7 @@ void ComputeAction::kernel(simgrid::xbt::ReplayAction&) void SleepAction::kernel(simgrid::xbt::ReplayAction&) { + const SleepParser& args = get_args(); XBT_DEBUG("Sleep for: %lf secs", args.time); int rank = simgrid::s4u::this_actor::get_pid(); TRACE_smpi_sleeping_in(rank, args.time); @@ -505,18 +513,20 @@ void SleepAction::kernel(simgrid::xbt::ReplayAction&) void LocationAction::kernel(simgrid::xbt::ReplayAction&) { + const LocationParser& args = get_args(); smpi_trace_set_call_location(args.filename.c_str(), args.line); } void TestAction::kernel(simgrid::xbt::ReplayAction&) { + const WaitTestParser& args = get_args(); MPI_Request request = req_storage.find(args.src, args.dst, args.tag); req_storage.remove(request); // if request is null here, this may mean that a previous test has succeeded // Different times in traced application and replayed version may lead to this // In this case, ignore the extra calls. if (request != MPI_REQUEST_NULL) { - TRACE_smpi_comm_in(my_proc_id, __func__, new simgrid::instr::NoOpTIData("test")); + TRACE_smpi_comm_in(get_pid(), __func__, new simgrid::instr::NoOpTIData("test")); MPI_Status status; int flag = 0; @@ -530,7 +540,7 @@ void TestAction::kernel(simgrid::xbt::ReplayAction&) else req_storage.add(request); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } } @@ -554,7 +564,7 @@ void WaitAllAction::kernel(simgrid::xbt::ReplayAction&) const unsigned int count_requests = req_storage.size(); if (count_requests > 0) { - TRACE_smpi_comm_in(my_proc_id, __func__, new simgrid::instr::Pt2PtTIData("waitall", -1, count_requests, "")); + TRACE_smpi_comm_in(get_pid(), __func__, new simgrid::instr::Pt2PtTIData("waitall", -1, count_requests, "")); std::vector> sender_receiver; std::vector reqs; req_storage.get_requests(reqs); @@ -569,167 +579,181 @@ void WaitAllAction::kernel(simgrid::xbt::ReplayAction&) for (auto const& pair : sender_receiver) { TRACE_smpi_recv(pair.first, pair.second, 0); } - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } } void BarrierAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, __func__, new simgrid::instr::NoOpTIData("barrier")); + TRACE_smpi_comm_in(get_pid(), __func__, new simgrid::instr::NoOpTIData("barrier")); colls::barrier(MPI_COMM_WORLD); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void BcastAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, "action_bcast", - new simgrid::instr::CollTIData("bcast", MPI_COMM_WORLD->group()->actor(args.root)->get_pid(), - -1.0, args.size, -1, Datatype::encode(args.datatype1), "")); + const BcastArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), "action_bcast", + new simgrid::instr::CollTIData("bcast", MPI_COMM_WORLD->group()->actor(args.root)->get_pid(), -1.0, + args.size, -1, Datatype::encode(args.datatype1), "")); colls::bcast(send_buffer(args.size * args.datatype1->size()), args.size, args.datatype1, args.root, MPI_COMM_WORLD); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void ReduceAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, "action_reduce", - new simgrid::instr::CollTIData("reduce", MPI_COMM_WORLD->group()->actor(args.root)->get_pid(), - args.comp_size, args.comm_size, -1, - Datatype::encode(args.datatype1), "")); + const ReduceArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), "action_reduce", + new simgrid::instr::CollTIData("reduce", MPI_COMM_WORLD->group()->actor(args.root)->get_pid(), + args.comp_size, args.comm_size, -1, + Datatype::encode(args.datatype1), "")); colls::reduce(send_buffer(args.comm_size * args.datatype1->size()), recv_buffer(args.comm_size * args.datatype1->size()), args.comm_size, args.datatype1, MPI_OP_NULL, args.root, MPI_COMM_WORLD); private_execute_flops(args.comp_size); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void AllReduceAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, "action_allreduce", new simgrid::instr::CollTIData("allreduce", -1, args.comp_size, args.comm_size, -1, - Datatype::encode(args.datatype1), "")); + const AllReduceArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), "action_allreduce", + new simgrid::instr::CollTIData("allreduce", -1, args.comp_size, args.comm_size, -1, + Datatype::encode(args.datatype1), "")); colls::allreduce(send_buffer(args.comm_size * args.datatype1->size()), recv_buffer(args.comm_size * args.datatype1->size()), args.comm_size, args.datatype1, MPI_OP_NULL, MPI_COMM_WORLD); private_execute_flops(args.comp_size); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void AllToAllAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, "action_alltoall", - new simgrid::instr::CollTIData("alltoall", -1, -1.0, args.send_size, args.recv_size, - Datatype::encode(args.datatype1), - Datatype::encode(args.datatype2))); + const AllToAllArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), "action_alltoall", + new simgrid::instr::CollTIData("alltoall", -1, -1.0, args.send_size, args.recv_size, + Datatype::encode(args.datatype1), + Datatype::encode(args.datatype2))); colls::alltoall(send_buffer(args.send_size * args.comm_size * args.datatype1->size()), args.send_size, args.datatype1, recv_buffer(args.recv_size * args.comm_size * args.datatype2->size()), args.recv_size, args.datatype2, MPI_COMM_WORLD); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void GatherAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, name.c_str(), new simgrid::instr::CollTIData(name, (name == "gather") ? args.root : -1, -1.0, args.send_size, args.recv_size, - Datatype::encode(args.datatype1), Datatype::encode(args.datatype2))); + const GatherArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), get_name().c_str(), + new simgrid::instr::CollTIData(get_name(), (get_name() == "gather") ? args.root : -1, -1.0, + args.send_size, args.recv_size, Datatype::encode(args.datatype1), + Datatype::encode(args.datatype2))); - if (name == "gather") { + if (get_name() == "gather") { int rank = MPI_COMM_WORLD->rank(); colls::gather(send_buffer(args.send_size * args.datatype1->size()), args.send_size, args.datatype1, (rank == args.root) ? recv_buffer(args.recv_size * args.comm_size * args.datatype2->size()) : nullptr, args.recv_size, args.datatype2, args.root, MPI_COMM_WORLD); - } - else + } else colls::allgather(send_buffer(args.send_size * args.datatype1->size()), args.send_size, args.datatype1, recv_buffer(args.recv_size * args.datatype2->size()), args.recv_size, args.datatype2, MPI_COMM_WORLD); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void GatherVAction::kernel(simgrid::xbt::ReplayAction&) { int rank = MPI_COMM_WORLD->rank(); + const GatherVArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), get_name().c_str(), + new simgrid::instr::VarCollTIData( + get_name(), (get_name() == "gatherv") ? args.root : -1, args.send_size, nullptr, -1, + args.recvcounts, Datatype::encode(args.datatype1), Datatype::encode(args.datatype2))); - TRACE_smpi_comm_in(my_proc_id, name.c_str(), new simgrid::instr::VarCollTIData( - name, (name == "gatherv") ? args.root : -1, args.send_size, nullptr, -1, args.recvcounts, - Datatype::encode(args.datatype1), Datatype::encode(args.datatype2))); - - if (name == "gatherv") { + if (get_name() == "gatherv") { colls::gatherv(send_buffer(args.send_size * args.datatype1->size()), args.send_size, args.datatype1, (rank == args.root) ? recv_buffer(args.recv_size_sum * args.datatype2->size()) : nullptr, args.recvcounts->data(), args.disps.data(), args.datatype2, args.root, MPI_COMM_WORLD); - } - else { + } else { colls::allgatherv(send_buffer(args.send_size * args.datatype1->size()), args.send_size, args.datatype1, recv_buffer(args.recv_size_sum * args.datatype2->size()), args.recvcounts->data(), args.disps.data(), args.datatype2, MPI_COMM_WORLD); } - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void ScatterAction::kernel(simgrid::xbt::ReplayAction&) { int rank = MPI_COMM_WORLD->rank(); - TRACE_smpi_comm_in(my_proc_id, "action_scatter", new simgrid::instr::CollTIData(name, args.root, -1.0, args.send_size, args.recv_size, - Datatype::encode(args.datatype1), - Datatype::encode(args.datatype2))); + const ScatterArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), "action_scatter", + new simgrid::instr::CollTIData(get_name(), args.root, -1.0, args.send_size, args.recv_size, + Datatype::encode(args.datatype1), + Datatype::encode(args.datatype2))); colls::scatter(send_buffer(args.send_size * args.datatype1->size()), args.send_size, args.datatype1, (rank == args.root) ? recv_buffer(args.recv_size * args.datatype2->size()) : nullptr, args.recv_size, args.datatype2, args.root, MPI_COMM_WORLD); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void ScatterVAction::kernel(simgrid::xbt::ReplayAction&) { int rank = MPI_COMM_WORLD->rank(); - TRACE_smpi_comm_in(my_proc_id, "action_scatterv", new simgrid::instr::VarCollTIData(name, args.root, -1, args.sendcounts, args.recv_size, - nullptr, Datatype::encode(args.datatype1), - Datatype::encode(args.datatype2))); + const ScatterVArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), "action_scatterv", + new simgrid::instr::VarCollTIData(get_name(), args.root, -1, args.sendcounts, args.recv_size, + nullptr, Datatype::encode(args.datatype1), + Datatype::encode(args.datatype2))); colls::scatterv((rank == args.root) ? send_buffer(args.send_size_sum * args.datatype1->size()) : nullptr, args.sendcounts->data(), args.disps.data(), args.datatype1, recv_buffer(args.recv_size * args.datatype2->size()), args.recv_size, args.datatype2, args.root, MPI_COMM_WORLD); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void ReduceScatterAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, "action_reducescatter", + const ReduceScatterArgParser& args = get_args(); + TRACE_smpi_comm_in( + get_pid(), "action_reducescatter", new simgrid::instr::VarCollTIData("reducescatter", -1, 0, nullptr, -1, args.recvcounts, - std::to_string(args.comp_size), /* ugly hack to print comp_size */ - Datatype::encode(args.datatype1))); + std::to_string(args.comp_size), /* ugly hack to print comp_size */ + Datatype::encode(args.datatype1))); colls::reduce_scatter(send_buffer(args.recv_size_sum * args.datatype1->size()), recv_buffer(args.recv_size_sum * args.datatype1->size()), args.recvcounts->data(), args.datatype1, MPI_OP_NULL, MPI_COMM_WORLD); private_execute_flops(args.comp_size); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } void AllToAllVAction::kernel(simgrid::xbt::ReplayAction&) { - TRACE_smpi_comm_in(my_proc_id, __func__, - new simgrid::instr::VarCollTIData( - "alltoallv", -1, args.send_size_sum, args.sendcounts, args.recv_size_sum, args.recvcounts, - Datatype::encode(args.datatype1), Datatype::encode(args.datatype2))); + const AllToAllVArgParser& args = get_args(); + TRACE_smpi_comm_in(get_pid(), __func__, + new simgrid::instr::VarCollTIData( + "alltoallv", -1, args.send_size_sum, args.sendcounts, args.recv_size_sum, args.recvcounts, + Datatype::encode(args.datatype1), Datatype::encode(args.datatype2))); colls::alltoallv(send_buffer(args.send_buf_size * args.datatype1->size()), args.sendcounts->data(), args.senddisps.data(), args.datatype1, recv_buffer(args.recv_buf_size * args.datatype2->size()), args.recvcounts->data(), args.recvdisps.data(), args.datatype2, MPI_COMM_WORLD); - TRACE_smpi_comm_out(my_proc_id); + TRACE_smpi_comm_out(get_pid()); } } // Replay Namespace }} // namespace simgrid::smpi diff --git a/src/smpi/plugins/sampi_loadbalancer.cpp b/src/smpi/plugins/sampi_loadbalancer.cpp index 4ae513fb23..be546b33fe 100644 --- a/src/smpi/plugins/sampi_loadbalancer.cpp +++ b/src/smpi/plugins/sampi_loadbalancer.cpp @@ -54,7 +54,7 @@ public: simgrid::s4u::Host* cur_host = simgrid::s4u::this_actor::get_host(); simgrid::s4u::Host* migrate_to_host; - TRACE_migration_call(my_proc_id, nullptr); + TRACE_migration_call(get_pid(), nullptr); // We only migrate every "cfg_migration_frequency"-times, not at every call migration_call_counter[simgrid::s4u::Actor::self()]++; @@ -68,7 +68,7 @@ public: static bool was_executed = false; if (not was_executed) { was_executed = true; - XBT_DEBUG("Process %li runs the load balancer", my_proc_id); + XBT_DEBUG("Process %li runs the load balancer", get_pid()); smpi_bench_begin(); lb.run(); smpi_bench_end(); @@ -82,7 +82,7 @@ public: if (cur_host != migrate_to_host) { // Origin and dest are not the same -> migrate std::vector migration_hosts = {cur_host, migrate_to_host}; std::vector comp_amount = {0, 0}; - std::vector comm_amount = {0, /*must not be 0*/ std::max(args.memory_consumption, 1.0), 0, 0}; + std::vector comm_amount = {0, /*must not be 0*/ std::max(get_args().memory_consumption, 1.0), 0, 0}; xbt_os_timer_t timer = smpi_process()->timer(); xbt_os_threadtimer_start(timer); @@ -91,8 +91,8 @@ public: smpi_execute(xbt_os_timer_elapsed(timer)); // Update the process and host mapping in SimGrid. - XBT_DEBUG("Migrating process %li from %s to %s", my_proc_id, cur_host->get_cname(), migrate_to_host->get_cname()); - TRACE_smpi_process_change_host(my_proc_id, migrate_to_host); + XBT_DEBUG("Migrating process %li from %s to %s", get_pid(), cur_host->get_cname(), migrate_to_host->get_cname()); + TRACE_smpi_process_change_host(get_pid(), migrate_to_host); simgrid::s4u::this_actor::set_host(migrate_to_host); } -- 2.20.1