From f52f951d51f3b4aea203b89b8081f43891727de0 Mon Sep 17 00:00:00 2001 From: Sean McGovern Date: Tue, 18 Oct 2022 02:54:09 +0000 Subject: [PATCH 1/3] #1932: use member variable instead of some pointer dereferencing in ActiveMessenger --- src/vt/messaging/active.cc | 20 +++++++++----------- src/vt/messaging/active.impl.h | 2 +- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/vt/messaging/active.cc b/src/vt/messaging/active.cc index aeafcddbcb..63ef24da3e 100644 --- a/src/vt/messaging/active.cc +++ b/src/vt/messaging/active.cc @@ -151,9 +151,8 @@ void ActiveMessenger::initialize() { } void ActiveMessenger::startup() { - auto const this_node = theContext()->getNode(); bare_handler_dummy_elm_id_for_lb_data_ = - elm::ElmIDBits::createBareHandler(this_node); + elm::ElmIDBits::createBareHandler(this_node_); #if vt_check_enabled(lblite) // Hook to collect LB data about objgroups @@ -225,7 +224,7 @@ EventType ActiveMessenger::sendMsgBytesWithPut( } vtWarnIf( - dest == theContext()->getNode() && + dest == this_node_ && not is_bcast && not theConfig()->vt_lb_self_migration, fmt::format("Destination {} should != this node", dest) @@ -375,7 +374,6 @@ EventType ActiveMessenger::sendMsgMPI( "sendMsgMPI: (multi): size={}\n", msg_size ); auto tag = allocateNewTag(); - auto this_node = theContext()->getNode(); // Send the actual data in multiple chunks PtrLenPairType tup = std::make_tuple(untyped_msg, msg_size); @@ -387,7 +385,7 @@ EventType ActiveMessenger::sendMsgMPI( mpi_event->setManagedMessage(base.to()); // Send the control message to receive the multiple chunks of data - auto m = makeMessage(info, this_node, msg_size); + auto m = makeMessage(info, this_node_, msg_size); sendMsg(dest, m); return event_id; @@ -491,9 +489,8 @@ EventType ActiveMessenger::doMessageSend( ); // Don't go through MPI with self-send, schedule the message locally instead - auto const this_node = theContext()->getNode(); if (deliver) { - if (dest != this_node) { + if (dest != this_node_) { sendMsgBytesWithPut(dest, base, send_tag); } else { recordLBDataCommForSend(dest, base, base.size()); @@ -611,7 +608,7 @@ std::tuple ActiveMessenger::sendDataMPI( } if (events.size() > 1) { - ret_event = theEvent()->createParentEvent(theContext()->getNode()); + ret_event = theEvent()->createParentEvent(this_node_); auto& holder = theEvent()->getEventHolder(ret_event); for (auto&& child_event : events) { holder.get_event()->addEventToList(child_event); @@ -864,8 +861,9 @@ void ActiveMessenger::recordLBDataCommForSend( NodeType const dest, MsgSharedPtr const& base, MsgSizeType const msg_size ) { - if (theContext()->getTask() != nullptr) { - auto lb = theContext()->getTask()->get(); + auto the_task = theContext()->getTask(); + if (the_task != nullptr) { + auto lb = the_task->get(); if (lb) { auto const& msg = base.get(); @@ -874,7 +872,7 @@ void ActiveMessenger::recordLBDataCommForSend( if (not already_recorded) { auto dest_elm_id = elm::ElmIDBits::createBareHandler(dest); - theContext()->getTask()->send(dest_elm_id, msg_size); + the_task->send(dest_elm_id, msg_size); } } } diff --git a/src/vt/messaging/active.impl.h b/src/vt/messaging/active.impl.h index 705dc17a4e..a955bb8cb0 100644 --- a/src/vt/messaging/active.impl.h +++ b/src/vt/messaging/active.impl.h @@ -169,7 +169,7 @@ ActiveMessenger::PendingSendType ActiveMessenger::sendMsgCopyableImpl( } if (is_bcast) { - dest = theContext()->getNode(); + dest = this_node_; } if (tag != no_tag) { envelopeSetTag(rawMsg->env, tag); From dacf6001f8d8eb4ae7403448731f298e656a9247 Mon Sep 17 00:00:00 2001 From: Sean McGovern Date: Tue, 18 Oct 2022 02:55:31 +0000 Subject: [PATCH 2/3] #1932: use member variable instead of dereferencing pointer in TerminationDetector --- src/vt/termination/termination.cc | 23 +++++++++-------------- src/vt/termination/termination.impl.h | 2 +- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/vt/termination/termination.cc b/src/vt/termination/termination.cc index 4d3eb497fb..d0af8168c4 100644 --- a/src/vt/termination/termination.cc +++ b/src/vt/termination/termination.cc @@ -154,12 +154,11 @@ TerminationDetector::getDSTerm(EpochType epoch, bool is_root) { if (isDS(epoch)) { auto iter = term_.find(epoch); if (iter == term_.end()) { - auto const this_node = theContext()->getNode(); term_.emplace( std::piecewise_construct, std::forward_as_tuple(epoch), std::forward_as_tuple( - TerminatorType{epoch,is_root,this_node} + TerminatorType{epoch,is_root,this_node_} ) ); iter = term_.find(epoch); @@ -278,11 +277,11 @@ std::shared_ptr TerminationDetector::makeGraph( auto root = std::make_shared(any_epoch_state_.getEpoch(), glabel); // Collect non-rooted epochs, just collective, excluding DS or other rooted // epochs (info about them is localized on the creation node) - auto const this_node = theContext()->getNode(); + for (auto const& elm : epoch_state_) { auto const ep = elm.first; bool const rooted = epoch::EpochManip::isRooted(ep); - if (not rooted or (rooted and epoch::EpochManip::node(ep) == this_node)) { + if (not rooted or (rooted and epoch::EpochManip::node(ep) == this_node_)) { if (not isEpochTerminated(elm.first)) { auto label = elm.second.getLabel(); live_epochs[ep] = std::make_shared(ep, label); @@ -292,7 +291,7 @@ std::shared_ptr TerminationDetector::makeGraph( for (auto const& elm : term_) { // Only include DS epochs that are created here. Other nodes do not have // proper successor info about the rooted, DS epochs - if (epoch::EpochManip::node(elm.first) == this_node) { + if (epoch::EpochManip::node(elm.first) == this_node_) { if (not isEpochTerminated(elm.first)) { auto label = elm.second.getLabel(); live_epochs[elm.first] = std::make_shared( @@ -668,13 +667,12 @@ void TerminationDetector::epochTerminated(EpochType const& epoch, CallFromEnum f // Matching consume on global epoch once a nested epoch terminates if (epoch != any_epoch_sentinel) { - auto const this_node = theContext()->getNode(); bool const is_rooted = isRooted(epoch); bool const is_ds = isDS(epoch); if ( not is_rooted or is_ds or - (is_rooted and epoch::EpochManip::node(epoch) == this_node) + (is_rooted and epoch::EpochManip::node(epoch) == this_node_) ) { consumeOnGlobal(epoch); } @@ -686,10 +684,9 @@ void TerminationDetector::inquireTerminated( ) { auto const& is_rooted = epoch::EpochManip::isRooted(epoch); auto const& epoch_root_node = epoch::EpochManip::node(epoch); - auto const& this_node = theContext()->getNode(); vtAssertInfo( - !is_rooted || epoch_root_node == this_node, + !is_rooted || epoch_root_node == this_node_, "Must be not rooted or this is root node", is_rooted, epoch_root_node, epoch, from ); @@ -757,9 +754,8 @@ TermStatusEnum TerminationDetector::testEpochTerminated(EpochType epoch) { if (theEpoch()->getTerminatedWindow(epoch)->isTerminated(epoch)) { status = TermStatusEnum::Terminated; } else if (is_rooted_epoch) { - auto const& this_node = theContext()->getNode(); auto const& root = epoch::EpochManip::node(epoch); - if (root == this_node) { + if (root == this_node_) { /* * The idea here is that if this is executed on the root, it must have * valid info on whether the rooted live or terminated @@ -776,7 +772,7 @@ TermStatusEnum TerminationDetector::testEpochTerminated(EpochType epoch) { * Send a message to the root node to find out whether this epoch is * terminated or not */ - auto msg = makeMessage(epoch,this_node); + auto msg = makeMessage(epoch,this_node_); theMsg()->sendMsg(root, msg); epoch_wait_status_.insert(epoch); } @@ -1123,8 +1119,7 @@ void TerminationDetector::activateEpoch(EpochType const& epoch) { "activateEpoch: epoch={:x}\n", epoch ); - auto const this_node = theContext()->getNode(); - if (isRooted(epoch) and epoch::EpochManip::node(epoch) == this_node) { + if (isRooted(epoch) and epoch::EpochManip::node(epoch) == this_node_) { produceOnGlobal(epoch); } diff --git a/src/vt/termination/termination.impl.h b/src/vt/termination/termination.impl.h index f9c2b4730e..1d5a3db7f7 100644 --- a/src/vt/termination/termination.impl.h +++ b/src/vt/termination/termination.impl.h @@ -120,7 +120,7 @@ inline void TerminationDetector::produceConsume( // If a node is not passed, use the current node (self-prod/cons) if (node == uninitialized_destination) { - node = this_node_; + node = this_node_; } if (produce) { From af8d214da56db48d74e0d64d0f85cedb89c5d404 Mon Sep 17 00:00:00 2001 From: Sean McGovern Date: Tue, 18 Oct 2022 03:00:21 +0000 Subject: [PATCH 3/3] #1932: fix indentation --- src/vt/termination/termination.impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vt/termination/termination.impl.h b/src/vt/termination/termination.impl.h index 1d5a3db7f7..219ed4c44c 100644 --- a/src/vt/termination/termination.impl.h +++ b/src/vt/termination/termination.impl.h @@ -120,7 +120,7 @@ inline void TerminationDetector::produceConsume( // If a node is not passed, use the current node (self-prod/cons) if (node == uninitialized_destination) { - node = this_node_; + node = this_node_; } if (produce) {