From b592b5aa6acbd39d705426f2b275ee8736548d0d Mon Sep 17 00:00:00 2001 From: SUTER Frederic Date: Fri, 30 Apr 2021 11:51:45 +0200 Subject: [PATCH] refactor link characteristics management across Clustered Zones --- .../simgrid/kernel/routing/ClusterZone.hpp | 10 ++++++++ .../simgrid/kernel/routing/DragonflyZone.hpp | 6 +---- .../simgrid/kernel/routing/FatTreeZone.hpp | 6 ----- include/simgrid/kernel/routing/TorusZone.hpp | 5 ---- src/kernel/routing/ClusterZone.cpp | 7 ++++++ src/kernel/routing/DragonflyZone.cpp | 23 ++++++++++++------- src/kernel/routing/FatTreeZone.cpp | 17 +++++--------- src/kernel/routing/TorusZone.cpp | 19 +++++++-------- 8 files changed, 47 insertions(+), 46 deletions(-) diff --git a/include/simgrid/kernel/routing/ClusterZone.hpp b/include/simgrid/kernel/routing/ClusterZone.hpp index 1280241ffb..5c6ebf8b51 100644 --- a/include/simgrid/kernel/routing/ClusterZone.hpp +++ b/include/simgrid/kernel/routing/ClusterZone.hpp @@ -76,14 +76,24 @@ class ClusterZone : public NetZoneImpl { bool has_loopback_ = false; unsigned int num_links_per_node_ = 1; /* may be 1 (if only a private link), 2 or 3 (if limiter and loopback) */ + s4u::Link::SharingPolicy link_sharing_policy_; //!< cluster links: sharing policy + double link_bw_; //!< cluster links: bandwidth + double link_lat_; //!< cluster links: latency + protected: void set_num_links_per_node(unsigned int num) { num_links_per_node_ = num; } resource::LinkImpl* get_uplink_from(unsigned int position) const { return private_links_.at(position).first; } resource::LinkImpl* get_downlink_to(unsigned int position) const { return private_links_.at(position).second; } + double get_link_latency() const { return link_lat_; } + double get_link_bandwidth() const { return link_bw_; } + s4u::Link::SharingPolicy get_link_sharing_policy() const { return link_sharing_policy_; } + public: explicit ClusterZone(const std::string& name); + /** @brief Set the characteristics of links inside a Cluster zone */ + virtual void set_link_characteristics(double bw, double lat, s4u::Link::SharingPolicy sharing_policy); void set_loopback(); bool has_loopback() const { return has_loopback_; } void set_limiter(); diff --git a/include/simgrid/kernel/routing/DragonflyZone.hpp b/include/simgrid/kernel/routing/DragonflyZone.hpp index 19f1f4a67c..382c4d1823 100644 --- a/include/simgrid/kernel/routing/DragonflyZone.hpp +++ b/include/simgrid/kernel/routing/DragonflyZone.hpp @@ -84,7 +84,7 @@ public: void set_topology(unsigned int n_groups, unsigned int groups_links, unsigned int n_chassis, unsigned int chassis_links, unsigned int n_routers, unsigned int routers_links, unsigned int nodes); /** @brief Set the characteristics of links inside the Dragonfly zone */ - void set_link_characteristics(double bw, double lat, s4u::Link::SharingPolicy sharing_policy); + void set_link_characteristics(double bw, double lat, s4u::Link::SharingPolicy sharing_policy) override; Coords rankId_to_coords(int rank_id) const; XBT_ATTRIB_DEPRECATED_v330("Please use rankId_to_coords(int)") void rankId_to_coords(int rank_id, unsigned int coords[4]) const; @@ -95,10 +95,6 @@ private: void generate_links(); 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; - double lat_ = 0; - unsigned int num_nodes_per_blade_ = 0; unsigned int num_blades_per_chassis_ = 0; unsigned int num_chassis_per_group_ = 0; diff --git a/include/simgrid/kernel/routing/FatTreeZone.hpp b/include/simgrid/kernel/routing/FatTreeZone.hpp index 7f064e7107..dee249c22b 100644 --- a/include/simgrid/kernel/routing/FatTreeZone.hpp +++ b/include/simgrid/kernel/routing/FatTreeZone.hpp @@ -120,10 +120,6 @@ class XBT_PRIVATE FatTreeZone : public ClusterZone { std::vector links_; std::vector nodes_by_level_; - s4u::Link::SharingPolicy link_sharing_policy_; //!< fat tree links: sharing policy - double link_bw_; //!< fat tree links: bandwidth - double link_lat_; //!< fat tree links: latency - void add_link(FatTreeNode* parent, unsigned int parent_port, FatTreeNode* child, unsigned int child_port); int get_level_position(const unsigned int level); void generate_labels(); @@ -153,8 +149,6 @@ public: /** @brief Set FatTree topology */ void set_topology(unsigned int n_levels, const std::vector& down_links, const std::vector& up_links, const std::vector& link_count); - /** @brief Set the characteristics of links inside the Fat Tree zone */ - void set_link_characteristics(double bw, double lat, s4u::Link::SharingPolicy sharing_policy); void add_processing_node(int id, resource::LinkImpl* limiter, resource::LinkImpl* loopback); void generate_dot_file(const std::string& filename = "fat_tree.dot") const; }; diff --git a/include/simgrid/kernel/routing/TorusZone.hpp b/include/simgrid/kernel/routing/TorusZone.hpp index 9d6273e949..ae6e6d980b 100644 --- a/include/simgrid/kernel/routing/TorusZone.hpp +++ b/include/simgrid/kernel/routing/TorusZone.hpp @@ -21,9 +21,6 @@ namespace routing { class XBT_PRIVATE TorusZone : public ClusterZone { std::vector dimensions_; - s4u::Link::SharingPolicy link_sharing_policy_; //!< torus links: sharing policy - double link_bw_; //!< torus links: bandwidth - double link_lat_; //!< torus links: latency public: using ClusterZone::ClusterZone; @@ -33,8 +30,6 @@ public: /** @brief Convert topology parameters from string to vector of uint */ static std::vector parse_topo_parameters(const std::string& topo_parameters); - /** @brief Set the characteristics of links inside the Torus zone */ - void set_link_characteristics(double bw, double lat, s4u::Link::SharingPolicy sharing_policy); }; } // namespace routing diff --git a/src/kernel/routing/ClusterZone.cpp b/src/kernel/routing/ClusterZone.cpp index 5d31b78f65..92383a49e6 100644 --- a/src/kernel/routing/ClusterZone.cpp +++ b/src/kernel/routing/ClusterZone.cpp @@ -35,6 +35,13 @@ void ClusterZone::set_limiter() } } +void ClusterZone::set_link_characteristics(double bw, double lat, s4u::Link::SharingPolicy sharing_policy) +{ + link_sharing_policy_ = sharing_policy; + link_bw_ = bw; + link_lat_ = lat; +} + void ClusterZone::add_private_link_at(unsigned int position, std::pair link) { private_links_.insert({position, link}); diff --git a/src/kernel/routing/DragonflyZone.cpp b/src/kernel/routing/DragonflyZone.cpp index f6e7d95efd..bfaa43b533 100644 --- a/src/kernel/routing/DragonflyZone.cpp +++ b/src/kernel/routing/DragonflyZone.cpp @@ -45,11 +45,9 @@ void DragonflyZone::rankId_to_coords(int rankId, unsigned int coords[4]) const / void DragonflyZone::set_link_characteristics(double bw, double lat, s4u::Link::SharingPolicy sharing_policy) { - sharing_policy_ = sharing_policy; + ClusterZone::set_link_characteristics(bw, lat, sharing_policy); if (sharing_policy == s4u::Link::SharingPolicy::SPLITDUPLEX) num_links_per_link_ = 2; - bw_ = bw; - lat_ = lat; } void DragonflyZone::set_topology(unsigned int n_groups, unsigned int groups_links, unsigned int n_chassis, @@ -170,11 +168,20 @@ void DragonflyZone::generate_link(const std::string& id, int numlinks, resource: XBT_DEBUG("Generating link %s", id.c_str()); *linkup = nullptr; *linkdown = nullptr; - 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(); + if (get_link_sharing_policy() == s4u::Link::SharingPolicy::SPLITDUPLEX) { + *linkup = create_link(id + "_UP", std::vector{get_link_bandwidth() * numlinks}) + ->set_latency(get_link_latency()) + ->seal() + ->get_impl(); + *linkdown = create_link(id + "_DOWN", std::vector{get_link_bandwidth() * numlinks}) + ->set_latency(get_link_latency()) + ->seal() + ->get_impl(); } else { - *linkup = create_link(id, std::vector{bw_ * numlinks})->set_latency(lat_)->seal()->get_impl(); + *linkup = create_link(id, std::vector{get_link_bandwidth() * numlinks}) + ->set_latency(get_link_latency()) + ->seal() + ->get_impl(); *linkdown = *linkup; } } @@ -200,7 +207,7 @@ void DragonflyZone::generate_links() generate_link(id, 1, &linkup, &linkdown); routers_[i].my_nodes_[j] = linkup; - if (sharing_policy_ == s4u::Link::SharingPolicy::SPLITDUPLEX) + if (get_link_sharing_policy() == s4u::Link::SharingPolicy::SPLITDUPLEX) routers_[i].my_nodes_[j + 1] = linkdown; uniqueId++; diff --git a/src/kernel/routing/FatTreeZone.cpp b/src/kernel/routing/FatTreeZone.cpp index 5613a21b7b..bac4382981 100644 --- a/src/kernel/routing/FatTreeZone.cpp +++ b/src/kernel/routing/FatTreeZone.cpp @@ -346,11 +346,13 @@ void FatTreeZone::add_link(FatTreeNode* parent, unsigned int parentPort, FatTree std::string id = "link_from_" + std::to_string(child->id) + "_" + std::to_string(parent->id) + "_" + std::to_string(uniqueId); - if (link_sharing_policy_ == s4u::Link::SharingPolicy::SPLITDUPLEX) { - linkup = create_link(id + "_UP", std::vector{link_bw_})->set_latency(link_lat_)->seal(); - linkdown = create_link(id + "_DOWN", std::vector{link_bw_})->set_latency(link_lat_)->seal(); + if (get_link_sharing_policy() == s4u::Link::SharingPolicy::SPLITDUPLEX) { + linkup = + create_link(id + "_UP", std::vector{get_link_bandwidth()})->set_latency(get_link_latency())->seal(); + linkdown = + create_link(id + "_DOWN", std::vector{get_link_bandwidth()})->set_latency(get_link_latency())->seal(); } else { - linkup = create_link(id, std::vector{link_bw_})->set_latency(link_lat_)->seal(); + linkup = create_link(id, std::vector{get_link_bandwidth()})->set_latency(get_link_latency())->seal(); linkdown = linkup; } uniqueId++; @@ -388,13 +390,6 @@ void FatTreeZone::check_topology(unsigned int n_levels, const std::vector& down_links, const std::vector& up_links, const std::vector& link_count) { diff --git a/src/kernel/routing/TorusZone.cpp b/src/kernel/routing/TorusZone.cpp index 793e9a6fd8..e1ad2dd25f 100644 --- a/src/kernel/routing/TorusZone.cpp +++ b/src/kernel/routing/TorusZone.cpp @@ -37,12 +37,16 @@ void TorusZone::create_links(int id, int rank, unsigned int position) std::string link_id = get_name() + "_link_from_" + std::to_string(id) + "_to_" + std::to_string(neighbor_rank_id); const s4u::Link* linkup; const s4u::Link* linkdown; - if (link_sharing_policy_ == s4u::Link::SharingPolicy::SPLITDUPLEX) { - linkup = create_link(link_id + "_UP", std::vector{link_bw_})->set_latency(link_lat_)->seal(); - linkdown = create_link(link_id + "_DOWN", std::vector{link_bw_})->set_latency(link_lat_)->seal(); + if (get_link_sharing_policy() == s4u::Link::SharingPolicy::SPLITDUPLEX) { + linkup = create_link(link_id + "_UP", std::vector{get_link_bandwidth()}) + ->set_latency(get_link_latency()) + ->seal(); + linkdown = create_link(link_id + "_DOWN", std::vector{get_link_bandwidth()}) + ->set_latency(get_link_latency()) + ->seal(); } else { - linkup = create_link(link_id, std::vector{link_bw_})->set_latency(link_lat_)->seal(); + linkup = create_link(link_id, std::vector{get_link_bandwidth()})->set_latency(get_link_latency())->seal(); linkdown = linkup; } /* @@ -71,13 +75,6 @@ std::vector TorusZone::parse_topo_parameters(const std::string& to return dimensions; } -void TorusZone::set_link_characteristics(double bw, double lat, s4u::Link::SharingPolicy sharing_policy) -{ - link_sharing_policy_ = sharing_policy; - link_bw_ = bw; - link_lat_ = lat; -} - void TorusZone::set_topology(const std::vector& dimensions) { xbt_assert(not dimensions.empty(), "Torus dimensions cannot be empty"); -- 2.20.1