From a39fb764301c06bc7e4ef71caa04d1af36dac761 Mon Sep 17 00:00:00 2001 From: SUTER Frederic Date: Tue, 13 Apr 2021 15:13:41 +0200 Subject: [PATCH] do not call sg_platf_new_link when creating Cluster-ed Zones --- .../simgrid/kernel/routing/DragonflyZone.hpp | 3 +- .../simgrid/kernel/routing/FatTreeZone.hpp | 22 +++--- src/kernel/routing/ClusterZone.cpp | 15 ++-- src/kernel/routing/DragonflyZone.cpp | 23 +++--- src/kernel/routing/FatTreeZone.cpp | 72 ++++++------------- src/kernel/routing/TorusZone.cpp | 22 +++--- src/surf/sg_platf.cpp | 17 ++--- 7 files changed, 66 insertions(+), 108 deletions(-) diff --git a/include/simgrid/kernel/routing/DragonflyZone.hpp b/include/simgrid/kernel/routing/DragonflyZone.hpp index 83a7b9623a..dfabbb2edc 100644 --- a/include/simgrid/kernel/routing/DragonflyZone.hpp +++ b/include/simgrid/kernel/routing/DragonflyZone.hpp @@ -79,8 +79,7 @@ private: void do_seal() override; void generate_routers(); void generate_links(); - void generate_link(const std::string& id, int numlinks, resource::LinkImpl** linkup, - resource::LinkImpl** linkdown) const; + void generate_link(const std::string& id, int numlinks, resource::LinkImpl** linkup, resource::LinkImpl** linkdown); simgrid::s4u::Link::SharingPolicy sharing_policy_ = simgrid::s4u::Link::SharingPolicy::SHARED; double bw_ = 0; diff --git a/include/simgrid/kernel/routing/FatTreeZone.hpp b/include/simgrid/kernel/routing/FatTreeZone.hpp index 5a3482b38c..8853322f02 100644 --- a/include/simgrid/kernel/routing/FatTreeZone.hpp +++ b/include/simgrid/kernel/routing/FatTreeZone.hpp @@ -50,8 +50,11 @@ public: /** If present, communications from this node to this node will pass through it * instead of passing by an upper level switch. */ - resource::LinkImpl* loopback; - FatTreeNode(const ClusterCreationArgs* cluster, int id, int level, int position); + resource::LinkImpl* loopback_; + FatTreeNode(int id, int level, int position, resource::LinkImpl* limiter, resource::LinkImpl* loopback) + : id(id), level(level), position(position), limiter_link_(limiter), loopback_(loopback) + { + } }; /** @brief Link in a fat tree (@ref FatTreeZone). @@ -61,15 +64,18 @@ public: */ class FatTreeLink { public: - FatTreeLink(const ClusterCreationArgs* cluster, FatTreeNode* source, FatTreeNode* destination); - /** Link going up in the tree */ - resource::LinkImpl* up_link_; - /** Link going down in the tree */ - resource::LinkImpl* down_link_; + FatTreeLink(FatTreeNode* src, FatTreeNode* dst, resource::LinkImpl* linkup, resource::LinkImpl* linkdown) + : up_node_(dst), down_node_(src), up_link_(linkup), down_link_(linkdown) + { + } /** Upper end of the link */ FatTreeNode* up_node_; /** Lower end of the link */ FatTreeNode* down_node_; + /** Link going up in the tree */ + resource::LinkImpl* up_link_; + /** Link going down in the tree */ + resource::LinkImpl* down_link_; }; /** @ingroup ROUTING_API @@ -138,7 +144,7 @@ public: * It will also store the cluster for future use. */ void parse_specific_arguments(ClusterCreationArgs* cluster) override; - void add_processing_node(int id); + void add_processing_node(int id, resource::LinkImpl* limiter, resource::LinkImpl* loopback); void generate_dot_file(const std::string& filename = "fat_tree.dot") const; }; } // namespace routing diff --git a/src/kernel/routing/ClusterZone.cpp b/src/kernel/routing/ClusterZone.cpp index 1084377ffe..f412f163cc 100644 --- a/src/kernel/routing/ClusterZone.cpp +++ b/src/kernel/routing/ClusterZone.cpp @@ -141,20 +141,13 @@ void ClusterZone::create_links_for_node(ClusterCreationArgs* cluster, int id, in { std::string link_id = cluster->id + "_link_" + std::to_string(id); - LinkCreationArgs link; - link.id = link_id; - link.bandwidths.push_back(cluster->bw); - link.latency = cluster->lat; - link.policy = cluster->sharing_policy; - sg_platf_new_link(&link); - const s4u::Link* linkUp; const s4u::Link* linkDown; - if (link.policy == simgrid::s4u::Link::SharingPolicy::SPLITDUPLEX) { - linkUp = s4u::Link::by_name(link_id + "_UP"); - linkDown = s4u::Link::by_name(link_id + "_DOWN"); + if (cluster->sharing_policy == simgrid::s4u::Link::SharingPolicy::SPLITDUPLEX) { + linkUp = create_link(link_id + "_UP", std::vector{cluster->bw})->set_latency(cluster->lat)->seal(); + linkDown = create_link(link_id + "_DOWN", std::vector{cluster->bw})->set_latency(cluster->lat)->seal(); } else { - linkUp = s4u::Link::by_name(link_id); + linkUp = create_link(link_id, std::vector{cluster->bw})->set_latency(cluster->lat)->seal(); linkDown = linkUp; } private_links_.insert({position, {linkUp->get_impl(), linkDown->get_impl()}}); diff --git a/src/kernel/routing/DragonflyZone.cpp b/src/kernel/routing/DragonflyZone.cpp index d06e6a7be9..ce0f59b878 100644 --- a/src/kernel/routing/DragonflyZone.cpp +++ b/src/kernel/routing/DragonflyZone.cpp @@ -141,25 +141,18 @@ void DragonflyZone::generate_routers() } void DragonflyZone::generate_link(const std::string& id, int numlinks, resource::LinkImpl** linkup, - resource::LinkImpl** linkdown) const + resource::LinkImpl** linkdown) { + XBT_DEBUG("Generating link %s", id.c_str()); *linkup = nullptr; *linkdown = nullptr; - LinkCreationArgs linkTemplate; - linkTemplate.bandwidths.push_back(this->bw_ * numlinks); - linkTemplate.latency = this->lat_; - linkTemplate.policy = this->sharing_policy_; - linkTemplate.id = id; - sg_platf_new_link(&linkTemplate); - XBT_DEBUG("Generating link %s", linkTemplate.id.c_str()); - resource::LinkImpl* link; - if (this->sharing_policy_ == s4u::Link::SharingPolicy::SPLITDUPLEX) { - *linkup = s4u::Link::by_name(linkTemplate.id + "_UP")->get_impl(); // check link? - *linkdown = s4u::Link::by_name(linkTemplate.id + "_DOWN")->get_impl(); // check link ? + if (sharing_policy_ == s4u::Link::SharingPolicy::SPLITDUPLEX) { + *linkup = create_link(id + "_UP", std::vector{bw_ * numlinks})->set_latency(lat_)->seal()->get_impl(); + *linkdown = create_link(id + "_DOWN", std::vector{bw_ * numlinks})->set_latency(lat_)->seal()->get_impl(); + } else { - link = s4u::Link::by_name(linkTemplate.id)->get_impl(); - *linkup = link; - *linkdown = link; + *linkup = create_link(id, std::vector{bw_ * numlinks})->set_latency(lat_)->seal()->get_impl(); + *linkdown = *linkup; } } diff --git a/src/kernel/routing/FatTreeZone.cpp b/src/kernel/routing/FatTreeZone.cpp index 0d396ee01e..4a02b99e29 100644 --- a/src/kernel/routing/FatTreeZone.cpp +++ b/src/kernel/routing/FatTreeZone.cpp @@ -71,9 +71,9 @@ void FatTreeZone::get_local_route(NetPoint* src, NetPoint* dst, RouteCreationArg /* In case destination is the source, and there is a loopback, let's use it instead of going up to a switch */ if (source->id == destination->id && has_loopback()) { - into->link_list.push_back(source->loopback); + into->link_list.push_back(source->loopback_); if (latency) - *latency += source->loopback->get_latency(); + *latency += source->loopback_->get_latency(); return; } @@ -255,7 +255,7 @@ void FatTreeZone::generate_switches() int k = 0; for (unsigned int i = 0; i < this->levels_; i++) { for (unsigned int j = 0; j < this->nodes_by_level_[i + 1]; j++) { - auto* newNode = new FatTreeNode(this->cluster_, --k, i + 1, j); + auto* newNode = new FatTreeNode(--k, i + 1, j, nullptr, nullptr); XBT_DEBUG("We create the switch %d(%u,%u)", newNode->id, newNode->level, newNode->position); newNode->children.resize(this->num_children_per_node_[i] * this->num_port_lower_level_[i]); if (i != this->levels_ - 1) { @@ -323,12 +323,12 @@ int FatTreeZone::get_level_position(const unsigned int level) return tempPosition; } -void FatTreeZone::add_processing_node(int id) +void FatTreeZone::add_processing_node(int id, resource::LinkImpl* limiter, resource::LinkImpl* loopback) { using std::make_pair; static int position = 0; FatTreeNode* newNode; - newNode = new FatTreeNode(this->cluster_, id, 0, position++); + newNode = new FatTreeNode(id, 0, position++, limiter, loopback); newNode->parents.resize(this->num_parents_per_node_[0] * this->num_port_lower_level_[0]); newNode->label.resize(this->levels_); this->compute_nodes_.insert(make_pair(id, newNode)); @@ -337,8 +337,22 @@ void FatTreeZone::add_processing_node(int id) void FatTreeZone::add_link(FatTreeNode* parent, unsigned int parentPort, FatTreeNode* child, unsigned int childPort) { - FatTreeLink* newLink; - newLink = new FatTreeLink(this->cluster_, child, parent); + static int uniqueId = 0; + s4u::Link* linkup; + s4u::Link* linkdown; + std::string id = + "link_from_" + std::to_string(child->id) + "_" + std::to_string(parent->id) + "_" + std::to_string(uniqueId); + + if (cluster_->sharing_policy == s4u::Link::SharingPolicy::SPLITDUPLEX) { + linkup = create_link(id + "_UP", std::vector{cluster_->bw})->set_latency(cluster_->lat)->seal(); + linkdown = create_link(id + "_DOWN", std::vector{cluster_->bw})->set_latency(cluster_->lat)->seal(); + } else { + linkup = create_link(id, std::vector{cluster_->bw})->set_latency(cluster_->lat)->seal(); + linkdown = linkup; + } + uniqueId++; + + FatTreeLink* newLink = new FatTreeLink(child, parent, linkup->get_impl(), linkdown->get_impl()); XBT_DEBUG("Creating a link between the parent (%u,%u,%u) and the child (%u,%u,%u)", parent->level, parent->position, parentPort, child->level, child->position, childPort); parent->children[parentPort] = newLink; @@ -428,50 +442,6 @@ void FatTreeZone::generate_dot_file(const std::string& filename) const file << "}"; file.close(); } - -FatTreeNode::FatTreeNode(const ClusterCreationArgs* cluster, int id, int level, int position) - : id(id), level(level), position(position) -{ - LinkCreationArgs linkTemplate; - if (cluster->limiter_link != 0.0) { - linkTemplate.bandwidths.push_back(cluster->limiter_link); - linkTemplate.latency = 0; - linkTemplate.policy = s4u::Link::SharingPolicy::SHARED; - linkTemplate.id = "limiter_" + std::to_string(id); - sg_platf_new_link(&linkTemplate); - this->limiter_link_ = s4u::Link::by_name(linkTemplate.id)->get_impl(); - } - if (cluster->loopback_bw != 0.0 || cluster->loopback_lat != 0.0) { - linkTemplate.bandwidths.push_back(cluster->loopback_bw); - linkTemplate.latency = cluster->loopback_lat; - linkTemplate.policy = s4u::Link::SharingPolicy::FATPIPE; - linkTemplate.id = "loopback_" + std::to_string(id); - sg_platf_new_link(&linkTemplate); - this->loopback = s4u::Link::by_name(linkTemplate.id)->get_impl(); - } -} - -FatTreeLink::FatTreeLink(const ClusterCreationArgs* cluster, FatTreeNode* downNode, FatTreeNode* upNode) - : up_node_(upNode), down_node_(downNode) -{ - static int uniqueId = 0; - LinkCreationArgs linkTemplate; - linkTemplate.bandwidths.push_back(cluster->bw); - linkTemplate.latency = cluster->lat; - linkTemplate.policy = cluster->sharing_policy; // sthg to do with that ? - linkTemplate.id = - "link_from_" + std::to_string(downNode->id) + "_" + std::to_string(upNode->id) + "_" + std::to_string(uniqueId); - sg_platf_new_link(&linkTemplate); - - if (cluster->sharing_policy == s4u::Link::SharingPolicy::SPLITDUPLEX) { - this->up_link_ = s4u::Link::by_name(linkTemplate.id + "_UP")->get_impl(); // check link? - this->down_link_ = s4u::Link::by_name(linkTemplate.id + "_DOWN")->get_impl(); // check link ? - } else { - this->up_link_ = s4u::Link::by_name(linkTemplate.id)->get_impl(); - this->down_link_ = this->up_link_; - } - uniqueId++; -} } // namespace routing } // namespace kernel diff --git a/src/kernel/routing/TorusZone.cpp b/src/kernel/routing/TorusZone.cpp index 0032c875f8..1663e3198a 100644 --- a/src/kernel/routing/TorusZone.cpp +++ b/src/kernel/routing/TorusZone.cpp @@ -35,26 +35,22 @@ void TorusZone::create_links_for_node(ClusterCreationArgs* cluster, int id, int // name of neighbor is not right for non contiguous cluster radicals (as id != rank in this case) std::string link_id = std::string(cluster->id) + "_link_from_" + std::to_string(id) + "_to_" + std::to_string(neighbor_rank_id); - link.id = link_id; - link.bandwidths.push_back(cluster->bw); - link.latency = cluster->lat; - link.policy = cluster->sharing_policy; - sg_platf_new_link(&link); - resource::LinkImpl* linkUp; - resource::LinkImpl* linkDown; - if (link.policy == s4u::Link::SharingPolicy::SPLITDUPLEX) { - linkUp = s4u::Link::by_name(link_id + "_UP")->get_impl(); - linkDown = s4u::Link::by_name(link_id + "_DOWN")->get_impl(); + s4u::Link* linkup; + s4u::Link* linkdown; + if (cluster->sharing_policy == s4u::Link::SharingPolicy::SPLITDUPLEX) { + linkup = create_link(link_id + "_UP", std::vector{cluster->bw})->set_latency(cluster->lat)->seal(); + linkdown = create_link(link_id + "_DOWN", std::vector{cluster->bw})->set_latency(cluster->lat)->seal(); + } else { - linkUp = s4u::Link::by_name(link_id)->get_impl(); - linkDown = linkUp; + linkup = create_link(link_id, std::vector{cluster->bw})->set_latency(cluster->lat)->seal(); + linkdown = linkup; } /* * Add the link to its appropriate position. * Note that position rankId*(xbt_dynar_length(dimensions)+has_loopback?+has_limiter?) * holds the link "rankId->rankId" */ - add_private_link_at(position + j, {linkUp, linkDown}); + add_private_link_at(position + j, {linkup->get_impl(), linkdown->get_impl()}); dim_product *= current_dimension; } } diff --git a/src/surf/sg_platf.cpp b/src/surf/sg_platf.cpp index b50f628d52..37bcf815c2 100644 --- a/src/surf/sg_platf.cpp +++ b/src/surf/sg_platf.cpp @@ -197,33 +197,34 @@ void sg_platf_new_cluster(simgrid::kernel::routing::ClusterCreationArgs* cluster // other columns are to store one or more link for the node // add a loopback link + simgrid::kernel::resource::LinkImpl* loopback = nullptr; if (cluster->loopback_bw > 0 || cluster->loopback_lat > 0) { std::string loopback_name = link_id + "_loopback"; XBT_DEBUG("", loopback_name.c_str(), cluster->loopback_bw); - auto* loopback = current_zone->create_link(loopback_name, std::vector{cluster->loopback_bw}) - ->set_sharing_policy(simgrid::s4u::Link::SharingPolicy::FATPIPE) - ->set_latency(cluster->loopback_lat) - ->seal() - ->get_impl(); + loopback = current_zone->create_link(loopback_name, std::vector{cluster->loopback_bw}) + ->set_sharing_policy(simgrid::s4u::Link::SharingPolicy::FATPIPE) + ->set_latency(cluster->loopback_lat) + ->seal() + ->get_impl(); current_zone->add_private_link_at(current_zone->node_pos(rankId), {loopback, loopback}); } // add a limiter link (shared link to account for maximal bandwidth of the node) + simgrid::kernel::resource::LinkImpl* limiter = nullptr; if (cluster->limiter_link > 0) { std::string limiter_name = std::string(link_id) + "_limiter"; XBT_DEBUG("", limiter_name.c_str(), cluster->limiter_link); - auto* limiter = - current_zone->create_link(limiter_name, std::vector{cluster->limiter_link})->seal()->get_impl(); + limiter = current_zone->create_link(limiter_name, std::vector{cluster->limiter_link})->seal()->get_impl(); current_zone->add_private_link_at(current_zone->node_pos_with_loopback(rankId), {limiter, limiter}); } // call the cluster function that adds the others links if (cluster->topology == simgrid::kernel::routing::ClusterTopology::FAT_TREE) { - static_cast(current_zone)->add_processing_node(i); + static_cast(current_zone)->add_processing_node(i, limiter, loopback); } else { current_zone->create_links_for_node(cluster, i, rankId, current_zone->node_pos_with_loopback_limiter(rankId)); } -- 2.20.1