From: Carl Hetherington Date: Tue, 23 Nov 2021 23:16:15 +0000 (+0100) Subject: Simplify and fix job scheduler, especially with respect to the priority system. X-Git-Tag: v2.15.178~10 X-Git-Url: https://main.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=d41a59b6ef7a8c935f182d498ae4df0bdd66ba02 Simplify and fix job scheduler, especially with respect to the priority system. --- diff --git a/src/lib/job_manager.cc b/src/lib/job_manager.cc index d8c0b02f2..608df7ef0 100644 --- a/src/lib/job_manager.cc +++ b/src/lib/job_manager.cc @@ -159,39 +159,30 @@ JobManager::scheduler () boost::mutex::scoped_lock lm (_mutex); - while (true) { - bool have_new = false; - bool have_running = false; - for (auto i: _jobs) { - if (i->running()) { - have_running = true; - } - if (i->is_new()) { - have_new = true; - } - } - - if ((!have_running && have_new) || _terminate) { - break; - } - - _empty_condition.wait (lm); - } - if (_terminate) { break; } + bool have_running = false; for (auto i: _jobs) { - if (i->is_new()) { - _connections.push_back (i->FinishedImmediate.connect(bind(&JobManager::job_finished, this))); - i->start (); + if (have_running && i->running()) { + i->pause_by_priority(); + } else if (!have_running && (i->is_new() || i->paused_by_priority())) { + if (i->is_new()) { + _connections.push_back (i->FinishedImmediate.connect(bind(&JobManager::job_finished, this))); + i->start (); + } else { + i->resume (); + } emit (boost::bind (boost::ref (ActiveJobsChanged), _last_active_job, i->json_name())); _last_active_job = i->json_name (); - /* Only start one job at once */ - break; + have_running = true; + } else if (!have_running && i->running()) { + have_running = true; } } + + _empty_condition.wait (lm); } } @@ -319,35 +310,9 @@ JobManager::increase_priority (shared_ptr job) } if (changed) { - priority_changed (); - } -} - - -void -JobManager::priority_changed () -{ - { - boost::mutex::scoped_lock lm (_mutex); - - bool first = true; - for (auto i: _jobs) { - if (first) { - if (i->is_new ()) { - i->start (); - } else if (i->paused_by_priority ()) { - i->resume (); - } - first = false; - } else { - if (i->running ()) { - i->pause_by_priority (); - } - } - } + _empty_condition.notify_all (); + emit (boost::bind(boost::ref(JobsReordered))); } - - emit (boost::bind(boost::ref(JobsReordered))); } @@ -370,7 +335,8 @@ JobManager::decrease_priority (shared_ptr job) } if (changed) { - priority_changed (); + _empty_condition.notify_all (); + emit (boost::bind(boost::ref(JobsReordered))); } } diff --git a/src/lib/job_manager.h b/src/lib/job_manager.h index ff5800aa8..71db33fd6 100644 --- a/src/lib/job_manager.h +++ b/src/lib/job_manager.h @@ -96,7 +96,6 @@ private: ~JobManager (); void scheduler (); void start (); - void priority_changed (); void job_finished (); mutable boost::mutex _mutex; diff --git a/test/job_manager_test.cc b/test/job_manager_test.cc new file mode 100644 index 000000000..1baa5767e --- /dev/null +++ b/test/job_manager_test.cc @@ -0,0 +1,130 @@ +/* + Copyright (C) 2012-2021 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + DCP-o-matic is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with DCP-o-matic. If not, see . + +*/ + + +/** @file test/job_manager_test.cc + * @brief Test JobManager. + * @ingroup selfcontained + */ + + +#include "lib/cross.h" +#include "lib/job.h" +#include "lib/job_manager.h" +#include + + +using std::make_shared; +using std::shared_ptr; +using std::string; +using std::vector; + + +class TestJob : public Job +{ +public: + explicit TestJob (shared_ptr film) + : Job (film) + { + + } + + ~TestJob () + { + stop_thread (); + } + + void set_finished_ok () { + set_state (FINISHED_OK); + } + + void set_finished_error () { + set_state (FINISHED_ERROR); + } + + void run () + { + while (true) { + if (finished ()) { + return; + } + } + } + + string name () const { + return ""; + } + + string json_name () const { + return ""; + } +}; + + +BOOST_AUTO_TEST_CASE (job_manager_test1) +{ + shared_ptr film; + + /* Single job */ + auto a = make_shared(film); + + JobManager::instance()->add (a); + dcpomatic_sleep_seconds (1); + BOOST_CHECK (a->running()); + a->set_finished_ok (); + dcpomatic_sleep_seconds (2); + BOOST_CHECK (a->finished_ok()); +} + + +BOOST_AUTO_TEST_CASE (job_manager_test2) +{ + shared_ptr film; + + vector> jobs; + for (int i = 0; i < 16; ++i) { + auto job = make_shared(film); + jobs.push_back (job); + JobManager::instance()->add (job); + } + + dcpomatic_sleep_seconds (1); + BOOST_CHECK (jobs[0]->running()); + jobs[0]->set_finished_ok(); + + dcpomatic_sleep_seconds (1); + BOOST_CHECK (!jobs[0]->running()); + BOOST_CHECK (jobs[1]->running()); + + /* Push our jobs[5] to the top of the list */ + for (int i = 0; i < 5; ++i) { + JobManager::instance()->increase_priority(jobs[5]); + } + + dcpomatic_sleep_seconds (1); + for (int i = 0; i < 16; ++i) { + BOOST_CHECK (i == 5 ? jobs[i]->running() : !jobs[i]->running()); + } + + for (auto job: jobs) { + job->set_finished_ok(); + } +} + diff --git a/test/job_test.cc b/test/job_test.cc deleted file mode 100644 index 7fb240843..000000000 --- a/test/job_test.cc +++ /dev/null @@ -1,93 +0,0 @@ -/* - Copyright (C) 2012-2021 Carl Hetherington - - This file is part of DCP-o-matic. - - DCP-o-matic is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - DCP-o-matic is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with DCP-o-matic. If not, see . - -*/ - - -/** @file test/job_test.cc - * @brief Test Job and JobManager. - * @ingroup selfcontained - */ - - -#include "lib/cross.h" -#include "lib/job.h" -#include "lib/job_manager.h" -#include - - -using std::make_shared; -using std::shared_ptr; -using std::string; - - -class TestJob : public Job -{ -public: - explicit TestJob (shared_ptr film) - : Job (film) - { - - } - - ~TestJob () - { - stop_thread (); - } - - void set_finished_ok () { - set_state (FINISHED_OK); - } - - void set_finished_error () { - set_state (FINISHED_ERROR); - } - - void run () - { - while (true) { - if (finished ()) { - return; - } - } - } - - string name () const { - return ""; - } - - string json_name () const { - return ""; - } -}; - - -BOOST_AUTO_TEST_CASE (job_manager_test) -{ - shared_ptr film; - - /* Single job */ - auto a = make_shared(film); - - JobManager::instance()->add (a); - dcpomatic_sleep_seconds (1); - BOOST_CHECK_EQUAL (a->running (), true); - a->set_finished_ok (); - dcpomatic_sleep_seconds (2); - BOOST_CHECK_EQUAL (a->finished_ok(), true); -} diff --git a/test/wscript b/test/wscript index ff6895d9a..a5366b3ec 100644 --- a/test/wscript +++ b/test/wscript @@ -96,7 +96,7 @@ def build(bld): interrupt_encoder_test.cc isdcf_name_test.cc j2k_bandwidth_test.cc - job_test.cc + job_manager_test.cc kdm_naming_test.cc low_bitrate_test.cc markers_test.cc