Move links_ to the netzone their are declared

Starting moving things out Engine class.

Links are stored in the netzone which their were declared.
Exception for __loopback__ link that is a special "model" link.

No changes in external Engine::get_links API.
Internally cleanup so someone can change the API.

-

diff --git a/include/simgrid/kernel/routing/NetZoneImpl.hpp b/include/simgrid/kernel/routing/NetZoneImpl.hpp
index 9890254..047fd61 100644 (file)
--- a/include/simgrid/kernel/routing/NetZoneImpl.hpp
+++ b/include/simgrid/kernel/routing/NetZoneImpl.hpp
@@ -77,6 +77,14 @@ class XBT_PUBLIC NetZoneImpl : public xbt::PropertyHolder {
 
   // our content, as known to our graph routing algorithm (maps vertex_id -> vertex)
   std::vector<kernel::routing::NetPoint*> vertices_;
+  class LinkDeleter {
+  public:
+    void operator()(resource::StandardLinkImpl* link);
+  };
+  std::map<std::string, std::unique_ptr<resource::StandardLinkImpl, LinkDeleter>, std::less<>> links_;
+  /* save split-duplex links separately, keep links_ with only LinkImpl* seen by the user
+   * members of a split-duplex are saved in the links_ */
+  std::map<std::string, std::unique_ptr<resource::SplitDuplexLinkImpl>, std::less<>> split_duplex_links_;
 
   NetZoneImpl* parent_ = nullptr;
   std::vector<NetZoneImpl*> children_; // sub-netzones
@@ -154,14 +162,33 @@ public:
   size_t get_host_count() const;
 
   std::vector<s4u::Link*> get_all_links() const;
+  std::vector<s4u::Link*> get_filtered_links(const std::function<bool(s4u::Link*)>& filter) const;
   size_t get_link_count() const;
 
+  /**
+   * @brief Searches by the link by its name inside this netzone.
+   * Recursively searches in child netzones
+   *
+   * @param name Link name
+   * @return Link object or nullptr if not found
+   */
+  resource::StandardLinkImpl* get_link_by_name_or_null(const std::string& name) const;
+
+  /**
+   * @brief Searches for split-duplex links by its name inside this netzone.
+   * Recursively searches in child netzones
+   *
+   * @param name Split-duplex Link name
+   * @return Link object or nullptr if not found
+   */
+  resource::SplitDuplexLinkImpl* get_split_duplex_link_by_name_or_null(const std::string& name) const;
+
   /** @brief Make a host within that NetZone */
   s4u::Host* create_host(const std::string& name, const std::vector<double>& speed_per_pstate);
   /** @brief Create a disk with the disk model from this NetZone */
   s4u::Disk* create_disk(const std::string& name, double read_bandwidth, double write_bandwidth);
   /** @brief Make a link within that NetZone */
-  virtual s4u::Link* create_link(const std::string& name, const std::vector<double>& bandwidths);
+  s4u::Link* create_link(const std::string& name, const std::vector<double>& bandwidths);
   s4u::SplitDuplexLink* create_split_duplex_link(const std::string& name, const std::vector<double>& bandwidths);
   /** @brief Make a router within that NetZone */
   NetPoint* create_router(const std::string& name);
@@ -220,6 +247,8 @@ private:
   virtual void do_seal()
   { /* obviously nothing to do by default */
   }
+  /** @brief Allows subclasses (wi-fi) to have their own create link method, but keep links_ updated */
+  virtual resource::StandardLinkImpl* do_create_link(const std::string& name, const std::vector<double>& bandwidths);
   void add_child(NetZoneImpl* new_zone);
 };
 } // namespace routing

diff --git a/include/simgrid/kernel/routing/WifiZone.hpp b/include/simgrid/kernel/routing/WifiZone.hpp
index 5fa671a..ee3961b 100644 (file)
--- a/include/simgrid/kernel/routing/WifiZone.hpp
+++ b/include/simgrid/kernel/routing/WifiZone.hpp
@@ -23,6 +23,7 @@ class XBT_PRIVATE WifiZone : public RoutedZone {
   NetPoint* access_point_        = nullptr; // Zone's gateway to the external world
 
   void do_seal() override;
+  resource::StandardLinkImpl* do_create_link(const std::string& name, const std::vector<double>& bandwidths) override;
 
 public:
   using RoutedZone::RoutedZone;
@@ -30,7 +31,6 @@ public:
   WifiZone& operator=(const WifiZone) = delete;
 
   void get_local_route(const NetPoint* src, const NetPoint* dst, Route* into, double* latency) override;
-  s4u::Link* create_link(const std::string& name, const std::vector<double>& bandwidths) override;
   NetPoint* get_access_point() const { return access_point_; }
 };
 } // namespace routing

diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp
index 04f762e..b147a66 100644 (file)
--- a/src/kernel/EngineImpl.cpp
+++ b/src/kernel/EngineImpl.cpp
@@ -176,9 +176,6 @@ EngineImpl::~EngineImpl()
   for (auto const& kv : netpoints_)
     delete kv.second;
 
-  while (not links_.empty())
-    links_.begin()->second->destroy();
-
   for (auto const& kv : mailboxes_)
     delete kv.second;
 
@@ -332,9 +329,6 @@ void EngineImpl::seal_platform() const
     return;
   sealed = true;
 
-  /* sealing resources before run: links */
-  for (auto const& kv : links_)
-    kv.second->get_iface()->seal();
   /* seal netzone root, recursively seal children netzones, hosts and disks */
   netzone_root_->seal();
 }
@@ -396,11 +390,6 @@ void EngineImpl::add_model(std::shared_ptr<resource::Model> model, const std::ve
   models_prio_[model_name] = std::move(model);
 }
 
-void EngineImpl::add_split_duplex_link(const std::string& name, std::unique_ptr<resource::SplitDuplexLinkImpl> link)
-{
-  split_duplex_links_[name] = std::move(link);
-}
-
 /** Wake up all actors waiting for a Surf action to finish */
 void EngineImpl::handle_ended_actions() const
 {

diff --git a/src/kernel/EngineImpl.hpp b/src/kernel/EngineImpl.hpp
index 28d6c57..f02256c 100644 (file)
--- a/src/kernel/EngineImpl.hpp
+++ b/src/kernel/EngineImpl.hpp
@@ -34,10 +34,6 @@ namespace kernel {
 
 class EngineImpl {
   std::map<std::string, s4u::Host*, std::less<>> hosts_;
-  std::map<std::string, resource::StandardLinkImpl*, std::less<>> links_;
-  /* save split-duplex links separately, keep links_ with only LinkImpl* seen by the user
-   * members of a split-duplex are saved in the links_ */
-  std::map<std::string, std::unique_ptr<resource::SplitDuplexLinkImpl>, std::less<>> split_duplex_links_;
   std::unordered_map<std::string, routing::NetPoint*> netpoints_;
   std::unordered_map<std::string, activity::MailboxImpl*> mailboxes_;
 
@@ -141,7 +137,6 @@ public:
   actor::ActorImpl* get_actor_by_pid(aid_t pid);
   void add_actor(aid_t pid, actor::ActorImpl* actor) { actor_list_[pid] = actor; }
   void remove_actor(aid_t pid) { actor_list_.erase(pid); }
-  void add_split_duplex_link(const std::string& name, std::unique_ptr<resource::SplitDuplexLinkImpl> link);
 
 #if SIMGRID_HAVE_MC
   void reset_actor_dynar() { xbt_dynar_reset(actors_vector_); }

diff --git a/src/kernel/resource/StandardLinkImpl.cpp b/src/kernel/resource/StandardLinkImpl.cpp
index ee06be4..8e04682 100644 (file)
--- a/src/kernel/resource/StandardLinkImpl.cpp
+++ b/src/kernel/resource/StandardLinkImpl.cpp
@@ -35,7 +35,6 @@ StandardLinkImpl::StandardLinkImpl(const std::string& name) : LinkImpl(name), pi
 void StandardLinkImpl::destroy()
 {
   s4u::Link::on_destruction(piface_);
-  s4u::Engine::get_instance()->link_unregister(get_name());
   delete this;
 }
 

diff --git a/src/kernel/routing/NetZoneImpl.cpp b/src/kernel/routing/NetZoneImpl.cpp
index fd03d35..0df9d74 100644 (file)
--- a/src/kernel/routing/NetZoneImpl.cpp
+++ b/src/kernel/routing/NetZoneImpl.cpp
@@ -13,9 +13,8 @@
 #include "src/kernel/resource/CpuImpl.hpp"
 #include "src/kernel/resource/DiskImpl.hpp"
 #include "src/kernel/resource/SplitDuplexLinkImpl.hpp"
-#include "src/surf/HostImpl.hpp"
-
 #include "src/kernel/resource/StandardLinkImpl.hpp"
+#include "src/surf/HostImpl.hpp"
 
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(ker_routing, kernel, "Kernel routing-related information");
 
@@ -71,6 +70,11 @@ xbt::signal<void(bool symmetrical, kernel::routing::NetPoint* src, kernel::routi
                  std::vector<kernel::resource::StandardLinkImpl*> const& link_list)>
     NetZoneImpl::on_route_creation;
 
+void NetZoneImpl::LinkDeleter::operator()(resource::StandardLinkImpl* link)
+{
+  link->destroy();
+}
+
 NetZoneImpl::NetZoneImpl(const std::string& name) : piface_(this), name_(name)
 {
   auto* engine = s4u::Engine::get_instance();
@@ -136,15 +140,35 @@ size_t NetZoneImpl::get_host_count() const
   return get_all_hosts().size();
 }
 
+std::vector<s4u::Link*> NetZoneImpl::get_filtered_links(const std::function<bool(s4u::Link*)>& filter) const
+{
+  std::vector<s4u::Link*> filtered_list;
+  for (auto const& kv : links_) {
+    s4u::Link* l = kv.second->get_iface();
+    if (filter(l))
+      filtered_list.push_back(l);
+  }
+
+  for (const auto* child : children_) {
+    auto child_links = child->get_filtered_links(filter);
+    filtered_list.insert(filtered_list.end(), std::make_move_iterator(child_links.begin()),
+                         std::make_move_iterator(child_links.end()));
+  }
+  return filtered_list;
+}
+
 std::vector<s4u::Link*> NetZoneImpl::get_all_links() const
 {
-  return s4u::Engine::get_instance()->get_filtered_links(
-      [this](const s4u::Link* link) { return link->get_impl()->get_englobing_zone() == this; });
+  return get_filtered_links([](s4u::Link*) { return true; });
 }
 
 size_t NetZoneImpl::get_link_count() const
 {
-  return get_all_links().size();
+  size_t total = links_.size();
+  for (const auto* child : children_) {
+    total += child->get_link_count();
+  }
+  return total;
 }
 
 s4u::Host* NetZoneImpl::create_host(const std::string& name, const std::vector<double>& speed_per_pstate)
@@ -161,6 +185,11 @@ s4u::Host* NetZoneImpl::create_host(const std::string& name, const std::vector<d
   return res;
 }
 
+resource::StandardLinkImpl* NetZoneImpl::do_create_link(const std::string& name, const std::vector<double>& bandwidths)
+{
+  return network_model_->create_link(name, bandwidths);
+}
+
 s4u::Link* NetZoneImpl::create_link(const std::string& name, const std::vector<double>& bandwidths)
 {
   xbt_assert(
@@ -168,7 +197,8 @@ s4u::Link* NetZoneImpl::create_link(const std::string& name, const std::vector<d
       "Impossible to create link: %s. Invalid network model: nullptr. Have you set the parent of this NetZone: %s?",
       name.c_str(), get_cname());
   xbt_assert(not sealed_, "Impossible to create link: %s. NetZone %s already sealed", name.c_str(), get_cname());
-  return network_model_->create_link(name, bandwidths)->set_englobing_zone(this)->get_iface();
+  links_[name].reset(do_create_link(name, bandwidths)->set_englobing_zone(this));
+  return links_[name]->get_iface();
 }
 
 s4u::SplitDuplexLink* NetZoneImpl::create_split_duplex_link(const std::string& name,
@@ -180,12 +210,10 @@ s4u::SplitDuplexLink* NetZoneImpl::create_split_duplex_link(const std::string& n
       name.c_str(), get_cname());
   xbt_assert(not sealed_, "Impossible to create link: %s. NetZone %s already sealed", name.c_str(), get_cname());
 
-  auto* link_up                  = network_model_->create_link(name + "_UP", bandwidths)->set_englobing_zone(this);
-  auto* link_down                = network_model_->create_link(name + "_DOWN", bandwidths)->set_englobing_zone(this);
-  auto link                      = std::make_unique<resource::SplitDuplexLinkImpl>(name, link_up, link_down);
-  auto* link_iface               = link->get_iface();
-  EngineImpl::get_instance()->add_split_duplex_link(name, std::move(link));
-  return link_iface;
+  auto* link_up             = create_link(name + "_UP", bandwidths)->get_impl()->set_englobing_zone(this);
+  auto* link_down           = create_link(name + "_DOWN", bandwidths)->get_impl()->set_englobing_zone(this);
+  split_duplex_links_[name] = std::make_unique<resource::SplitDuplexLinkImpl>(name, link_up, link_down);
+  return split_duplex_links_[name]->get_iface();
 }
 
 s4u::Disk* NetZoneImpl::create_disk(const std::string& name, double read_bandwidth, double write_bandwidth)
@@ -251,6 +279,36 @@ std::vector<resource::StandardLinkImpl*> NetZoneImpl::get_link_list_impl(const s
   return links;
 }
 
+resource::StandardLinkImpl* NetZoneImpl::get_link_by_name_or_null(const std::string& name) const
+{
+  auto link_it = links_.find(name);
+  if (link_it != links_.end())
+    return link_it->second.get();
+
+  for (const auto* child : children_) {
+    auto* link = child->get_link_by_name_or_null(name);
+    if (link)
+      return link;
+  }
+
+  return nullptr;
+}
+
+resource::SplitDuplexLinkImpl* NetZoneImpl::get_split_duplex_link_by_name_or_null(const std::string& name) const
+{
+  auto link_it = split_duplex_links_.find(name);
+  if (link_it != split_duplex_links_.end())
+    return link_it->second.get();
+
+  for (const auto* child : children_) {
+    auto* link = child->get_split_duplex_link_by_name_or_null(name);
+    if (link)
+      return link;
+  }
+
+  return nullptr;
+}
+
 void NetZoneImpl::add_route(NetPoint* /*src*/, NetPoint* /*dst*/, NetPoint* /*gw_src*/, NetPoint* /*gw_dst*/,
                             const std::vector<s4u::LinkInRoute>& /*link_list_*/, bool /*symmetrical*/)
 {
@@ -544,6 +602,11 @@ void NetZoneImpl::seal()
   for (auto* host : get_all_hosts()) {
     host->seal();
   }
+
+  /* sealing links */
+  for (auto const& kv : links_)
+    kv.second->get_iface()->seal();
+
   for (auto* sub_net : get_children()) {
     sub_net->seal();
   }

diff --git a/src/kernel/routing/WifiZone.cpp b/src/kernel/routing/WifiZone.cpp
index 81596ca..e193b8b 100644 (file)
--- a/src/kernel/routing/WifiZone.cpp
+++ b/src/kernel/routing/WifiZone.cpp
@@ -45,14 +45,14 @@ void WifiZone::get_local_route(const NetPoint* src, const NetPoint* dst, Route*
   }
 }
 
-s4u::Link* WifiZone::create_link(const std::string& name, const std::vector<double>& bandwidths)
+resource::StandardLinkImpl* WifiZone::do_create_link(const std::string& name, const std::vector<double>& bandwidths)
 {
   xbt_assert(wifi_link_ == nullptr,
              "WIFI netzone %s contains more than one link. Please only declare one, the wifi link.", get_cname());
 
-  wifi_link_ = get_network_model()->create_wifi_link(name, bandwidths)->set_englobing_zone(this);
+  wifi_link_ = get_network_model()->create_wifi_link(name, bandwidths);
   wifi_link_->set_sharing_policy(s4u::Link::SharingPolicy::WIFI, {});
-  return wifi_link_->get_iface();
+  return wifi_link_;
 }
 } // namespace routing
 } // namespace kernel

diff --git a/src/s4u/s4u_Engine.cpp b/src/s4u/s4u_Engine.cpp
index d9da9e1..ef3c17e 100644 (file)
--- a/src/s4u/s4u_Engine.cpp
+++ b/src/s4u/s4u_Engine.cpp
@@ -15,6 +15,8 @@
 #include "mc/mc.h"
 #include "src/instr/instr_private.hpp"
 #include "src/kernel/EngineImpl.hpp"
+#include "src/kernel/resource/SplitDuplexLinkImpl.hpp"
+#include "src/kernel/resource/StandardLinkImpl.hpp"
 #include "src/mc/mc_replay.hpp"
 #include "xbt/config.hpp"
 
@@ -236,25 +238,30 @@ Host* Engine::host_by_name_or_null(const std::string& name) const
  */
 Link* Engine::link_by_name(const std::string& name) const
 {
-  auto link = pimpl->links_.find(name);
-  if (link == pimpl->links_.end())
+  auto* link = link_by_name_or_null(name);
+  if (not link)
     throw std::invalid_argument(std::string("Link not found: ") + name);
-  return link->second->get_iface();
+  return link;
 }
 
 SplitDuplexLink* Engine::split_duplex_link_by_name(const std::string& name) const
 {
-  auto link = pimpl->split_duplex_links_.find(name);
-  if (link == pimpl->split_duplex_links_.end())
+  auto* link_impl = pimpl->netzone_root_ ? pimpl->netzone_root_->get_split_duplex_link_by_name_or_null(name) : nullptr;
+  if (not link_impl)
     throw std::invalid_argument(std::string("Link not found: ") + name);
-  return link->second->get_iface();
+  return link_impl->get_iface();
 }
 
 /** @brief Find a link from its name (or nullptr if that link does not exist) */
 Link* Engine::link_by_name_or_null(const std::string& name) const
 {
-  auto link = pimpl->links_.find(name);
-  return link == pimpl->links_.end() ? nullptr : link->second->get_iface();
+  Link* link = nullptr;
+  if (pimpl->netzone_root_) {
+    auto* link_impl = pimpl->netzone_root_->get_link_by_name_or_null(name);
+    if (link_impl)
+      link = link_impl->get_iface();
+  }
+  return link;
 }
 
 /** @brief Find a mailbox from its name or create one if it does not exist) */
@@ -274,38 +281,43 @@ Mailbox* Engine::mailbox_by_name_or_create(const std::string& name) const
 
 void Engine::link_register(const std::string& name, const Link* link)
 {
-  pimpl->links_[name] = link->get_impl();
+  //  pimpl->links_[name] = link->get_impl(); //FIXME
 }
 
 void Engine::link_unregister(const std::string& name)
 {
-  pimpl->links_.erase(name);
+  //  pimpl->links_.erase(name); FIXME
 }
 
 /** @brief Returns the amount of links in the platform */
 size_t Engine::get_link_count() const
 {
-  return pimpl->links_.size();
+  int count = 0;
+  if (pimpl->netzone_root_) {
+    count += pimpl->netzone_root_->get_link_count();
+    /* keep behavior where internal __loopback__ link from network model is given to user */
+    count += pimpl->netzone_root_->get_network_model()->loopback_ ? 1 : 0;
+  }
+  return count;
 }
 
 /** @brief Returns the list of all links found in the platform */
 std::vector<Link*> Engine::get_all_links() const
 {
-  std::vector<Link*> res;
-  for (auto const& kv : pimpl->links_)
-    res.push_back(kv.second->get_iface());
-  return res;
+  return get_filtered_links([](Link*) { return true; });
 }
 
 std::vector<Link*> Engine::get_filtered_links(const std::function<bool(Link*)>& filter) const
 {
-  std::vector<Link*> filtered_list;
-  for (auto const& kv : pimpl->links_) {
-    Link* l = kv.second->get_iface();
-    if (filter(l))
-      filtered_list.push_back(l);
+  std::vector<Link*> res;
+  if (pimpl->netzone_root_) {
+    res = pimpl->netzone_root_->get_filtered_links(filter);
+    /* keep behavior where internal __loopback__ link from network model is given to user */
+    if (pimpl->netzone_root_->get_network_model()->loopback_ &&
+        filter(pimpl->netzone_root_->get_network_model()->loopback_->get_iface()))
+      res.push_back(pimpl->netzone_root_->get_network_model()->loopback_->get_iface());
   }
-  return filtered_list;
+  return res;
 }
 
 size_t Engine::get_actor_count() const

diff --git a/src/surf/network_cm02.cpp b/src/surf/network_cm02.cpp
index 010b39c..2da2664 100644 (file)
--- a/src/surf/network_cm02.cpp
+++ b/src/surf/network_cm02.cpp
@@ -100,7 +100,7 @@ NetworkCm02Model::NetworkCm02Model(const std::string& name) : NetworkModel(name)
   loopback_ = create_link("__loopback__", {config::get_value<double>("network/loopback-bw")});
   loopback_->set_sharing_policy(s4u::Link::SharingPolicy::FATPIPE, {});
   loopback_->set_latency(config::get_value<double>("network/loopback-lat"));
-  loopback_->seal();
+  loopback_->get_iface()->seal();
 }
 
 void NetworkCm02Model::check_lat_factor_cb()