From 2d37e348a09783cda723c7019640ee69de168324 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 3 Feb 2019 15:57:04 +0100 Subject: [PATCH] sanitize the OOP of kernel::profile * Add a Profile::next() returning the next event of the list. This opens the way to future profile kinds, based on statistical generator instead of explicit lists. Earlier, FES was advancing the internal pointer of the Profile. * Change FutureEvtSet::add_trace(Profile, Resource) into Profile::schedule(FutureEvtSet, Resource). This way, Profile knows about its FES and there is no need for a global (also making testing easier), and FES does not have to mess with the internals of Profile. --- src/kernel/resource/Resource.cpp | 2 +- src/kernel/resource/profile/trace_mgr.cpp | 77 +++++++++++-------- src/kernel/resource/profile/trace_mgr.hpp | 7 +- .../resource/profile/trace_mgr_test.cpp | 6 +- src/surf/cpu_interface.cpp | 2 +- src/surf/cpu_ti.cpp | 6 +- src/surf/network_interface.cpp | 4 +- 7 files changed, 61 insertions(+), 43 deletions(-) diff --git a/src/kernel/resource/Resource.cpp b/src/kernel/resource/Resource.cpp index b975e924c9..e835455196 100644 --- a/src/kernel/resource/Resource.cpp +++ b/src/kernel/resource/Resource.cpp @@ -67,7 +67,7 @@ void Resource::set_state_profile(profile::Profile* profile) { xbt_assert(state_event_ == nullptr, "Cannot set a second state profile to %s", get_cname()); - state_event_ = future_evt_set.add_trace(profile, this); + state_event_ = profile->schedule(&future_evt_set, this); } kernel::lmm::Constraint* Resource::get_constraint() const diff --git a/src/kernel/resource/profile/trace_mgr.cpp b/src/kernel/resource/profile/trace_mgr.cpp index 151f1f1744..7a273835ee 100644 --- a/src/kernel/resource/profile/trace_mgr.cpp +++ b/src/kernel/resource/profile/trace_mgr.cpp @@ -41,6 +41,44 @@ Profile::Profile() event_list.push_back(val); } Profile::~Profile() = default; + +/** @brief Register this profile for that resource onto that FES, + * and get an iterator over the integrated trace */ +Event* Profile::schedule(FutureEvtSet* fes, resource::Resource* resource) +{ + Event* event = new Event(); + event->profile = this; + event->idx = 0; + event->resource = resource; + event->free_me = false; + + xbt_assert((event->idx < event_list.size()), "Your profile should have at least one event!"); + + fes_ = fes; + fes_->add_event(0.0 /* start time */, event); + + return event; +} + +/** @brief Gets the next event from a profile */ +DatedValue Profile::next(Event* event) +{ + double event_date = fes_->next_date(); + DatedValue dateVal = event_list.at(event->idx); + + if (event->idx < event_list.size() - 1) { + fes_->add_event(event_date + dateVal.date_, event); + event->idx++; + } else if (dateVal.date_ > 0) { /* Last element. Shall we loop? */ + fes_->add_event(event_date + dateVal.date_, event); + event->idx = 1; /* idx=0 is a placeholder to store when events really start */ + } else { /* If we don't loop, we don't need this event anymore */ + event->free_me = true; + } + + return dateVal; +} + Profile* Profile::from_string(std::string name, std::string input, double periodicity) { int linecount = 0; @@ -108,23 +146,13 @@ FutureEvtSet::~FutureEvtSet() } } -/** @brief Registers a new trace into the future event set, and get an iterator over the integrated trace */ -Event* FutureEvtSet::add_trace(Profile* profile, resource::Resource* resource) +/** @brief Schedules an event to a future date */ +void FutureEvtSet::add_event(double date, Event* evt) { - Event* event = new Event(); - event->profile = profile; - event->idx = 0; - event->resource = resource; - event->free_me = false; - - xbt_assert((event->idx < profile->event_list.size()), "Your profile should have at least one event!"); - - heap_.emplace(0.0 /* start time */, event); - - return event; + heap_.emplace(date, evt); } -/** @brief returns the date of the next occurring event */ +/** @brief returns the date of the next occurring event (or -1 if empty) */ double FutureEvtSet::next_date() const { return heap_.empty() ? -1.0 : heap_.top().first; @@ -134,30 +162,17 @@ double FutureEvtSet::next_date() const Event* FutureEvtSet::pop_leq(double date, double* value, resource::Resource** resource) { double event_date = next_date(); - if (event_date > date) + if (event_date > date || heap_.empty()) return nullptr; - if (heap_.empty()) - return nullptr; Event* event = heap_.top().second; - heap_.pop(); - Profile* profile = event->profile; - *resource = event->resource; - - DatedValue dateVal = profile->event_list.at(event->idx); + DatedValue dateVal = profile->next(event); + *resource = event->resource; *value = dateVal.value_; - if (event->idx < profile->event_list.size() - 1) { - heap_.emplace(event_date + dateVal.date_, event); - event->idx++; - } else if (dateVal.date_ > 0) { /* Last element. Shall we loop? */ - heap_.emplace(event_date + dateVal.date_, event); - event->idx = 1; /* idx=0 is a placeholder to store when events really start */ - } else { /* If we don't loop, we don't need this event anymore */ - event->free_me = true; - } + heap_.pop(); return event; } diff --git a/src/kernel/resource/profile/trace_mgr.hpp b/src/kernel/resource/profile/trace_mgr.hpp index f4c2fe6192..03d7e72ef1 100644 --- a/src/kernel/resource/profile/trace_mgr.hpp +++ b/src/kernel/resource/profile/trace_mgr.hpp @@ -77,11 +77,16 @@ public: /** Creates an empty trace */ explicit Profile(); virtual ~Profile(); + Event* schedule(FutureEvtSet* fes, resource::Resource* resource); + DatedValue next(Event* event); static Profile* from_file(std::string path); static Profile* from_string(std::string name, std::string input, double periodicity); // private: std::vector event_list; + +private: + FutureEvtSet* fes_ = nullptr; }; /** @brief Future Event Set (collection of iterators over the traces) @@ -92,7 +97,7 @@ public: virtual ~FutureEvtSet(); double next_date() const; Event* pop_leq(double date, double* value, resource::Resource** resource); - Event* add_trace(Profile* profile, resource::Resource* resource); + void add_event(double date, Event* evt); private: typedef std::pair Qelt; diff --git a/src/kernel/resource/profile/trace_mgr_test.cpp b/src/kernel/resource/profile/trace_mgr_test.cpp index 76604b52ec..d09af5ee79 100644 --- a/src/kernel/resource/profile/trace_mgr_test.cpp +++ b/src/kernel/resource/profile/trace_mgr_test.cpp @@ -32,7 +32,6 @@ public: static std::vector trace2vector(const char* str) { std::vector res; - simgrid::kernel::profile::Profile* trace = simgrid::kernel::profile::Profile::from_string("TheName", str, 0); XBT_VERB("---------------------------------------------------------"); XBT_VERB("data>>\n%s< trace2vector(const char MockedResource daResource; simgrid::kernel::profile::FutureEvtSet fes; - simgrid::kernel::profile::Event* insertedIt = fes.add_trace(trace, &daResource); + simgrid::kernel::profile::Event* insertedIt = trace->schedule(&fes, &daResource); while (fes.next_date() <= 20.0 && fes.next_date() >= 0) { thedate = fes.next_date(); @@ -63,9 +62,6 @@ static std::vector trace2vector(const char return res; } -/* Fails in a way that is difficult to test: xbt_assert should become throw -BOOST_AUTO_TEST_CASE(no_evt_noloop) { -}*/ TEST_CASE("kernel::profile: Resource profiles, defining the external load", "kernel::profile") { diff --git a/src/surf/cpu_interface.cpp b/src/surf/cpu_interface.cpp index 5cf965e155..0c7181333b 100644 --- a/src/surf/cpu_interface.cpp +++ b/src/surf/cpu_interface.cpp @@ -138,7 +138,7 @@ void Cpu::set_speed_profile(kernel::profile::Profile* profile) { xbt_assert(speed_.event == nullptr, "Cannot set a second speed trace to Host %s", host_->get_cname()); - speed_.event = future_evt_set.add_trace(profile, this); + speed_.event = profile->schedule(&future_evt_set, this); } diff --git a/src/surf/cpu_ti.cpp b/src/surf/cpu_ti.cpp index 4f25126e5d..35c0c0316e 100644 --- a/src/surf/cpu_ti.cpp +++ b/src/surf/cpu_ti.cpp @@ -375,8 +375,10 @@ void CpuTi::set_speed_profile(kernel::profile::Profile* profile) /* add a fake trace event if periodicity == 0 */ if (profile && profile->event_list.size() > 1) { kernel::profile::DatedValue val = profile->event_list.back(); - if (val.date_ < 1e-12) - speed_.event = future_evt_set.add_trace(new simgrid::kernel::profile::Profile(), this); + if (val.date_ < 1e-12) { + simgrid::kernel::profile::Profile* prof = new simgrid::kernel::profile::Profile(); + speed_.event = prof->schedule(&future_evt_set, this); + } } } diff --git a/src/surf/network_interface.cpp b/src/surf/network_interface.cpp index 999afe5c8b..6d2a96e063 100644 --- a/src/surf/network_interface.cpp +++ b/src/surf/network_interface.cpp @@ -148,13 +148,13 @@ void LinkImpl::on_bandwidth_change() void LinkImpl::set_bandwidth_profile(profile::Profile* profile) { xbt_assert(bandwidth_.event == nullptr, "Cannot set a second bandwidth profile to Link %s", get_cname()); - bandwidth_.event = future_evt_set.add_trace(profile, this); + bandwidth_.event = profile->schedule(&future_evt_set, this); } void LinkImpl::set_latency_profile(profile::Profile* profile) { xbt_assert(latency_.event == nullptr, "Cannot set a second latency profile to Link %s", get_cname()); - latency_.event = future_evt_set.add_trace(profile, this); + latency_.event = profile->schedule(&future_evt_set, this); } /********** -- 2.20.1