From 3577c5ad9b3db70921211e8ec8fe16dfb076b6cf Mon Sep 17 00:00:00 2001 From: Bruno Donassolo Date: Wed, 19 May 2021 20:55:39 +0200 Subject: [PATCH] Refactor of NetworkCm02Model::communicate Divide and conquer. Reorganize code. Cosmetics. --- src/surf/network_cm02.cpp | 194 +++++++++++++++++++++------------ src/surf/network_cm02.hpp | 18 +++ src/surf/network_interface.cpp | 3 - src/surf/network_interface.hpp | 4 +- 4 files changed, 146 insertions(+), 73 deletions(-) diff --git a/src/surf/network_cm02.cpp b/src/surf/network_cm02.cpp index 9c88a3f094..cb1e7cfbdb 100644 --- a/src/surf/network_cm02.cpp +++ b/src/surf/network_cm02.cpp @@ -199,35 +199,58 @@ void NetworkCm02Model::update_actions_state_full(double /*now*/, double delta) } } -Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double size, double rate) +void NetworkCm02Model::comm_action_expand_constraints(const s4u::Host* src, const s4u::Host* dst, + NetworkCm02Action* action, const std::vector& route, + const std::vector& back_route) { - double latency = 0.0; - std::vector back_route; - std::vector route; - std::vector s4u_route; - std::unordered_set netzones; - std::unordered_set s4u_netzones; - - XBT_IN("(%s,%s,%g,%g)", src->get_cname(), dst->get_cname(), size, rate); - - kernel::routing::NetZoneImpl::get_global_route_with_netzones(src->get_netpoint(), dst->get_netpoint(), route, - &latency, netzones); + /* expand route links constraints for route and back_route */ + NetworkWifiLink* src_wifi_link = nullptr; + NetworkWifiLink* dst_wifi_link = nullptr; + if (not route.empty() && route.front()->get_sharing_policy() == s4u::Link::SharingPolicy::WIFI) { + src_wifi_link = static_cast(route.front()); + } + if (route.size() > 1 && route.back()->get_sharing_policy() == s4u::Link::SharingPolicy::WIFI) { + dst_wifi_link = static_cast(route.back()); + } - xbt_assert(not route.empty() || latency > 0, - "You're trying to send data from %s to %s but there is no connecting path between these two hosts.", - src->get_cname(), dst->get_cname()); + /* WI-FI links needs special treatment, do it here */ + if (src_wifi_link != nullptr) + get_maxmin_system()->expand(src_wifi_link->get_constraint(), action->get_variable(), + 1.0 / src_wifi_link->get_host_rate(src)); + if (dst_wifi_link != nullptr) + get_maxmin_system()->expand(dst_wifi_link->get_constraint(), action->get_variable(), + 1.0 / dst_wifi_link->get_host_rate(dst)); - bool failed = std::any_of(route.begin(), route.end(), [](const LinkImpl* link) { return not link->is_on(); }); + for (auto const* link : route) { + if (link->get_sharing_policy() != s4u::Link::SharingPolicy::WIFI) + get_maxmin_system()->expand(link->get_constraint(), action->get_variable(), 1.0); + } if (cfg_crosstraffic) { - dst->route_to(src, back_route, nullptr); - if (not failed) - failed = - std::any_of(back_route.begin(), back_route.end(), [](const LinkImpl* link) { return not link->is_on(); }); + XBT_DEBUG("Crosstraffic active: adding backward flow using 5%% of the available bandwidth"); + if (dst_wifi_link != nullptr) + get_maxmin_system()->expand(dst_wifi_link->get_constraint(), action->get_variable(), + .05 / dst_wifi_link->get_host_rate(dst)); + if (src_wifi_link != nullptr) + get_maxmin_system()->expand(src_wifi_link->get_constraint(), action->get_variable(), + .05 / src_wifi_link->get_host_rate(src)); + + for (auto const* link : back_route) { + if (link->get_sharing_policy() != s4u::Link::SharingPolicy::WIFI) + get_maxmin_system()->expand(link->get_constraint(), action->get_variable(), .05); + } + // Change concurrency_share here, if you want that cross-traffic is included in the SURF concurrency + // (You would also have to change simgrid::kernel::lmm::Element::get_concurrency()) + // action->getVariable()->set_concurrency_share(2) } +} +NetworkCm02Action* NetworkCm02Model::comm_action_create(s4u::Host* src, s4u::Host* dst, double size, + const std::vector& route, bool failed) +{ NetworkWifiLink* src_wifi_link = nullptr; NetworkWifiLink* dst_wifi_link = nullptr; + /* many checks related to Wi-Fi links */ if (not route.empty() && route.front()->get_sharing_policy() == s4u::Link::SharingPolicy::WIFI) { src_wifi_link = static_cast(route.front()); xbt_assert(src_wifi_link->get_host_rate(src) != -1, @@ -250,25 +273,56 @@ Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double siz "Did you declare an access_point in your WIFI zones?", route[i]->get_cname(), src->get_cname(), dst->get_cname(), i + 1, route.size()); + for (auto const* link : route) { + if (link->get_sharing_policy() == s4u::Link::SharingPolicy::WIFI) { + xbt_assert(link == src_wifi_link || link == dst_wifi_link, + "Wifi links can only occur at the beginning of the route (meaning that it's attached to the src) or " + "at its end (meaning that it's attached to the dst"); + } + } + + /* create action and do some initializations */ NetworkCm02Action* action; if (src_wifi_link == nullptr && dst_wifi_link == nullptr) action = new NetworkCm02Action(this, *src, *dst, size, failed); else action = new NetworkWifiAction(this, *src, *dst, size, failed, src_wifi_link, dst_wifi_link); - action->sharing_penalty_ = latency; - action->latency_ = latency; - action->set_user_bound(rate); if (is_update_lazy()) { action->set_last_update(); } - if (sg_weight_S_parameter > 0) { - action->sharing_penalty_ = - std::accumulate(route.begin(), route.end(), action->sharing_penalty_, [](double total, LinkImpl* const& link) { - return total + sg_weight_S_parameter / link->get_bandwidth(); - }); + return action; +} + +bool NetworkCm02Model::comm_get_route_info(const s4u::Host* src, const s4u::Host* dst, double& latency, + std::vector& route, std::vector& back_route, + std::unordered_set& netzones) const +{ + kernel::routing::NetZoneImpl::get_global_route_with_netzones(src->get_netpoint(), dst->get_netpoint(), route, + &latency, netzones); + + xbt_assert(not route.empty() || latency > 0, + "You're trying to send data from %s to %s but there is no connecting path between these two hosts.", + src->get_cname(), dst->get_cname()); + + bool failed = std::any_of(route.begin(), route.end(), [](const LinkImpl* link) { return not link->is_on(); }); + + if (cfg_crosstraffic) { + dst->route_to(src, back_route, nullptr); + if (not failed) + failed = + std::any_of(back_route.begin(), back_route.end(), [](const LinkImpl* link) { return not link->is_on(); }); } + return failed; +} + +void NetworkCm02Model::comm_action_set_bounds(const s4u::Host* src, const s4u::Host* dst, double size, + NetworkCm02Action* action, const std::vector& route, + const std::unordered_set& netzones) +{ + std::vector s4u_route; + std::unordered_set s4u_netzones; /* transform data to user structures if necessary */ if (lat_factor_cb_ || bw_factor_cb_) { @@ -282,11 +336,18 @@ Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double siz } else { bw_factor = get_bandwidth_factor(size); } + /* get mininum bandwidth among links in the route and multiply by correct factor */ + const auto& min_bw = std::min_element( + route.begin(), route.end(), [](const auto& a, const auto& b) { return a->get_bandwidth() < b->get_bandwidth(); }); - double bandwidth_bound = route.empty() ? -1.0 : bw_factor * route.front()->get_bandwidth(); + double bandwidth_bound = min_bw == route.end() ? -1.0 : bw_factor * (*min_bw)->get_bandwidth(); - for (auto const& link : route) - bandwidth_bound = std::min(bandwidth_bound, bw_factor * link->get_bandwidth()); + if (bw_constraint_cb_) { + action->set_user_bound( + bw_constraint_cb_(action->get_user_bound(), bandwidth_bound, size, src, dst, s4u_route, s4u_netzones)); + } else { + action->set_user_bound(get_bandwidth_constraint(action->get_user_bound(), bandwidth_bound, size)); + } action->lat_current_ = action->latency_; if (lat_factor_cb_) { @@ -294,14 +355,11 @@ Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double siz } else { action->latency_ *= get_latency_factor(size); } +} - if (bw_constraint_cb_) { - action->set_user_bound( - bw_constraint_cb_(action->get_user_bound(), bandwidth_bound, size, src, dst, s4u_route, s4u_netzones)); - } else { - action->set_user_bound(get_bandwidth_constraint(action->get_user_bound(), bandwidth_bound, size)); - } - +void NetworkCm02Model::comm_action_set_variable(NetworkCm02Action* action, const std::vector& route, + const std::vector& back_route) +{ size_t constraints_per_variable = route.size(); constraints_per_variable += back_route.size(); @@ -319,6 +377,7 @@ Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double siz } else action->set_variable(get_maxmin_system()->variable_new(action, 1.0, -1.0, constraints_per_variable)); + /* after setting the variable, update the bounds depending on user configuration */ if (action->get_user_bound() < 0) { get_maxmin_system()->update_variable_bound( action->get_variable(), (action->lat_current_ > 0) ? cfg_tcp_gamma / (2.0 * action->lat_current_) : -1.0); @@ -328,40 +387,39 @@ Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double siz ? std::min(action->get_user_bound(), cfg_tcp_gamma / (2.0 * action->lat_current_)) : action->get_user_bound()); } +} - if (src_wifi_link != nullptr) - get_maxmin_system()->expand(src_wifi_link->get_constraint(), action->get_variable(), - 1.0 / src_wifi_link->get_host_rate(src)); - if (dst_wifi_link != nullptr) - get_maxmin_system()->expand(dst_wifi_link->get_constraint(), action->get_variable(), - 1.0 / dst_wifi_link->get_host_rate(dst)); +Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double size, double rate) +{ + double latency = 0.0; + std::vector back_route; + std::vector route; + std::unordered_set netzones; - for (auto const* link : route) { - // WIFI links are handled manually just above, so skip them now - if (link->get_sharing_policy() == s4u::Link::SharingPolicy::WIFI) { - xbt_assert(link == src_wifi_link || link == dst_wifi_link, - "Wifi links can only occur at the beginning of the route (meaning that it's attached to the src) or " - "at its end (meaning that it's attached to the dst"); - } else { - get_maxmin_system()->expand(link->get_constraint(), action->get_variable(), 1.0); - } - } + XBT_IN("(%s,%s,%g,%g)", src->get_cname(), dst->get_cname(), size, rate); - if (cfg_crosstraffic) { - XBT_DEBUG("Crosstraffic active: adding backward flow using 5%% of the available bandwidth"); - if (dst_wifi_link != nullptr) - get_maxmin_system()->expand(dst_wifi_link->get_constraint(), action->get_variable(), - .05 / dst_wifi_link->get_host_rate(dst)); - if (src_wifi_link != nullptr) - get_maxmin_system()->expand(src_wifi_link->get_constraint(), action->get_variable(), - .05 / src_wifi_link->get_host_rate(src)); - for (auto const* link : back_route) - if (link->get_sharing_policy() != s4u::Link::SharingPolicy::WIFI) - get_maxmin_system()->expand(link->get_constraint(), action->get_variable(), .05); - // Change concurrency_share here, if you want that cross-traffic is included in the SURF concurrency - // (You would also have to change simgrid::kernel::lmm::Element::get_concurrency()) - // action->getVariable()->set_concurrency_share(2) + bool failed = comm_get_route_info(src, dst, latency, route, back_route, netzones); + + NetworkCm02Action* action = comm_action_create(src, dst, size, route, failed); + action->sharing_penalty_ = latency; + action->latency_ = latency; + action->set_user_bound(rate); + + if (sg_weight_S_parameter > 0) { + action->sharing_penalty_ = + std::accumulate(route.begin(), route.end(), action->sharing_penalty_, [](double total, LinkImpl* const& link) { + return total + sg_weight_S_parameter / link->get_bandwidth(); + }); } + + /* setting bandwidth and latency bounds considering route and configured bw/lat factors */ + comm_action_set_bounds(src, dst, size, action, route, netzones); + + /* creating the maxmin variable associated to this action */ + comm_action_set_variable(action, route, back_route); + + /* expand maxmin system to consider this communication in bw constraint for each link in route and back_route */ + comm_action_expand_constraints(src, dst, action, route, back_route); XBT_OUT(); simgrid::s4u::Link::on_communicate(*action); diff --git a/src/surf/network_cm02.hpp b/src/surf/network_cm02.hpp index 6765921bfb..0cf669d2ee 100644 --- a/src/surf/network_cm02.hpp +++ b/src/surf/network_cm02.hpp @@ -30,6 +30,24 @@ class XBT_PRIVATE NetworkSmpiModel; *********/ class NetworkCm02Model : public NetworkModel { + /** @brief Get route information (2-way) */ + bool comm_get_route_info(const s4u::Host* src, const s4u::Host* dst, /* OUT */ double& latency, + std::vector& route, std::vector& back_route, + std::unordered_set& netzones) const; + /** @brief Create network action for this communication */ + NetworkCm02Action* comm_action_create(s4u::Host* src, s4u::Host* dst, double size, + const std::vector& route, bool failed); + /** @brief Expand link contraint considering this new communication action */ + void comm_action_expand_constraints(const s4u::Host* src, const s4u::Host* dst, NetworkCm02Action* action, + const std::vector& route, const std::vector& back_route); + /** @brief Set communication bounds for latency and bandwidth */ + void comm_action_set_bounds(const s4u::Host* src, const s4u::Host* dst, double size, NetworkCm02Action* action, + const std::vector& route, + const std::unordered_set& netzones); + /** @brief Create maxmin variable in communication action */ + void comm_action_set_variable(NetworkCm02Action* action, const std::vector& route, + const std::vector& back_route); + public: explicit NetworkCm02Model(const std::string& name); LinkImpl* create_link(const std::string& name, const std::vector& bandwidths) final; diff --git a/src/surf/network_interface.cpp b/src/surf/network_interface.cpp index 98e6ad9a9a..2cb1cdfb4d 100644 --- a/src/surf/network_interface.cpp +++ b/src/surf/network_interface.cpp @@ -61,9 +61,6 @@ LinkImpl::LinkImpl(const std::string& name) : Resource_T(name), piface_(this) if (name != "__loopback__") xbt_assert(not s4u::Link::by_name_or_null(name), "Link '%s' declared several times in the platform.", name.c_str()); - latency_.scale = 1; - bandwidth_.scale = 1; - s4u::Engine::get_instance()->link_register(name, &piface_); XBT_DEBUG("Create link '%s'", name.c_str()); } diff --git a/src/surf/network_interface.hpp b/src/surf/network_interface.hpp index 3b0c8933ef..3861316ed3 100644 --- a/src/surf/network_interface.hpp +++ b/src/surf/network_interface.hpp @@ -160,8 +160,8 @@ public: * Profile must contain absolute values */ virtual LinkImpl* set_latency_profile(kernel::profile::Profile* profile); - Metric latency_ = {0.0, 0, nullptr}; - Metric bandwidth_ = {1.0, 0, nullptr}; + Metric latency_ = {0.0, 1, nullptr}; + Metric bandwidth_ = {1.0, 1, nullptr}; }; /********** -- 2.20.1