From a39a90a68780cd9dd43fadcacf9bce1d3c3df26d Mon Sep 17 00:00:00 2001 From: Frederic Suter Date: Mon, 1 Mar 2021 14:14:55 +0100 Subject: [PATCH] Consider Link latency as an optional parameter --- .../simgrid/kernel/routing/NetZoneImpl.hpp | 2 +- include/simgrid/kernel/routing/WifiZone.hpp | 2 +- include/simgrid/s4u/Link.hpp | 2 +- src/kernel/routing/NetZoneImpl.cpp | 12 ++--------- src/kernel/routing/VivaldiZone.cpp | 6 ++++-- src/kernel/routing/WifiZone.cpp | 4 ++-- src/s4u/s4u_Link.cpp | 4 ++++ src/surf/network_cm02.cpp | 21 +++++++++---------- src/surf/network_cm02.hpp | 6 +++--- src/surf/network_constant.cpp | 2 +- src/surf/network_constant.hpp | 2 +- src/surf/network_interface.cpp | 16 +++++++++++++- src/surf/network_interface.hpp | 9 +++++--- src/surf/network_ns3.hpp | 2 +- src/surf/network_wifi.cpp | 2 -- src/surf/network_wifi.hpp | 2 +- src/surf/ptask_L07.cpp | 18 +++++++--------- src/surf/ptask_L07.hpp | 7 +++---- src/surf/sg_platf.cpp | 6 +++++- 19 files changed, 69 insertions(+), 56 deletions(-) diff --git a/include/simgrid/kernel/routing/NetZoneImpl.hpp b/include/simgrid/kernel/routing/NetZoneImpl.hpp index 62c25c3f91..a353e60ff3 100644 --- a/include/simgrid/kernel/routing/NetZoneImpl.hpp +++ b/include/simgrid/kernel/routing/NetZoneImpl.hpp @@ -122,7 +122,7 @@ public: /** @brief Make a host within that NetZone */ s4u::Host* create_host(const std::string& name, const std::vector& speed_per_pstate, int core_count); /** @brief Make a link within that NetZone */ - virtual s4u::Link* create_link(const std::string& name, const std::vector& bandwidths, double latency, + virtual s4u::Link* create_link(const std::string& name, const std::vector& bandwidths, s4u::Link::SharingPolicy policy); /** @brief Creates a new route in this NetZone */ virtual void add_bypass_route(NetPoint* src, NetPoint* dst, NetPoint* gw_src, NetPoint* gw_dst, diff --git a/include/simgrid/kernel/routing/WifiZone.hpp b/include/simgrid/kernel/routing/WifiZone.hpp index f08aa1f57b..23728e800d 100644 --- a/include/simgrid/kernel/routing/WifiZone.hpp +++ b/include/simgrid/kernel/routing/WifiZone.hpp @@ -27,7 +27,7 @@ public: void seal() override; void get_local_route(NetPoint* src, NetPoint* dst, RouteCreationArgs* into, double* latency) override; - s4u::Link* create_link(const std::string& name, const std::vector& bandwidths, double latency, + s4u::Link* create_link(const std::string& name, const std::vector& bandwidths, s4u::Link::SharingPolicy policy) override; NetPoint* get_access_point() {return access_point_;} diff --git a/include/simgrid/s4u/Link.hpp b/include/simgrid/s4u/Link.hpp index 58e26124c7..134a1f87a6 100644 --- a/include/simgrid/s4u/Link.hpp +++ b/include/simgrid/s4u/Link.hpp @@ -90,7 +90,7 @@ public: void turn_on(); bool is_on() const; void turn_off(); - + void seal(); /** Setup the profile with states events (ON or OFF). The profile must contain boolean values. */ void set_state_profile(kernel::profile::Profile* profile); /** Setup the profile with bandwidth events (peak speed changes due to external load). diff --git a/src/kernel/routing/NetZoneImpl.cpp b/src/kernel/routing/NetZoneImpl.cpp index 215fe4748b..dc473bd9bd 100644 --- a/src/kernel/routing/NetZoneImpl.cpp +++ b/src/kernel/routing/NetZoneImpl.cpp @@ -65,18 +65,10 @@ int NetZoneImpl::get_host_count() const return count; } -s4u::Link* NetZoneImpl::create_link(const std::string& name, const std::vector& bandwidths, double latency, +s4u::Link* NetZoneImpl::create_link(const std::string& name, const std::vector& bandwidths, s4u::Link::SharingPolicy policy) { - static double last_warned_latency = sg_surf_precision; - if (latency != 0.0 && latency < last_warned_latency) { - XBT_WARN("Latency for link %s is smaller than surf/precision (%g < %g)." - " For more accuracy, consider setting \"--cfg=surf/precision:%g\".", - name.c_str(), latency, sg_surf_precision, latency); - last_warned_latency = latency; - } - - auto* l = surf_network_model->create_link(name, bandwidths, latency, policy); + auto* l = surf_network_model->create_link(name, bandwidths, policy); return l->get_iface(); } diff --git a/src/kernel/routing/VivaldiZone.cpp b/src/kernel/routing/VivaldiZone.cpp index 575d87e41a..4b392c56ae 100644 --- a/src/kernel/routing/VivaldiZone.cpp +++ b/src/kernel/routing/VivaldiZone.cpp @@ -76,9 +76,11 @@ void VivaldiZone::set_peer_link(NetPoint* netpoint, double bw_in, double bw_out, std::string link_up = "link_" + netpoint->get_name() + "_UP"; std::string link_down = "link_" + netpoint->get_name() + "_DOWN"; resource::LinkImpl* linkUp = - network_model_->create_link(link_up, std::vector(1, bw_out), 0, s4u::Link::SharingPolicy::SHARED); + network_model_->create_link(link_up, std::vector(1, bw_out), s4u::Link::SharingPolicy::SHARED); + linkUp->seal(); resource::LinkImpl* linkDown = - network_model_->create_link(link_down, std::vector(1, bw_in), 0, s4u::Link::SharingPolicy::SHARED); + network_model_->create_link(link_down, std::vector(1, bw_in), s4u::Link::SharingPolicy::SHARED); + linkDown->seal(); private_links_.insert({netpoint->id(), {linkUp, linkDown}}); } diff --git a/src/kernel/routing/WifiZone.cpp b/src/kernel/routing/WifiZone.cpp index 52965610d3..1985a9593e 100644 --- a/src/kernel/routing/WifiZone.cpp +++ b/src/kernel/routing/WifiZone.cpp @@ -55,7 +55,7 @@ void WifiZone::get_local_route(NetPoint* src, NetPoint* dst, RouteCreationArgs* } } } -s4u::Link* WifiZone::create_link(const std::string& name, const std::vector& bandwidths, double latency, +s4u::Link* WifiZone::create_link(const std::string& name, const std::vector& bandwidths, s4u::Link::SharingPolicy policy) { xbt_assert(wifi_link_ == nullptr, @@ -63,7 +63,7 @@ s4u::Link* WifiZone::create_link(const std::string& name, const std::vectorget_impl(); return s4u_link; } diff --git a/src/s4u/s4u_Link.cpp b/src/s4u/s4u_Link.cpp index 59cee96d9e..26797d1e9b 100644 --- a/src/s4u/s4u_Link.cpp +++ b/src/s4u/s4u_Link.cpp @@ -103,6 +103,10 @@ void Link::turn_off() { simgrid::kernel::actor::simcall([this]() { this->pimpl_->turn_off(); }); } +void Link::seal() +{ + simgrid::kernel::actor::simcall([this]() { this->pimpl_->seal(); }); +} bool Link::is_on() const { diff --git a/src/surf/network_cm02.cpp b/src/surf/network_cm02.cpp index 3265a9919c..a63e6767ee 100644 --- a/src/surf/network_cm02.cpp +++ b/src/surf/network_cm02.cpp @@ -89,18 +89,18 @@ NetworkCm02Model::NetworkCm02Model() set_maxmin_system(new lmm::System(select)); loopback_ = NetworkCm02Model::create_link("__loopback__", std::vector{simgrid::config::get_value("network/loopback-bw")}, - simgrid::config::get_value("network/loopback-lat"), - s4u::Link::SharingPolicy::FATPIPE); + s4u::Link::SharingPolicy::FATPIPE)->set_latency(simgrid::config::get_value("network/loopback-lat")); + loopback_->seal(); } -LinkImpl* NetworkCm02Model::create_link(const std::string& name, const std::vector& bandwidths, double latency, +LinkImpl* NetworkCm02Model::create_link(const std::string& name, const std::vector& bandwidths, s4u::Link::SharingPolicy policy) { if (policy == s4u::Link::SharingPolicy::WIFI) return new NetworkWifiLink(this, name, bandwidths, get_maxmin_system()); xbt_assert(bandwidths.size() == 1, "Non-WIFI links must use only 1 bandwidth."); - return new NetworkCm02Link(this, name, bandwidths[0], latency, policy, get_maxmin_system()); + return new NetworkCm02Link(this, name, bandwidths[0], policy, get_maxmin_system()); } void NetworkCm02Model::update_actions_state_lazy(double now, double /*delta*/) @@ -308,20 +308,15 @@ Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double siz /************ * Resource * ************/ -NetworkCm02Link::NetworkCm02Link(NetworkCm02Model* model, const std::string& name, double bandwidth, double latency, +NetworkCm02Link::NetworkCm02Link(NetworkCm02Model* model, const std::string& name, double bandwidth, s4u::Link::SharingPolicy policy, kernel::lmm::System* system) : LinkImpl(model, name, system->constraint_new(this, sg_bandwidth_factor * bandwidth)) { bandwidth_.scale = 1.0; bandwidth_.peak = bandwidth; - latency_.scale = 1.0; - latency_.peak = latency; - if (policy == s4u::Link::SharingPolicy::FATPIPE) get_constraint()->unshare(); - - simgrid::s4u::Link::on_creation(*get_iface()); } void NetworkCm02Link::apply_event(kernel::profile::Event* triggered, double value) @@ -374,14 +369,17 @@ void NetworkCm02Link::set_bandwidth(double value) } } -void NetworkCm02Link::set_latency(double value) +LinkImpl* NetworkCm02Link::set_latency(double value) { + latency_check(value); + double delta = value - latency_.peak; const kernel::lmm::Variable* var; const kernel::lmm::Element* elem = nullptr; const kernel::lmm::Element* nextelem = nullptr; int numelem = 0; + latency_.scale = 1.0; latency_.peak = value; while ((var = get_constraint()->get_variable_safe(&elem, &nextelem, &numelem))) { @@ -404,6 +402,7 @@ void NetworkCm02Link::set_latency(double value) if (not action->is_suspended()) get_model()->get_maxmin_system()->update_variable_penalty(action->get_variable(), action->sharing_penalty_); } + return this; } /********** diff --git a/src/surf/network_cm02.hpp b/src/surf/network_cm02.hpp index 759cc8679c..a4a881172f 100644 --- a/src/surf/network_cm02.hpp +++ b/src/surf/network_cm02.hpp @@ -32,7 +32,7 @@ class NetworkCm02Model : public NetworkModel { public: NetworkCm02Model(); ~NetworkCm02Model() override = default; - LinkImpl* create_link(const std::string& name, const std::vector& bandwidths, double latency, + LinkImpl* create_link(const std::string& name, const std::vector& bandwidths, s4u::Link::SharingPolicy policy) override; void update_actions_state_lazy(double now, double delta) override; void update_actions_state_full(double now, double delta) override; @@ -45,12 +45,12 @@ public: class NetworkCm02Link : public LinkImpl { public: - NetworkCm02Link(NetworkCm02Model* model, const std::string& name, double bandwidth, double latency, + NetworkCm02Link(NetworkCm02Model* model, const std::string& name, double bandwidth, s4u::Link::SharingPolicy policy, lmm::System* system); ~NetworkCm02Link() override = default; void apply_event(kernel::profile::Event* event, double value) override; void set_bandwidth(double value) override; - void set_latency(double value) override; + LinkImpl* set_latency(double value) override; }; /********** diff --git a/src/surf/network_constant.cpp b/src/surf/network_constant.cpp index 9d06f9e023..37dc9beef2 100644 --- a/src/surf/network_constant.cpp +++ b/src/surf/network_constant.cpp @@ -28,7 +28,7 @@ NetworkConstantModel::NetworkConstantModel() : NetworkModel(Model::UpdateAlgo::F } LinkImpl* NetworkConstantModel::create_link(const std::string& name, const std::vector& /*bandwidth*/, - double /*latency*/, s4u::Link::SharingPolicy) + s4u::Link::SharingPolicy) { xbt_die("Refusing to create the link %s: there is no link in the Constant network model. " "Please remove any link from your platform (and switch to routing='None')", diff --git a/src/surf/network_constant.hpp b/src/surf/network_constant.hpp index a5336b3f68..751b2256dd 100644 --- a/src/surf/network_constant.hpp +++ b/src/surf/network_constant.hpp @@ -21,7 +21,7 @@ public: double next_occurring_event(double now) override; void update_actions_state(double now, double delta) override; - LinkImpl* create_link(const std::string& name, const std::vector& bws, double lat, + LinkImpl* create_link(const std::string& name, const std::vector& bws, s4u::Link::SharingPolicy policy) override; }; diff --git a/src/surf/network_interface.cpp b/src/surf/network_interface.cpp index 496383dd26..958ef73a50 100644 --- a/src/surf/network_interface.cpp +++ b/src/surf/network_interface.cpp @@ -117,6 +117,17 @@ s4u::Link::SharingPolicy LinkImpl::get_sharing_policy() const return get_constraint()->get_sharing_policy(); } +void LinkImpl::latency_check(double latency) +{ + static double last_warned_latency = sg_surf_precision; + if (latency != 0.0 && latency < last_warned_latency) { + XBT_WARN("Latency for link %s is smaller than surf/precision (%g < %g)." + " For more accuracy, consider setting \"--cfg=surf/precision:%g\".", + get_cname(), latency, sg_surf_precision, latency); + last_warned_latency = latency; + } +} + void LinkImpl::turn_on() { if (not is_on()) { @@ -143,7 +154,10 @@ void LinkImpl::turn_off() } } } - +void LinkImpl::seal() +{ + simgrid::s4u::Link::on_creation(*get_iface()); +} void LinkImpl::on_bandwidth_change() const { s4u::Link::on_bandwidth_change(this->piface_); diff --git a/src/surf/network_interface.hpp b/src/surf/network_interface.hpp index f092a17651..0e64eb16be 100644 --- a/src/surf/network_interface.hpp +++ b/src/surf/network_interface.hpp @@ -45,10 +45,9 @@ public: * * @param name The name of the Link * @param bandwidth The initial bandwidth of the Link in bytes per second - * @param latency The initial latency of the Link in seconds * @param policy The sharing policy of the Link */ - virtual LinkImpl* create_link(const std::string& name, const std::vector& bandwidths, double latency, + virtual LinkImpl* create_link(const std::string& name, const std::vector& bandwidths, s4u::Link::SharingPolicy policy) = 0; /** @@ -119,6 +118,8 @@ protected: public: void destroy(); // Must be called instead of the destructor + void latency_check(double latency); + /** @brief Public interface */ const s4u::Link* get_iface() const { return &piface_; } s4u::Link* get_iface() { return &piface_; } @@ -133,7 +134,7 @@ public: double get_latency() const; /** @brief Update the latency in seconds of current Link */ - virtual void set_latency(double value) = 0; + virtual LinkImpl* set_latency(double value) = 0; /** @brief The sharing policy */ virtual s4u::Link::SharingPolicy get_sharing_policy() const; @@ -144,6 +145,8 @@ public: void turn_on() override; void turn_off() override; + void seal(); + void on_bandwidth_change() const; virtual void diff --git a/src/surf/network_ns3.hpp b/src/surf/network_ns3.hpp index 5a04c04699..c1a2e4471f 100644 --- a/src/surf/network_ns3.hpp +++ b/src/surf/network_ns3.hpp @@ -38,7 +38,7 @@ public: void apply_event(profile::Event* event, double value) override; void set_bandwidth(double) override { THROW_UNIMPLEMENTED; } - void set_latency(double) override { THROW_UNIMPLEMENTED; } + LinkImpl* set_latency(double) override { THROW_UNIMPLEMENTED; } void set_bandwidth_profile(profile::Profile* profile) override; void set_latency_profile(profile::Profile* profile) override; s4u::Link::SharingPolicy get_sharing_policy() const override { return sharing_policy_; } diff --git a/src/surf/network_wifi.cpp b/src/surf/network_wifi.cpp index d741ce48e1..8fd7e08051 100644 --- a/src/surf/network_wifi.cpp +++ b/src/surf/network_wifi.cpp @@ -23,8 +23,6 @@ NetworkWifiLink::NetworkWifiLink(NetworkCm02Model* model, const std::string& nam { for (auto bandwidth : bandwidths) bandwidths_.push_back({bandwidth, 1.0, nullptr}); - - simgrid::s4u::Link::on_creation(*get_iface()); } void NetworkWifiLink::set_host_rate(const s4u::Host* host, int rate_level) diff --git a/src/surf/network_wifi.hpp b/src/surf/network_wifi.hpp index dc16347976..867045c3b8 100644 --- a/src/surf/network_wifi.hpp +++ b/src/surf/network_wifi.hpp @@ -52,7 +52,7 @@ public: s4u::Link::SharingPolicy get_sharing_policy() const override; void apply_event(kernel::profile::Event*, double) override { THROW_UNIMPLEMENTED; } void set_bandwidth(double) override { THROW_UNIMPLEMENTED; } - void set_latency(double) override { THROW_UNIMPLEMENTED; } + LinkImpl* set_latency(double) override { THROW_UNIMPLEMENTED; } void refresh_decay_bandwidths(); bool toggle_decay_model(); int get_host_count() const; diff --git a/src/surf/ptask_L07.cpp b/src/surf/ptask_L07.cpp index 71126721dc..d8757a07e3 100644 --- a/src/surf/ptask_L07.cpp +++ b/src/surf/ptask_L07.cpp @@ -59,8 +59,8 @@ NetworkL07Model::NetworkL07Model(HostL07Model* hmodel, kernel::lmm::System* sys) set_maxmin_system(sys); loopback_ = NetworkL07Model::create_link("__loopback__", std::vector{simgrid::config::get_value("network/loopback-bw")}, - simgrid::config::get_value("network/loopback-lat"), - s4u::Link::SharingPolicy::FATPIPE); + s4u::Link::SharingPolicy::FATPIPE)->set_latency(simgrid::config::get_value("network/loopback-lat")); + loopback_->seal(); } NetworkL07Model::~NetworkL07Model() @@ -228,10 +228,10 @@ kernel::resource::Cpu* CpuL07Model::create_cpu(s4u::Host* host, const std::vecto } kernel::resource::LinkImpl* NetworkL07Model::create_link(const std::string& name, const std::vector& bandwidths, - double latency, s4u::Link::SharingPolicy policy) + s4u::Link::SharingPolicy policy) { xbt_assert(bandwidths.size() == 1, "Non WIFI link must have only 1 bandwidth."); - return new LinkL07(this, name, bandwidths[0], latency, policy); + return new LinkL07(this, name, bandwidths[0], policy); } /************ @@ -246,17 +246,13 @@ CpuL07::CpuL07(CpuL07Model* model, simgrid::s4u::Host* host, const std::vectorget_maxmin_system()->constraint_new(this, bandwidth)) { bandwidth_.peak = bandwidth; - latency_.peak = latency; if (policy == s4u::Link::SharingPolicy::FATPIPE) get_constraint()->unshare(); - - s4u::Link::on_creation(*get_iface()); } kernel::resource::CpuAction* CpuL07::execution_start(double size) @@ -362,8 +358,9 @@ void LinkL07::set_bandwidth(double value) get_model()->get_maxmin_system()->update_constraint_bound(get_constraint(), bandwidth_.peak * bandwidth_.scale); } -void LinkL07::set_latency(double value) +kernel::resource::LinkImpl* LinkL07::set_latency(double value) { + latency_check(value); const kernel::lmm::Variable* var; L07Action *action; const kernel::lmm::Element* elem = nullptr; @@ -373,6 +370,7 @@ void LinkL07::set_latency(double value) action = static_cast(var->get_id()); action->updateBound(); } + return this; } LinkL07::~LinkL07() = default; diff --git a/src/surf/ptask_L07.hpp b/src/surf/ptask_L07.hpp index 18baba818e..de06c8ab23 100644 --- a/src/surf/ptask_L07.hpp +++ b/src/surf/ptask_L07.hpp @@ -64,7 +64,7 @@ public: NetworkL07Model& operator=(const NetworkL07Model&) = delete; ~NetworkL07Model() override; kernel::resource::LinkImpl* create_link(const std::string& name, const std::vector& bandwidths, - double latency, s4u::Link::SharingPolicy policy) override; + s4u::Link::SharingPolicy policy) override; kernel::resource::Action* communicate(s4u::Host* src, s4u::Host* dst, double size, double rate) override; @@ -97,15 +97,14 @@ protected: class LinkL07 : public kernel::resource::LinkImpl { public: - LinkL07(NetworkL07Model* model, const std::string& name, double bandwidth, double latency, - s4u::Link::SharingPolicy policy); + LinkL07(NetworkL07Model* model, const std::string& name, double bandwidth, s4u::Link::SharingPolicy policy); LinkL07(const LinkL07&) = delete; LinkL07& operator=(const LinkL07&) = delete; ~LinkL07() override; bool is_used() const override; void apply_event(kernel::profile::Event* event, double value) override; void set_bandwidth(double value) override; - void set_latency(double value) override; + LinkImpl* set_latency(double value) override; }; /********** diff --git a/src/surf/sg_platf.cpp b/src/surf/sg_platf.cpp index 2db602b584..8ffe7dbf18 100644 --- a/src/surf/sg_platf.cpp +++ b/src/surf/sg_platf.cpp @@ -109,7 +109,9 @@ simgrid::kernel::routing::NetPoint* sg_platf_new_router(const std::string& name, static void sg_platf_new_link(const simgrid::kernel::routing::LinkCreationArgs* args, const std::string& link_name) { - simgrid::s4u::Link* link = routing_get_current()->create_link(link_name, args->bandwidths, args->latency, args->policy); + simgrid::s4u::Link* link = routing_get_current()->create_link(link_name, args->bandwidths, args->policy); + if (args->policy != simgrid::s4u::Link::SharingPolicy::WIFI) + link->get_impl()->set_latency(args->latency); if (args->properties) link->set_properties(*args->properties); @@ -121,6 +123,8 @@ static void sg_platf_new_link(const simgrid::kernel::routing::LinkCreationArgs* l->set_bandwidth_profile(args->bandwidth_trace); if (args->state_trace) l->set_state_profile(args->state_trace); + + link->seal(); } void sg_platf_new_link(const simgrid::kernel::routing::LinkCreationArgs* link) -- 2.20.1