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 d8c0b02f2f210a58c863fe81d072dfecdd883468..608df7ef0af9063729824f7c3967d6d76b7a02fe 100644 (file)
--- 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> 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> 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 ff5800aa84a6d2da70eed283eead48c9de6e2609..71db33fd6bf9d2b14fe8037ec1c56a5d7de1de67 100644 (file)
--- 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 (file)
index 0000000..1baa576
--- /dev/null
+++ b/test/job_manager_test.cc
@@ -0,0 +1,130 @@
+/*
+    Copyright (C) 2012-2021 Carl Hetherington <cth@carlh.net>
+
+    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 <http://www.gnu.org/licenses/>.
+
+*/
+
+
+/** @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 <boost/test/unit_test.hpp>
+
+
+using std::make_shared;
+using std::shared_ptr;
+using std::string;
+using std::vector;
+
+
+class TestJob : public Job
+{
+public:
+       explicit TestJob (shared_ptr<Film> 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> film;
+
+       /* Single job */
+       auto a = make_shared<TestJob>(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> film;
+
+       vector<shared_ptr<TestJob>> jobs;
+       for (int i = 0; i < 16; ++i) {
+               auto job = make_shared<TestJob>(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 (file)
index 7fb2408..0000000
--- a/test/job_test.cc
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
-    Copyright (C) 2012-2021 Carl Hetherington <cth@carlh.net>
-
-    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 <http://www.gnu.org/licenses/>.
-
-*/
-
-
-/** @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 <boost/test/unit_test.hpp>
-
-
-using std::make_shared;
-using std::shared_ptr;
-using std::string;
-
-
-class TestJob : public Job
-{
-public:
-       explicit TestJob (shared_ptr<Film> 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> film;
-
-       /* Single job */
-       auto a = make_shared<TestJob>(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 ff6895d9a935036ccd7ce8d6ddbbfdbc5b9a26e5..a5366b3ecb8fe87ad495b80e626ab00a6edb6d09 100644 (file)
--- 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