From 6e0814e4b79b806d4334e24adbde33c979a1f738 Mon Sep 17 00:00:00 2001 From: Jonathan Lifflander Date: Mon, 24 Oct 2022 16:00:43 -0700 Subject: [PATCH] #1983: dispatch: fix bugs in message deletion --- src/vt/objgroup/dispatch/dispatch.h | 79 --------------------- src/vt/objgroup/dispatch/dispatch.impl.h | 76 -------------------- src/vt/objgroup/dispatch/dispatch_base.h | 88 ------------------------ src/vt/objgroup/manager.cc | 44 ++---------- src/vt/objgroup/manager.fwd.h | 7 +- src/vt/objgroup/manager.h | 46 +------------ src/vt/objgroup/manager.impl.h | 25 +------ src/vt/objgroup/manager.static.h | 70 +++++++++++++++++++ 8 files changed, 85 insertions(+), 350 deletions(-) delete mode 100644 src/vt/objgroup/dispatch/dispatch.h delete mode 100644 src/vt/objgroup/dispatch/dispatch.impl.h delete mode 100644 src/vt/objgroup/dispatch/dispatch_base.h diff --git a/src/vt/objgroup/dispatch/dispatch.h b/src/vt/objgroup/dispatch/dispatch.h deleted file mode 100644 index c2fae7b93b..0000000000 --- a/src/vt/objgroup/dispatch/dispatch.h +++ /dev/null @@ -1,79 +0,0 @@ -/* -//@HEADER -// ***************************************************************************** -// -// dispatch.h -// DARMA/vt => Virtual Transport -// -// Copyright 2019-2021 National Technology & Engineering Solutions of Sandia, LLC -// (NTESS). Under the terms of Contract DE-NA0003525 with NTESS, the U.S. -// Government retains certain rights in this software. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: -// -// * Redistributions of source code must retain the above copyright notice, -// this list of conditions and the following disclaimer. -// -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// -// * Neither the name of the copyright holder nor the names of its -// contributors may be used to endorse or promote products derived from this -// software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. -// -// Questions? Contact darma@sandia.gov -// -// ***************************************************************************** -//@HEADER -*/ - -#if !defined INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_H -#define INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_H - -#include "vt/config.h" -#include "vt/objgroup/common.h" -#include "vt/objgroup/dispatch/dispatch_base.h" -#include "vt/messaging/message/smart_ptr.h" - -namespace vt { namespace objgroup { namespace dispatch { - -template -struct Dispatch final : DispatchBase { - Dispatch() = delete; - Dispatch(Dispatch const&) = delete; - Dispatch(Dispatch&&) = default; - - Dispatch(ObjGroupProxyType in_proxy, ObjT* in_obj) - : DispatchBase(in_proxy), obj_(in_obj) - { } - - virtual ~Dispatch() = default; - - void run( - HandlerType han, BaseMessage* msg, NodeType from_node, ActionType cont - ) override; - void* objPtr() const override { return obj_; } - -private: - ObjT* obj_ = nullptr; -}; - -}}} /* end namespace vt::objgroup::dispatch */ - -#include "vt/objgroup/dispatch/dispatch.impl.h" - -#endif /*INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_H*/ diff --git a/src/vt/objgroup/dispatch/dispatch.impl.h b/src/vt/objgroup/dispatch/dispatch.impl.h deleted file mode 100644 index 0f7a9143c9..0000000000 --- a/src/vt/objgroup/dispatch/dispatch.impl.h +++ /dev/null @@ -1,76 +0,0 @@ -/* -//@HEADER -// ***************************************************************************** -// -// dispatch.impl.h -// DARMA/vt => Virtual Transport -// -// Copyright 2019-2021 National Technology & Engineering Solutions of Sandia, LLC -// (NTESS). Under the terms of Contract DE-NA0003525 with NTESS, the U.S. -// Government retains certain rights in this software. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: -// -// * Redistributions of source code must retain the above copyright notice, -// this list of conditions and the following disclaimer. -// -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// -// * Neither the name of the copyright holder nor the names of its -// contributors may be used to endorse or promote products derived from this -// software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. -// -// Questions? Contact darma@sandia.gov -// -// ***************************************************************************** -//@HEADER -*/ - -#if !defined INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_IMPL_H -#define INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_IMPL_H - -#include "vt/config.h" -#include "vt/objgroup/common.h" -#include "vt/registry/auto/auto_registry.h" -#include "vt/objgroup/holder/holder_base.h" - -namespace vt { namespace objgroup { namespace dispatch { - -template -void Dispatch::run( - HandlerType han, BaseMessage* msg, NodeType from_node, ActionType cont -) { - vtAssert(obj_ != nullptr, "Must have a valid object"); - - auto tmsg = static_cast(msg); - auto m = promoteMsg(tmsg); - auto holder = detail::getHolderBase(han); - auto const& elm_id = holder->getElmID(); - auto lb_data = &holder->getLBData(); - - runnable::makeRunnable(m, true, han, from_node) - .withContinuation(cont) - .withObjGroup(obj_) - .withLBData(lb_data, elm_id) - .withTDEpochFromMsg() - .enqueue(); -} - -}}} /* end namespace vt::objgroup::dispatch */ - -#endif /*INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_IMPL_H*/ diff --git a/src/vt/objgroup/dispatch/dispatch_base.h b/src/vt/objgroup/dispatch/dispatch_base.h deleted file mode 100644 index 142a55d0e9..0000000000 --- a/src/vt/objgroup/dispatch/dispatch_base.h +++ /dev/null @@ -1,88 +0,0 @@ -/* -//@HEADER -// ***************************************************************************** -// -// dispatch_base.h -// DARMA/vt => Virtual Transport -// -// Copyright 2019-2021 National Technology & Engineering Solutions of Sandia, LLC -// (NTESS). Under the terms of Contract DE-NA0003525 with NTESS, the U.S. -// Government retains certain rights in this software. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: -// -// * Redistributions of source code must retain the above copyright notice, -// this list of conditions and the following disclaimer. -// -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// -// * Neither the name of the copyright holder nor the names of its -// contributors may be used to endorse or promote products derived from this -// software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. -// -// Questions? Contact darma@sandia.gov -// -// ***************************************************************************** -//@HEADER -*/ - -#if !defined INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_BASE_H -#define INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_BASE_H - -#include "vt/config.h" -#include "vt/objgroup/common.h" -#include "vt/messaging/message/smart_ptr.h" - -namespace vt { namespace objgroup { namespace dispatch { - -/* - * DispatchBase implements type erasure to dispatch to a obj group without - * encoding the message directly in the message (as an alternative to using a - * std::function) - */ - -struct DispatchBase { - explicit DispatchBase(ObjGroupProxyType in_proxy) - : proxy_(in_proxy) - { } - - virtual ~DispatchBase() = default; - - /* - * Dispatch to the handler; the base is closed around the proper object - * pointer that is type-erased here - */ - virtual void run( - HandlerType han, BaseMessage* msg, NodeType from_node, ActionType cont - ) = 0; - virtual void* objPtr() const = 0; - - ObjGroupProxyType proxy() const { return proxy_; } - - template - void serialize(Serializer& s) { - s | proxy_; - } - -private: - ObjGroupProxyType proxy_ = no_obj_group; -}; - -}}} /* end namespace vt::objgroup::dispatch */ - -#endif /*INCLUDED_VT_OBJGROUP_DISPATCH_DISPATCH_BASE_H*/ diff --git a/src/vt/objgroup/manager.cc b/src/vt/objgroup/manager.cc index 35109cef1f..18888093dc 100644 --- a/src/vt/objgroup/manager.cc +++ b/src/vt/objgroup/manager.cc @@ -83,35 +83,6 @@ ObjGroupProxyType ObjGroupManager::getProxy(ObjGroupProxyType proxy) { return proxy; } -void ObjGroupManager::dispatch( - MsgSharedPtr msg, HandlerType han, NodeType from_node, - ActionType cont -) { - // Extract the control-bit sequence from the handler - auto const ctrl = HandlerManager::getHandlerControl(han); - vt_debug_print( - verbose, objgroup, - "dispatch: ctrl={:x}, han={:x}\n", ctrl, han - ); - auto const node = 0; - auto const proxy = proxy::ObjGroupProxy::create(ctrl, node, true); - auto dispatch_iter = dispatch_.find(proxy); - vt_debug_print( - normal, objgroup, - "dispatch: try ctrl={:x}, han={:x}, has dispatch={}\n", - ctrl, han, dispatch_iter != dispatch_.end() - ); - if (dispatch_iter == dispatch_.end()) { - auto const epoch = envelopeGetEpoch(msg->env); - if (epoch != no_epoch and epoch != term::any_epoch_sentinel) { - theTerm()->produce(epoch); - } - pending_[proxy].emplace_back(msg, from_node, cont, han); - } else { - dispatch_iter->second->run(han, msg.get(), from_node, cont); - } -} - ObjGroupProxyType ObjGroupManager::makeCollectiveImpl( std::string const& label, HolderBasePtrType base, void* obj_ptr ) { @@ -166,15 +137,12 @@ elm::ElementIDStruct ObjGroupManager::getNextElm(ObjGroupProxyType proxy) { } } -void dispatchObjGroup( - MsgSharedPtr msg, HandlerType han, NodeType from_node, - ActionType cont -) { - vt_debug_print( - verbose, objgroup, - "dispatchObjGroup: han={:x}\n", han - ); - return theObjGroup()->dispatch(msg, han, from_node, cont); +std::unordered_map>& getObjs() { + return theObjGroup()->objs_; +} + +std::unordered_map>& getPending() { + return theObjGroup()->pending_; } }} /* end namespace vt::objgroup */ diff --git a/src/vt/objgroup/manager.fwd.h b/src/vt/objgroup/manager.fwd.h index 2bd37cfdee..c95045240c 100644 --- a/src/vt/objgroup/manager.fwd.h +++ b/src/vt/objgroup/manager.fwd.h @@ -60,11 +60,14 @@ namespace detail { holder::HolderBase* getHolderBase(HandlerType handler); } /* end namespace detail */ +template void dispatchObjGroup( - MsgSharedPtr msg, HandlerType han, NodeType from_node, - ActionType cont + MsgSharedPtr msg, HandlerType han, NodeType from_node, ActionType cont ); +std::unordered_map>& getObjs(); +std::unordered_map>& getPending(); + template messaging::PendingSend send(MsgSharedPtr msg, HandlerType han, NodeType node); template diff --git a/src/vt/objgroup/manager.h b/src/vt/objgroup/manager.h index 65c7aa31c7..571d258593 100644 --- a/src/vt/objgroup/manager.h +++ b/src/vt/objgroup/manager.h @@ -52,7 +52,6 @@ #include "vt/objgroup/holder/holder.h" #include "vt/objgroup/holder/holder_user.h" #include "vt/objgroup/holder/holder_basic.h" -#include "vt/objgroup/dispatch/dispatch.h" #include "vt/messaging/message/message.h" #include "vt/messaging/message/smart_ptr.h" #include "vt/messaging/pending_send.h" @@ -89,31 +88,9 @@ struct ObjGroupManager : runtime::component::Component { using MakeFnType = std::function()>; using HolderBaseType = holder::HolderBase; using HolderBasePtrType = std::unique_ptr; - using DispatchBaseType = dispatch::DispatchBase; - using DispatchBasePtrType = std::unique_ptr; using PendingSendType = messaging::PendingSend; -private: - struct PendingRecv { - PendingRecv( - MsgSharedPtr in_msg, NodeType in_from_node, - ActionType in_cont, HandlerType in_han - ) : msg_(in_msg), - from_node_(in_from_node), - cont_(in_cont), - han_(in_han) - { } - - MsgSharedPtr msg_; - NodeType from_node_ = uninitialized_destination; - ActionType cont_ = nullptr; - HandlerType han_ = uninitialized_handler; - }; - public: - using MsgContainerType = std::vector; - - /** * \internal \brief Construct the ObjGroupManager */ @@ -350,22 +327,6 @@ struct ObjGroupManager : runtime::component::Component { template std::string getLabel(ProxyType proxy) const; - /* - * Dispatch to a live obj group pointer with a handler - */ - /** - * \internal \brief Dispatch message to objgroup - * - * \param[in] msg the message - * \param[in] han the handler to invoke - * \param[in] from_node the node it was from - * \param[in] cont optional continuation to execute after - */ - void dispatch( - MsgSharedPtr msg, HandlerType han, NodeType from_node, - ActionType cont - ); - /** * \internal \brief Send a message to an objgroup * @@ -408,7 +369,6 @@ struct ObjGroupManager : runtime::component::Component { template void serialize(SerializerT& s) { s | cur_obj_id_ - | dispatch_ | objs_ | obj_to_proxy_ | pending_ @@ -417,6 +377,8 @@ struct ObjGroupManager : runtime::component::Component { // Friend function to access the holder without including this header file friend holder::HolderBase* detail::getHolderBase(HandlerType handler); + friend std::unordered_map& getObjs(); + friend std::unordered_map>& getPending(); private: /** @@ -476,14 +438,12 @@ struct ObjGroupManager : runtime::component::Component { private: /// The current obj ID, sequential on each node for collective construction ObjGroupIDType cur_obj_id_ = fst_obj_group_id; - /// Function to dispatch to the base class for type-erasure to run handler - std::unordered_map dispatch_; /// Type-erased pointers to the objects held on this node std::unordered_map objs_; /// Reverse lookup map from an object pointer to the proxy std::unordered_map obj_to_proxy_; /// Messages that are pending creation for delivery - std::unordered_map pending_; + std::unordered_map> pending_; /// Map of object groups' labels std::unordered_map labels_; }; diff --git a/src/vt/objgroup/manager.impl.h b/src/vt/objgroup/manager.impl.h index 75637bfbba..dc38c39239 100644 --- a/src/vt/objgroup/manager.impl.h +++ b/src/vt/objgroup/manager.impl.h @@ -52,7 +52,6 @@ #include "vt/objgroup/holder/holder.h" #include "vt/objgroup/holder/holder_user.h" #include "vt/objgroup/holder/holder_basic.h" -#include "vt/objgroup/dispatch/dispatch.h" #include "vt/objgroup/type_registry/registry.h" #include "vt/registry/auto/auto_registry.h" #include "vt/collective/collective_alg.h" @@ -130,15 +129,6 @@ ObjGroupManager::makeCollective(MakeFnType fn, std::string const& label) { template void ObjGroupManager::destroyCollective(ProxyType proxy) { auto const proxy_bits = proxy.getProxy(); - auto iter = dispatch_.find(proxy_bits); - if (iter != dispatch_.end()) { - auto ptr = iter->second->objPtr(); - auto obj_iter = obj_to_proxy_.find(ptr); - if (obj_iter != obj_to_proxy_.end()) { - obj_to_proxy_.erase(obj_iter); - } - dispatch_.erase(iter); - } auto obj_iter = objs_.find(proxy_bits); if (obj_iter != objs_.end()) { objs_.erase(obj_iter); @@ -152,28 +142,15 @@ void ObjGroupManager::destroyCollective(ProxyType proxy) { template void ObjGroupManager::regObjProxy(ObjT* obj, ObjGroupProxyType proxy) { - auto iter = dispatch_.find(proxy); - vtAssertExpr(iter == dispatch_.end()); vt_debug_print( normal, objgroup, "regObjProxy: obj={}, proxy={:x}\n", print_ptr(obj), proxy ); - DispatchBasePtrType b = std::make_unique>(proxy,obj); - dispatch_.emplace( - std::piecewise_construct, - std::forward_as_tuple(proxy), - std::forward_as_tuple(std::move(b)) - ); auto pending_iter = pending_.find(proxy); if (pending_iter != pending_.end()) { for (auto&& pending : pending_iter->second) { - auto const& msg = pending.msg_; - auto const epoch = envelopeGetEpoch(msg->env); - dispatch(msg, pending.han_, pending.from_node_, pending.cont_); - if (epoch != no_epoch) { - theTerm()->consume(epoch); - } + pending(); } pending_.erase(pending_iter); } diff --git a/src/vt/objgroup/manager.static.h b/src/vt/objgroup/manager.static.h index 8f6f46f0e5..c10694c822 100644 --- a/src/vt/objgroup/manager.static.h +++ b/src/vt/objgroup/manager.static.h @@ -46,6 +46,7 @@ #include "vt/config.h" #include "vt/objgroup/common.h" +#include "vt/objgroup/proxy/proxy_bits.h" #include "vt/objgroup/holder/holder_base.h" #include "vt/messaging/active.h" #include "vt/runnable/make_runnable.h" @@ -102,6 +103,75 @@ messaging::PendingSend broadcast(MsgSharedPtr msg, HandlerType han) { return theMsg()->broadcastMsg(han, msg); } +namespace detail { + +template +void dispatchImpl( + MsgSharedPtr const& msg, HandlerType han, NodeType from_node, + ActionType cont, ObjT* obj +) { + auto holder = detail::getHolderBase(han); + auto const& elm_id = holder->getElmID(); + auto lb_data = &holder->getLBData(); + runnable::makeRunnable(msg, true, han, from_node) + .withContinuation(cont) + .withObjGroup(obj) + .withLBData(lb_data, elm_id) + .withTDEpochFromMsg() + .enqueue(); +} + +template +void dispatch( + MsgSharedPtr msg, HandlerType han, NodeType from_node, + ActionType cont +) { + // Extract the control-bit sequence from the handler + auto const ctrl = HandlerManager::getHandlerControl(han); + vt_debug_print( + verbose, objgroup, + "dispatch: ctrl={:x}, han={:x}\n", ctrl, han + ); + auto const node = 0; + auto const proxy = proxy::ObjGroupProxy::create(ctrl, node, true); + auto& objs = getObjs(); + auto obj_iter = objs.find(proxy); + vt_debug_print( + normal, objgroup, + "dispatch: try ctrl={:x}, han={:x}, has dispatch={}\n", + ctrl, han, obj_iter != objs.end() + ); + if (obj_iter == objs.end()) { + auto const epoch = envelopeGetEpoch(msg->env); + if (epoch != no_epoch and epoch != term::any_epoch_sentinel) { + theTerm()->produce(epoch); + } + auto& pending = getPending(); + pending[proxy].emplace_back([=]{ + auto& objs2 = getObjs(); + auto obj_iter2 = objs2.find(proxy); + vtAssert(obj_iter2 != objs2.end(), "Obj must exist"); + detail::dispatchImpl(msg, han, from_node, cont, obj_iter2->second->getPtr()); + }); + } else { + detail::dispatchImpl(msg, han, from_node, cont, obj_iter->second->getPtr()); + } +} + +} /* end namespace detail */ + +template +void dispatchObjGroup( + MsgSharedPtr msg, HandlerType han, NodeType from_node, + ActionType cont +) { + vt_debug_print( + verbose, objgroup, + "dispatchObjGroup: han={:x}\n", han + ); + return detail::dispatch(msg, han, from_node, cont); +} + }} /* end namespace vt::objgroup */ #endif /*INCLUDED_VT_OBJGROUP_MANAGER_STATIC_H*/