From 142a7ceb870c0b948957245e34af7569cd8f4223 Mon Sep 17 00:00:00 2001 From: Florian Goth Date: Mon, 9 Aug 2021 18:36:28 +0200 Subject: [PATCH 1/9] first round --- src/libmarqov/marqovqueue.h | 27 ++++++++++++++++++++------ src/libmarqov/marqovqueue_helpers.h | 15 ++++++++++++--- src/libmarqov/marqovscheduler.h | 30 +++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/libmarqov/marqovqueue.h b/src/libmarqov/marqovqueue.h index b0c8963..49b94a8 100644 --- a/src/libmarqov/marqovqueue.h +++ b/src/libmarqov/marqovqueue.h @@ -29,6 +29,10 @@ namespace ThreadPool { resize(initc); } + /** Move constructor of queue. + * + */ + Queue(Queue&& rhs) : semaphores{}, queue(semaphores.work), workers{std::move(rhs.workers)} {} ~Queue() { flags.stop.store(true); @@ -103,7 +107,8 @@ namespace ThreadPool { Toggle stop;///< A flag for denoting that the threads should stop. Toggle prune;///< A flag for denoting that the threads should terminate. - Flags() : stop(false), prune(false) {} + Flags() noexcept : stop(false), prune(false) {} + Flags(const Flags&) noexcept = default; } flags;///< flags /** A helper structure for some threadpool statistics. @@ -112,11 +117,11 @@ namespace ThreadPool { auint enqueued; ///< count how many threads are in the queue. auint assigned; ///< count how many threads are worked on. - /** Construct our statistics with initial values. + /** Construct our statistics with initial zero values. */ - Stats() : enqueued(0), assigned(0) {} + Stats() noexcept : enqueued(0), assigned(0) {} } stats;///< Actual instance for our statistics. - + /** A helper structure to bundle the bookkeeping of workers. */ struct Workers @@ -129,9 +134,19 @@ namespace ThreadPool * * @param tc How many workers do we intend to have. */ - Workers(const uint tc) : count(0), target_count(tc) {} + Workers(const uint tc) noexcept : busy_mtx{}, busy{}, count(0), target_count(tc) {} + /** Move Constructor + * + * Takes ownership of other threads. + */ + Workers(Workers&& rhs) : busy_mtx{}, busy{}, count(rhs.count.load()), target_count(rhs.target_count.load()) + { + std::lock_guard lk(rhs.busy_mtx); + std::swap(busy, rhs.busy); + rhs.count.store(0); + } } workers; ///< An instance of the worker bookkeeping structure. - + /** trigger synchronization */ void sync() diff --git a/src/libmarqov/marqovqueue_helpers.h b/src/libmarqov/marqovqueue_helpers.h index 0ab095c..4003e5a 100644 --- a/src/libmarqov/marqovqueue_helpers.h +++ b/src/libmarqov/marqovqueue_helpers.h @@ -57,8 +57,13 @@ namespace ThreadPool */ struct Semaphore { + /** Default constructor. + * + * Cannot be declared noexcept since condition variables may throw. + */ + Semaphore() = default; std::mutex mtx; ///< The mutex that we internally use for setting up a lock. - std::condition_variable cv; ///< A condition variable for notifxing waiting threads. + std::condition_variable cv; ///< A condition variable for notifying waiting threads. /** Blocks the current thread until the condition variable is woken up. */ void wait() @@ -66,7 +71,7 @@ namespace ThreadPool std::unique_lock lk(mtx); cv.wait(lk); } - + /** Wait until the condition variable is woken up and check the predicate. * * @param _check A predicate that is to be checked. @@ -76,7 +81,7 @@ namespace ThreadPool std::unique_lock lk(mtx); cv.wait(lk, _check); } - + /** Wait until the condition variable is woken up, check the predicate, or just wait for a certain amount of time. * * @tparam T The type of the time difference. @@ -124,6 +129,10 @@ namespace ThreadPool * @param _s We take the semaphore as argument that we will use to notify waiting threads about changes to the queue. */ ThreadSafeQueue(Semaphore& _s) : s(_s) {} + /** Deletes Move constructor. + */ + ThreadSafeQueue(const ThreadSafeQueue&& rhs) = delete; + ThreadSafeQueue(const ThreadSafeQueue&) = delete; /** Add something to the end of the queue. * * @param t the new item that we put into the queue. diff --git a/src/libmarqov/marqovscheduler.h b/src/libmarqov/marqovscheduler.h index aa9e59d..bb51ec2 100644 --- a/src/libmarqov/marqovscheduler.h +++ b/src/libmarqov/marqovscheduler.h @@ -229,6 +229,30 @@ namespace MARQOV } masterstop = true; } + + Scheduler() = delete; + Scheduler(const Scheduler&) = delete; + /** Move Copy Constructor + * + * The other object over whose resources we take ownership. + * mutexes are a bit odd here. We don't reuse the other mutexes but use our own. + * + * @param rhs the other object + */ + + Scheduler(Scheduler&& rhs) : mutexes{}, maxpt(rhs.maxpt), ptqueue(std::move(rhs.ptqueue)), ptplan{}, masterstop(rhs.masterstop), + masterwork{}, workqueue(masterwork), imvector(std::move(rhs.simvector)),taskqueue{std::move(rhs.taskqueue)}, gamekernels{} + { + + std::swap(ptplan, rhs.ptplan); + std::swap(simvector, rhs.simvector); + + std::swap(gamekernels, rhs.gamekernels); + if (rhs.taskqueue.tasks_enqueued() > 0 || rhs.taskqueue.tasks_assigned() > 0) + throw std::runtime_error("[MARQOV::Scheduler] invalid assignment"); + } + Scheduler& operator=(const Scheduler&) = delete; + Scheduler& operator=(Scheduler&& ) = delete; private: /** * Simstate helper class @@ -363,5 +387,11 @@ namespace MARQOV typedef decltype(makeCore(std::declval(), std::declval())) MarqovType; typedef Scheduler MarqovScheduler; ///< Holds the type of a scheduler for these simulations. }; + + template + auto makeScheduler(const Parameters& args, uint t) + { + return typename GetSchedulerType::MarqovScheduler(1); + } }; #endif -- GitLab From a5c99f01de5f37a8e90abe78cd122e0fead0f8b2 Mon Sep 17 00:00:00 2001 From: Florian Goth Date: Mon, 9 Aug 2021 18:36:49 +0200 Subject: [PATCH 2/9] first round --- src/libmarqov/marqovscheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libmarqov/marqovscheduler.h b/src/libmarqov/marqovscheduler.h index bb51ec2..2142f5c 100644 --- a/src/libmarqov/marqovscheduler.h +++ b/src/libmarqov/marqovscheduler.h @@ -241,7 +241,7 @@ namespace MARQOV */ Scheduler(Scheduler&& rhs) : mutexes{}, maxpt(rhs.maxpt), ptqueue(std::move(rhs.ptqueue)), ptplan{}, masterstop(rhs.masterstop), - masterwork{}, workqueue(masterwork), imvector(std::move(rhs.simvector)),taskqueue{std::move(rhs.taskqueue)}, gamekernels{} + masterwork{}, workqueue(masterwork), simvector(std::move(rhs.simvector)),taskqueue{std::move(rhs.taskqueue)}, gamekernels{} { std::swap(ptplan, rhs.ptplan); -- GitLab From 7529861ad7828b746a08c43c1cd5bea6e52a3ee0 Mon Sep 17 00:00:00 2001 From: Florian Goth Date: Mon, 9 Aug 2021 19:21:46 +0200 Subject: [PATCH 3/9] fix rightful clang error --- src/libmarqov/marqovqueue.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libmarqov/marqovqueue.h b/src/libmarqov/marqovqueue.h index 49b94a8..ceb81ef 100644 --- a/src/libmarqov/marqovqueue.h +++ b/src/libmarqov/marqovqueue.h @@ -108,7 +108,7 @@ namespace ThreadPool Toggle stop;///< A flag for denoting that the threads should stop. Toggle prune;///< A flag for denoting that the threads should terminate. Flags() noexcept : stop(false), prune(false) {} - Flags(const Flags&) noexcept = default; + Flags(const Flags& rhs) noexcept : stop(rhs.stop.load()), prune(rhs.prune.load()) {} } flags;///< flags /** A helper structure for some threadpool statistics. -- GitLab From d0ab93ff607b227af97e72929c6a96911a58603d Mon Sep 17 00:00:00 2001 From: Florian Goth Date: Mon, 9 Aug 2021 20:19:48 +0200 Subject: [PATCH 4/9] docs and some convenience. Also use it in the heisenberg tutorial. --- src/libmarqov/marqovscheduler.h | 26 +++++++++++++++++++------- src/mysimpleheisenbergthreaded.cpp | 4 +--- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/libmarqov/marqovscheduler.h b/src/libmarqov/marqovscheduler.h index 2142f5c..e7cadec 100644 --- a/src/libmarqov/marqovscheduler.h +++ b/src/libmarqov/marqovscheduler.h @@ -212,7 +212,7 @@ namespace MARQOV * @param maxptsteps How many parallel tempering steps do we do. * @param nthreads how many threads should be used. If not specified defaults to what is reported by the OS. */ - Scheduler(int maxptsteps, uint nthreads = 0) : maxpt(maxptsteps), masterstop(false), masterwork{}, + Scheduler(uint maxptsteps = 1, uint nthreads = 0) : maxpt(maxptsteps), masterstop(false), masterwork{}, workqueue(masterwork), taskqueue(((nthreads == 0)?std::thread::hardware_concurrency():nthreads)) {} @@ -229,8 +229,8 @@ namespace MARQOV } masterstop = true; } - - Scheduler() = delete; + +// Scheduler() = delete; //FIXME: If this would be present, the call to Scheduler() would be ambiguous Scheduler(const Scheduler&) = delete; /** Move Copy Constructor * @@ -387,11 +387,23 @@ namespace MARQOV typedef decltype(makeCore(std::declval(), std::declval())) MarqovType; typedef Scheduler MarqovScheduler; ///< Holds the type of a scheduler for these simulations. }; - - template - auto makeScheduler(const Parameters& args, uint t) + + /** A helper function to figure out the type of the scheduler and actually create it. + * + * @tparam Hamiltonian The Hamiltonian to use for the simulations. + * @tparam Lattice The lattice to use for the simulations. + * @tparam RNGType The Random Number Generator to use. + * @tparam Parameters the tuple that makes up the arguments for a simulation. + * @tparam Ts a parameter pack of the remaining arguements to the scheduler. + * + * @param args A dummy parameter to get rid of template magic on the user end. + * @param ts The remaining arguments for the scheduler. + * + */ + template + auto makeScheduler(const Parameters& args, Ts&&... ts) { - return typename GetSchedulerType::MarqovScheduler(1); + return typename GetSchedulerType::MarqovScheduler(std::forward(ts)...); } }; #endif diff --git a/src/mysimpleheisenbergthreaded.cpp b/src/mysimpleheisenbergthreaded.cpp index 508880a..6ed26ff 100644 --- a/src/mysimpleheisenbergthreaded.cpp +++ b/src/mysimpleheisenbergthreaded.cpp @@ -87,9 +87,7 @@ int main() } // Set up the MARQOV schedular - typedef typename GetSchedulerType::MarqovScheduler SchedulerType; - SchedulerType sched(1); //MARQOV makes use of all available threads by default. - + auto sched = makeScheduler(args); // Feed parameters to the scheduler which creates the simulations for(auto p : v) sched.createSimfromParameter(p); -- GitLab From 35749700da7e8832a21c85af40d05aeaba9c7fa8 Mon Sep 17 00:00:00 2001 From: Florian Goth Date: Mon, 9 Aug 2021 20:30:58 +0200 Subject: [PATCH 5/9] Use it in another model --- src/main.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index d1c37df..b6f1047 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -333,16 +333,9 @@ void selectsim() typedef Ising Hamiltonian; typedef ConstantCoordinationLattice Lattice; - typedef std::tuple, MARQOV::Config, typename decltype(hp)::value_type > ParameterType; - typedef typename GetSchedulerType::MarqovScheduler SchedulerType; - - // Lattice size loop for (std::size_t j=0; j 0 || rhs.taskqueue.tasks_assigned() > 0) - throw std::runtime_error("[MARQOV::Scheduler] invalid assignment"); + throw std::runtime_error("[MARQOV::CXX11Scheduler] invalid assignment"); } - Scheduler& operator=(const Scheduler&) = delete; - Scheduler& operator=(Scheduler&& ) = delete; + CXX11Scheduler& operator=(const CXX11Scheduler&) = delete; + CXX11Scheduler& operator=(CXX11Scheduler&& ) = delete; private: /** * Simstate helper class @@ -430,9 +430,9 @@ namespace MARQOV MPI_Comm_rank(marqov_COMM, &myrank); } MPIScheduler(const MPIScheduler&) = delete; - ~MPIScheduler() { -// MPI_Finalize(); - } + ~MPIScheduler() = default; + MPIScheduler& operator=(const MPIScheduler&) = delete; + MPIScheduler& operator=(MPIScheduler&& ) = delete; private: int rrctr; MPI_Comm marqov_COMM;///< our own MPI communicator. -- GitLab From cb67f65ffd0c05f6e468542c98e27d53da29c017 Mon Sep 17 00:00:00 2001 From: Florian Goth Date: Tue, 17 Aug 2021 15:40:39 +0200 Subject: [PATCH 8/9] try the move constructor for the mpischeduler --- src/libmarqov/marqovscheduler.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libmarqov/marqovscheduler.h b/src/libmarqov/marqovscheduler.h index 3e781da..6734bfe 100644 --- a/src/libmarqov/marqovscheduler.h +++ b/src/libmarqov/marqovscheduler.h @@ -429,6 +429,13 @@ namespace MARQOV MPI_Comm_size(marqov_COMM, &nr_nodes); MPI_Comm_rank(marqov_COMM, &myrank); } + /** Move Copy Constructor + * + * The other object over whose resources we take ownership. + * + * @param rhs the other object + */ + MPIScheduler(MPIScheduler&& rhs) : rrctr(rhs.rrctr), marqov_COMM(rhs.marqov_COMM), MASTER(rhs.MASTER), myScheduler(std::move(rhs.myScheduler)), myrank(rhs.myrank), nr_nodes(rhs.nr_nodes), maxpt(rhs.maxpt) {} MPIScheduler(const MPIScheduler&) = delete; ~MPIScheduler() = default; MPIScheduler& operator=(const MPIScheduler&) = delete; -- GitLab From 2bab9adc5c3a2cb340b7eb2415a09ec9f15e07c6 Mon Sep 17 00:00:00 2001 From: Florian Goth Date: Tue, 17 Aug 2021 16:13:26 +0200 Subject: [PATCH 9/9] fix compilation --- src/libmarqov/marqovscheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libmarqov/marqovscheduler.h b/src/libmarqov/marqovscheduler.h index 6734bfe..204e05d 100644 --- a/src/libmarqov/marqovscheduler.h +++ b/src/libmarqov/marqovscheduler.h @@ -435,7 +435,7 @@ namespace MARQOV * * @param rhs the other object */ - MPIScheduler(MPIScheduler&& rhs) : rrctr(rhs.rrctr), marqov_COMM(rhs.marqov_COMM), MASTER(rhs.MASTER), myScheduler(std::move(rhs.myScheduler)), myrank(rhs.myrank), nr_nodes(rhs.nr_nodes), maxpt(rhs.maxpt) {} + MPIScheduler(MPIScheduler&& rhs) : rrctr(rhs.rrctr), marqov_COMM(rhs.marqov_COMM), myScheduler(std::move(rhs.myScheduler)), myrank(rhs.myrank), nr_nodes(rhs.nr_nodes), maxpt(rhs.maxpt) {} MPIScheduler(const MPIScheduler&) = delete; ~MPIScheduler() = default; MPIScheduler& operator=(const MPIScheduler&) = delete; -- GitLab