From 681600546425819d6810e4e595d35e843130d878 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Thu, 8 Dec 2016 10:21:09 +0100 Subject: [PATCH 1/1] actually, BypassRoute can be made private to AsImpl --- include/simgrid/s4u/As.hpp | 5 +- src/kernel/routing/AsImpl.cpp | 42 ++++++++- src/kernel/routing/AsImpl.hpp | 145 +++++++++++++++-------------- src/kernel/routing/BypassRoute.hpp | 29 ------ src/s4u/s4u_as.cpp | 32 ------- tools/cmake/DefinePackages.cmake | 1 - 6 files changed, 119 insertions(+), 135 deletions(-) delete mode 100644 src/kernel/routing/BypassRoute.hpp diff --git a/include/simgrid/s4u/As.hpp b/include/simgrid/s4u/As.hpp index 33fcb48e11..f025657357 100644 --- a/include/simgrid/s4u/As.hpp +++ b/include/simgrid/s4u/As.hpp @@ -27,7 +27,6 @@ namespace kernel { namespace routing { class AsImpl; class NetCard; - class BypassRoute; } } namespace s4u { @@ -56,7 +55,7 @@ public: /* Add content to the AS, at parsing time. It should be sealed afterward. */ virtual int addComponent(kernel::routing::NetCard *elm); /* A host, a router or an AS, whatever */ virtual void addRoute(sg_platf_route_cbarg_t route); - void addBypassRoute(sg_platf_route_cbarg_t e_route); + virtual void addBypassRoute(sg_platf_route_cbarg_t e_route) = 0; /*** Called on each newly created regular route (not on bypass routes) */ static simgrid::xbt::signal, kernel::routing::BypassRoute*> - bypassRoutes_; // src x dst -> route xbt_dict_t children_ = xbt_dict_new_homogeneous(nullptr); // sub-ASes }; diff --git a/src/kernel/routing/AsImpl.cpp b/src/kernel/routing/AsImpl.cpp index 1be8910964..d30d7866fd 100644 --- a/src/kernel/routing/AsImpl.cpp +++ b/src/kernel/routing/AsImpl.cpp @@ -7,7 +7,6 @@ #include "simgrid/s4u/host.hpp" #include "src/kernel/routing/AsImpl.hpp" -#include "src/kernel/routing/BypassRoute.hpp" #include "src/kernel/routing/NetCard.hpp" #include "src/surf/cpu_interface.hpp" #include "src/surf/network_interface.hpp" @@ -18,6 +17,14 @@ namespace simgrid { namespace kernel { namespace routing { + class BypassRoute { + public: + explicit BypassRoute(NetCard* gwSrc, NetCard* gwDst) : gw_src(gwSrc), gw_dst(gwDst) {} + const NetCard* gw_src; + const NetCard* gw_dst; + std::vector links; + }; + AsImpl::AsImpl(As* father, const char* name) : As(father, name) { xbt_assert(nullptr == xbt_lib_get_or_null(as_router_lib, name, ROUTING_ASR_LEVEL), @@ -27,6 +34,11 @@ namespace simgrid { xbt_lib_set(as_router_lib, name, ROUTING_ASR_LEVEL, static_cast(netcard_)); XBT_DEBUG("AS '%s' created with the id '%d'", name, netcard_->id()); } + AsImpl::~AsImpl() + { + for (auto& kv : bypassRoutes_) + delete kv.second; + } simgrid::s4u::Host* AsImpl::createHost(const char* name, std::vector* speedPerPstate, int coreAmount) { @@ -42,6 +54,34 @@ namespace simgrid { return res; } + void AsImpl::addBypassRoute(sg_platf_route_cbarg_t e_route) + { + /* Argument validity checks */ + if (e_route->gw_dst) { + XBT_DEBUG("Load bypassASroute from %s@%s to %s@%s", e_route->src->cname(), e_route->gw_src->cname(), + e_route->dst->cname(), e_route->gw_dst->cname()); + xbt_assert(!e_route->link_list->empty(), "Bypass route between %s@%s and %s@%s cannot be empty.", + e_route->src->cname(), e_route->gw_src->cname(), e_route->dst->cname(), e_route->gw_dst->cname()); + xbt_assert(bypassRoutes_.find({e_route->src, e_route->dst}) == bypassRoutes_.end(), + "The bypass route between %s@%s and %s@%s already exists.", e_route->src->cname(), + e_route->gw_src->cname(), e_route->dst->cname(), e_route->gw_dst->cname()); + } else { + XBT_DEBUG("Load bypassRoute from %s to %s", e_route->src->cname(), e_route->dst->cname()); + xbt_assert(!e_route->link_list->empty(), "Bypass route between %s and %s cannot be empty.", e_route->src->cname(), + e_route->dst->cname()); + xbt_assert(bypassRoutes_.find({e_route->src, e_route->dst}) == bypassRoutes_.end(), + "The bypass route between %s and %s already exists.", e_route->src->cname(), e_route->dst->cname()); + } + + /* Build a copy that will be stored in the dict */ + kernel::routing::BypassRoute* newRoute = new kernel::routing::BypassRoute(e_route->gw_src, e_route->gw_dst); + for (auto link : *e_route->link_list) + newRoute->links.push_back(link); + + /* Store it */ + bypassRoutes_.insert({{e_route->src, e_route->dst}, newRoute}); + } + /** @brief Get the common ancestor and its first children in each line leading to src and dst * * In the recursive case, this sets common_ancestor, src_ancestor and dst_ancestor are set as follows. diff --git a/src/kernel/routing/AsImpl.hpp b/src/kernel/routing/AsImpl.hpp index 8719b8337e..2ff562085e 100644 --- a/src/kernel/routing/AsImpl.hpp +++ b/src/kernel/routing/AsImpl.hpp @@ -15,80 +15,89 @@ namespace simgrid { namespace kernel { +class EngineImpl; namespace routing { - /** @brief Autonomous Systems - * - * An AS is a network container, in charge of routing information between elements (hosts) and to the nearby ASes. - * In SimGrid, there is a hierarchy of ASes, ie a tree with a unique root AS, that you can retrieve from the - * s4u::Engine. - * - * The purpose of the kernel::routing module is to retrieve the routing path between two points in a time- and - * space-efficient manner. This is done by AsImpl::getGlobalRoute(), called when creating a communication to - * retrieve both the list of links that the create communication will use, and the summed latency that these - * links represent. - * - * The network could recompute the latency by itself from the list, but it would require an additional link - * set traversal. This operation being on the critical path of SimGrid, the routing computes the latency on the - * behalf of the network. - * - * Finding the path between two nodes is rather complex because we navigate a hierarchy of ASes, each of them - * being a full network. In addition, the routing can declare shortcuts (called bypasses), either within an AS - * at the route level or directly between ASes. Also, each AS can use a differing routing algorithm, depending - * on its class. @ref{AsFull} have a full matrix giving explicitly the path between any pair of their - * contained nodes, while @ref{AsDijkstra} or @ref{AsFloyd} rely on a shortest path algorithm. @ref{AsVivaldi} - * does not even have any link but only use only coordinate information to compute the latency. - * - * So AsImpl::getGlobalRoute builds the path recursively asking its specific information to each traversed AS with - * AsImpl::getLocalRoute, that is redefined in each sub-class. - * The algorithm for that is explained in http://hal.inria.fr/hal-00650233/ - * - */ - XBT_PUBLIC_CLASS AsImpl : public s4u::As - { - protected: - explicit AsImpl(As * father, const char* name); +class BypassRoute; - public: - /** @brief Make an host within that AS */ - simgrid::s4u::Host* createHost(const char* name, std::vector* speedPerPstate, int coreAmount); +/** @brief Autonomous Systems + * + * An AS is a network container, in charge of routing information between elements (hosts) and to the nearby ASes. + * In SimGrid, there is a hierarchy of ASes, ie a tree with a unique root AS, that you can retrieve from the + * s4u::Engine. + * + * The purpose of the kernel::routing module is to retrieve the routing path between two points in a time- and + * space-efficient manner. This is done by AsImpl::getGlobalRoute(), called when creating a communication to + * retrieve both the list of links that the create communication will use, and the summed latency that these + * links represent. + * + * The network could recompute the latency by itself from the list, but it would require an additional link + * set traversal. This operation being on the critical path of SimGrid, the routing computes the latency on the + * behalf of the network. + * + * Finding the path between two nodes is rather complex because we navigate a hierarchy of ASes, each of them + * being a full network. In addition, the routing can declare shortcuts (called bypasses), either within an AS + * at the route level or directly between ASes. Also, each AS can use a differing routing algorithm, depending + * on its class. @ref{AsFull} have a full matrix giving explicitly the path between any pair of their + * contained nodes, while @ref{AsDijkstra} or @ref{AsFloyd} rely on a shortest path algorithm. @ref{AsVivaldi} + * does not even have any link but only use only coordinate information to compute the latency. + * + * So AsImpl::getGlobalRoute builds the path recursively asking its specific information to each traversed AS with + * AsImpl::getLocalRoute, that is redefined in each sub-class. + * The algorithm for that is explained in http://hal.inria.fr/hal-00650233/ + * + */ +XBT_PUBLIC_CLASS AsImpl : public s4u::As +{ + friend simgrid::kernel::EngineImpl; // it destroys rootAs_ - protected: - /** - * @brief Probe the routing path between two points that are local to the called AS. - * - * @param src where from - * @param dst where to - * @param into Container into which the traversed links and gateway informations should be pushed - * @param latency Accumulator in which the latencies should be added (caller must set it to 0) - */ - virtual void getLocalRoute(NetCard * src, NetCard * dst, sg_platf_route_cbarg_t into, double* latency) = 0; - /** @brief retrieves the list of all routes of size 1 (of type src x dst x Link) */ - /* returns whether we found a bypass path */ - bool getBypassRoute(routing::NetCard * src, routing::NetCard * dst, - /* OUT */ std::vector * links, double* latency); +protected: + explicit AsImpl(As * father, const char* name); + virtual ~AsImpl(); - public: - /* @brief get the route between two nodes in the full platform - * - * @param src where from - * @param dst where to - * @param links Accumulator in which all traversed links should be pushed (caller must empty it) - * @param latency Accumulator in which the latencies should be added (caller must set it to 0) - */ - static void getGlobalRoute(routing::NetCard * src, routing::NetCard * dst, - /* OUT */ std::vector * links, double* latency); +public: + /** @brief Make an host within that AS */ + simgrid::s4u::Host* createHost(const char* name, std::vector* speedPerPstate, int coreAmount); + /** @brief Creates a new route in this AS */ + void addBypassRoute(sg_platf_route_cbarg_t e_route) override; + +protected: + /** + * @brief Probe the routing path between two points that are local to the called AS. + * + * @param src where from + * @param dst where to + * @param into Container into which the traversed links and gateway informations should be pushed + * @param latency Accumulator in which the latencies should be added (caller must set it to 0) + */ + virtual void getLocalRoute(NetCard * src, NetCard * dst, sg_platf_route_cbarg_t into, double* latency) = 0; + /** @brief retrieves the list of all routes of size 1 (of type src x dst x Link) */ + /* returns whether we found a bypass path */ + bool getBypassRoute(routing::NetCard * src, routing::NetCard * dst, + /* OUT */ std::vector * links, double* latency); + +public: + /* @brief get the route between two nodes in the full platform + * + * @param src where from + * @param dst where to + * @param links Accumulator in which all traversed links should be pushed (caller must empty it) + * @param latency Accumulator in which the latencies should be added (caller must set it to 0) + */ + static void getGlobalRoute(routing::NetCard * src, routing::NetCard * dst, + /* OUT */ std::vector * links, double* latency); - virtual void getGraph(xbt_graph_t graph, xbt_dict_t nodes, xbt_dict_t edges) = 0; - enum class RoutingMode { - unset = 0, /**< Undefined type */ - base, /**< Base case: use simple link lists for routing */ - recursive /**< Recursive case: also return gateway information */ - }; - /* FIXME: protect the following fields once the construction madness is sorted out */ - RoutingMode hierarchy_ = RoutingMode::unset; + virtual void getGraph(xbt_graph_t graph, xbt_dict_t nodes, xbt_dict_t edges) = 0; + enum class RoutingMode { + unset = 0, /**< Undefined type */ + base, /**< Base case: use simple link lists for routing */ + recursive /**< Recursive case: also return gateway information */ + }; + /* FIXME: protect the following fields once the construction madness is sorted out */ + RoutingMode hierarchy_ = RoutingMode::unset; - private: - routing::NetCard* netcard_ = nullptr; // Our representative in the father AS +private: + std::map, BypassRoute*> bypassRoutes_; // src x dst -> route + routing::NetCard* netcard_ = nullptr; // Our representative in the father AS }; }}}; // Namespace simgrid::kernel::routing diff --git a/src/kernel/routing/BypassRoute.hpp b/src/kernel/routing/BypassRoute.hpp deleted file mode 100644 index e791d39998..0000000000 --- a/src/kernel/routing/BypassRoute.hpp +++ /dev/null @@ -1,29 +0,0 @@ -/* Copyright (c) 2013-2016. The SimGrid Team. All rights reserved. */ - -/* This program is free software; you can redistribute it and/or modify it - * under the terms of the license (GNU LGPL) which comes with this package. */ - -#ifndef KERNEL_ROUTING_BYPASSROUTE_HPP_ -#define KERNEL_ROUTING_BYPASSROUTE_HPP_ - -#include -#include - -#include "src/kernel/routing/NetCard.hpp" - -namespace simgrid { -namespace kernel { -namespace routing { - -class BypassRoute { -public: - explicit BypassRoute(NetCard* gwSrc, NetCard* gwDst) : gw_src(gwSrc), gw_dst(gwDst) {} - const NetCard* gw_src; - const NetCard* gw_dst; - std::vector links; -}; -} -} -} - -#endif /* KERNEL_ROUTING_BYPASSROUTE_HPP_ */ diff --git a/src/s4u/s4u_as.cpp b/src/s4u/s4u_as.cpp index 1a8587b16d..9da12af168 100644 --- a/src/s4u/s4u_as.cpp +++ b/src/s4u/s4u_as.cpp @@ -9,7 +9,6 @@ #include #include -#include "src/kernel/routing/BypassRoute.hpp" #include "src/kernel/routing/NetCard.hpp" #include "src/surf/network_interface.hpp" // Link FIXME: move to proper header #include "src/surf/surf_routing.hpp" @@ -39,8 +38,6 @@ namespace simgrid { xbt_dict_foreach(children_, cursor, key, elem) { delete (As*)elem; } xbt_dict_free(&children_); - for (auto& kv : bypassRoutes_) - delete kv.second; xbt_free(name_); } @@ -80,33 +77,4 @@ namespace simgrid { xbt_die("AS %s does not accept new routes (wrong class).", name_); } - void As::addBypassRoute(sg_platf_route_cbarg_t e_route) - { - /* Argument validity checks */ - if (e_route->gw_dst) { - XBT_DEBUG("Load bypassASroute from %s@%s to %s@%s", e_route->src->name().c_str(), e_route->gw_src->name().c_str(), - e_route->dst->name().c_str(), e_route->gw_dst->name().c_str()); - xbt_assert(!e_route->link_list->empty(), "Bypass route between %s@%s and %s@%s cannot be empty.", - e_route->src->name().c_str(), e_route->gw_src->name().c_str(), e_route->dst->name().c_str(), - e_route->gw_dst->name().c_str()); - xbt_assert(bypassRoutes_.find({e_route->src, e_route->dst}) == bypassRoutes_.end(), - "The bypass route between %s@%s and %s@%s already exists.", e_route->src->name().c_str(), - e_route->gw_src->name().c_str(), e_route->dst->name().c_str(), e_route->gw_dst->name().c_str()); - } else { - XBT_DEBUG("Load bypassRoute from %s to %s", e_route->src->name().c_str(), e_route->dst->name().c_str()); - xbt_assert(!e_route->link_list->empty(), "Bypass route between %s and %s cannot be empty.", - e_route->src->name().c_str(), e_route->dst->name().c_str()); - xbt_assert(bypassRoutes_.find({e_route->src, e_route->dst}) == bypassRoutes_.end(), - "The bypass route between %s and %s already exists.", e_route->src->name().c_str(), - e_route->dst->name().c_str()); - } - - /* Build a copy that will be stored in the dict */ - kernel::routing::BypassRoute* newRoute = new kernel::routing::BypassRoute(e_route->gw_src, e_route->gw_dst); - for (auto link : *e_route->link_list) - newRoute->links.push_back(link); - - /* Store it */ - bypassRoutes_.insert({{e_route->src, e_route->dst}, newRoute}); - } } }; // namespace simgrid::s4u diff --git a/tools/cmake/DefinePackages.cmake b/tools/cmake/DefinePackages.cmake index 7d5b8d431e..7e9dc7c668 100644 --- a/tools/cmake/DefinePackages.cmake +++ b/tools/cmake/DefinePackages.cmake @@ -74,7 +74,6 @@ set(EXTRA_DIST src/kernel/routing/AsRoutedGraph.hpp src/kernel/routing/AsNone.hpp src/kernel/routing/AsVivaldi.hpp - src/kernel/routing/BypassRoute.hpp src/kernel/routing/NetCard.hpp src/surf/storage_interface.hpp -- 2.20.1