From 6ee0c147e010bf1d2da12813b60593e805e49f3b Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Thu, 20 Jul 2023 19:56:36 +0200 Subject: [PATCH] Implement Exec::wait_any on top of ActivitySet and deprecate one method - Also fix a bit the behavior when ActivitySet::wait_any_for() results in a timeout. Believe it or not, this was not tested and plainly wrong until now. Now, it's a bit too complex, as the simcall returns -1 and forces the caller to detect it and throw TimeoutException manually. It should be cleaned as in wait_for, somehow. - The C API is not deprecated yet, because ActivitySet has no C API yet - The C++ API is not fully deprecated yet, because the unit test is not converted to ActivitySet yet. Still a long way to go to clean it up :( --- examples/c/exec-waitany/exec-waitany.c | 2 +- examples/c/exec-waitany/exec-waitany.tesh | 3 +- include/simgrid/exec.h | 2 ++ include/simgrid/s4u/Exec.hpp | 24 +++++++++++---- src/s4u/s4u_ActivitySet.cpp | 5 +-- src/s4u/s4u_Exec.cpp | 37 ++++++++++++++++++----- 6 files changed, 54 insertions(+), 19 deletions(-) diff --git a/examples/c/exec-waitany/exec-waitany.c b/examples/c/exec-waitany/exec-waitany.c index bfff989960..2dd9d86471 100644 --- a/examples/c/exec-waitany/exec-waitany.c +++ b/examples/c/exec-waitany/exec-waitany.c @@ -48,7 +48,7 @@ static void worker(int argc, char* argv[]) pos = sg_exec_wait_any(pending_execs, pending_execs_count); if (pos < 0) { - XBT_INFO("Do not wait any longer for an activity"); + XBT_INFO("Do not wait any longer for an activity (timeout received)"); pending_execs_count = 0; } else { XBT_INFO("Activity at position %zd is complete", pos); diff --git a/examples/c/exec-waitany/exec-waitany.tesh b/examples/c/exec-waitany/exec-waitany.tesh index 2ea0293135..57fb2bab93 100644 --- a/examples/c/exec-waitany/exec-waitany.tesh +++ b/examples/c/exec-waitany/exec-waitany.tesh @@ -1,6 +1,5 @@ #!/usr/bin/env tesh -! output sort 19 $ ${bindir:=.}/c-exec-waitany ${platfdir}/multicore_machine.xml "--log=root.fmt:[%10.6r]%e[%14P]%e%m%n" > [ 0.000000] [ worker] Activity Exec-0 has started for 1 seconds > [ 0.000000] [worker_timeout] Activity Exec-0 has started for 1 seconds @@ -16,7 +15,7 @@ $ ${bindir:=.}/c-exec-waitany ${platfdir}/multicore_machine.xml "--log=root.fmt: > [ 3.000000] [worker_timeout] 1 activities remain pending > [ 3.000000] [ worker] Activity at position 1 is complete > [ 3.000000] [ worker] 1 activities remain pending -> [ 7.000000] [worker_timeout] Do not wait any longer for an activity +> [ 7.000000] [worker_timeout] Do not wait any longer for an activity (timeout received) > [ 7.000000] [worker_timeout] 0 activities remain pending > [ 8.000000] [ worker] Activity at position 0 is complete > [ 8.000000] [ worker] 0 activities remain pending diff --git a/include/simgrid/exec.h b/include/simgrid/exec.h index 42e413b7f3..aed0390e0e 100644 --- a/include/simgrid/exec.h +++ b/include/simgrid/exec.h @@ -24,7 +24,9 @@ XBT_PUBLIC void sg_exec_cancel(sg_exec_t exec); XBT_PUBLIC int sg_exec_test(sg_exec_t exec); XBT_PUBLIC sg_error_t sg_exec_wait(sg_exec_t exec); XBT_PUBLIC sg_error_t sg_exec_wait_for(sg_exec_t exec, double timeout); +// XBT_ATTRIB_DEPRECATED_v339("Please use sg_activity_set instead") TODO: C bindings of ActivitySet XBT_PUBLIC ssize_t sg_exec_wait_any_for(sg_exec_t* execs, size_t count, double timeout); +// XBT_ATTRIB_DEPRECATED_v339("Please use sg_activity_set instead") TODO: C bindings of ActivitySet XBT_PUBLIC ssize_t sg_exec_wait_any(sg_exec_t* execs, size_t count); SG_END_DECL diff --git a/include/simgrid/s4u/Exec.hpp b/include/simgrid/s4u/Exec.hpp index b8e728a985..482f4bd7a9 100644 --- a/include/simgrid/s4u/Exec.hpp +++ b/include/simgrid/s4u/Exec.hpp @@ -50,12 +50,6 @@ public: /*! \static Initiate the creation of an Exec. Setters have to be called afterwards */ static ExecPtr init(); - /*! \static take a vector of s4u::ExecPtr and return when one of them is finished. - * The return value is the rank of the first finished ExecPtr. */ - static ssize_t wait_any(const std::vector& execs) { return wait_any_for(execs, -1); } - /*! \static Same as wait_any, but with a timeout. If the timeout occurs, parameter last is returned.*/ - static ssize_t wait_any_for(const std::vector& execs, double timeout); - /** @brief On sequential executions, returns the amount of flops that remain to be done; This cannot be used on * parallel executions. */ double get_remaining() const override; @@ -81,6 +75,24 @@ public: double get_cost() const; bool is_parallel() const { return parallel_; } bool is_assigned() const override; + +#ifndef DOXYGEN + static ssize_t deprecated_wait_any_for(const std::vector& execs, + double timeout); // XBT_ATTRIB_DEPRECATED_v339 + /*! \static take a vector of s4u::ExecPtr and return when one of them is finished. + * The return value is the rank of the first finished ExecPtr. */ + XBT_ATTRIB_DEPRECATED_v339("Please use ActivitySet instead") static ssize_t + wait_any(const std::vector& execs) + { + return deprecated_wait_any_for(execs, -1); + } + /*! \static Same as wait_any, but with a timeout. If the timeout occurs, parameter last is returned.*/ + // XBT_ATTRIB_DEPRECATED_v339("Please use ActivitySet instead") TODO: update activity-lifecycle/testing_test-wait.cpp + static ssize_t wait_any_for(const std::vector& execs, double timeout) + { + return deprecated_wait_any_for(execs, timeout); + } +#endif }; } // namespace simgrid::s4u diff --git a/src/s4u/s4u_ActivitySet.cpp b/src/s4u/s4u_ActivitySet.cpp index 4d704206ca..6e7ededfde 100644 --- a/src/s4u/s4u_ActivitySet.cpp +++ b/src/s4u/s4u_ActivitySet.cpp @@ -6,6 +6,7 @@ #include "src/kernel/activity/ActivityImpl.hpp" #include "src/kernel/actor/ActorImpl.hpp" #include "src/kernel/actor/CommObserver.hpp" +#include #include #include @@ -76,8 +77,8 @@ ActivityPtr ActivitySet::wait_any_for(double timeout) observer.get_timeout()); }, &observer); - xbt_assert(changed_pos != -1, - "ActivityImpl::wait_any_for is not supposed to return -1 but instead to raise exceptions"); + if (changed_pos == -1) + throw TimeoutException(XBT_THROW_POINT, "Timeouted"); auto ret = activities_.at(changed_pos); erase(ret); diff --git a/src/s4u/s4u_Exec.cpp b/src/s4u/s4u_Exec.cpp index 49f935e7ed..4a641e2c8e 100644 --- a/src/s4u/s4u_Exec.cpp +++ b/src/s4u/s4u_Exec.cpp @@ -6,6 +6,7 @@ #include "simgrid/simix.hpp" #include #include +#include #include #include @@ -51,12 +52,23 @@ Exec* Exec::do_start() return this; } -ssize_t Exec::wait_any_for(const std::vector& execs, double timeout) +ssize_t Exec::deprecated_wait_any_for(const std::vector& execs, double timeout) // XBT_ATTRIB_DEPRECATED_v339 { - std::vector activities; + if (execs.empty()) + return -1; + ActivitySet set; for (const auto& exec : execs) - activities.push_back(boost::dynamic_pointer_cast(exec)); - return Activity::wait_any_for(activities, timeout); + set.push(exec); + try { + auto* ret = set.wait_any_for(timeout).get(); + for (size_t i = 0; i < execs.size(); i++) + if (execs[i].get() == ret) + return i; + + } catch (TimeoutException& e) { + return -1; + } + return -1; } /** @brief change the execution bound @@ -319,18 +331,27 @@ sg_error_t sg_exec_wait_for(sg_exec_t exec, double timeout) return status; } -ssize_t sg_exec_wait_any(sg_exec_t* execs, size_t count) +ssize_t sg_exec_wait_any(sg_exec_t* execs, size_t count) // XBT_ATTRIB_DEPRECATED_v339 { - return sg_exec_wait_any_for(execs, count, -1.0); + std::vector s4u_execs; + for (size_t i = 0; i < count; i++) + s4u_execs.emplace_back(execs[i], false); + + ssize_t pos = simgrid::s4u::Exec::deprecated_wait_any_for(s4u_execs, -1.0); + for (size_t i = 0; i < count; i++) { + if (pos != -1 && static_cast(pos) != i) + s4u_execs[i]->add_ref(); + } + return pos; } -ssize_t sg_exec_wait_any_for(sg_exec_t* execs, size_t count, double timeout) +ssize_t sg_exec_wait_any_for(sg_exec_t* execs, size_t count, double timeout) // XBT_ATTRIB_DEPRECATED_v339 { std::vector s4u_execs; for (size_t i = 0; i < count; i++) s4u_execs.emplace_back(execs[i], false); - ssize_t pos = simgrid::s4u::Exec::wait_any_for(s4u_execs, timeout); + ssize_t pos = simgrid::s4u::Exec::deprecated_wait_any_for(s4u_execs, timeout); for (size_t i = 0; i < count; i++) { if (pos != -1 && static_cast(pos) != i) s4u_execs[i]->add_ref(); -- 2.20.1