From 8272db823371b6f8f5933c630852468397470d88 Mon Sep 17 00:00:00 2001 From: isanae <14251494+isanae@users.noreply.github.com> Date: Mon, 30 Nov 2020 10:50:20 -0500 Subject: [PATCH] made threaded_run() private, everything should use parallel() comments --- src/cmd/list.cpp | 4 +- src/tasks/modorganizer.cpp | 18 +-- src/tasks/task.cpp | 180 ++++++++++++------------- src/tasks/task.h | 268 +++++++++++++++++++++++++++++-------- src/tasks/task_manager.cpp | 1 + src/tasks/tasks.h | 1 + src/tasks/translations.cpp | 19 ++- src/utility/threading.cpp | 13 +- src/utility/threading.h | 5 +- 9 files changed, 332 insertions(+), 177 deletions(-) diff --git a/src/cmd/list.cpp b/src/cmd/list.cpp index 2bd8e8d..f98964d 100644 --- a/src/cmd/list.cpp +++ b/src/cmd/list.cpp @@ -82,8 +82,8 @@ void list_command::dump(const std::vector& v, std::size_t indent) const << " - " << join(t->names(), ",") << "\n"; - if (auto* ct=dynamic_cast(t)) - dump(ct->children(), indent + 1); + if (auto* pt=dynamic_cast(t)) + dump(pt->children(), indent + 1); } } diff --git a/src/tasks/modorganizer.cpp b/src/tasks/modorganizer.cpp index 561995f..a36f390 100644 --- a/src/tasks/modorganizer.cpp +++ b/src/tasks/modorganizer.cpp @@ -7,13 +7,17 @@ namespace mob::tasks static std::mutex g_super_mutex; static std::atomic g_super_initialized = false; -std::string make_short_name(const std::string& name) +std::vector make_names(std::vector names) { - const auto dash = name.find("-"); - if (dash == std::string::npos) - return name; + auto short_name = names[0]; - return name.substr(dash + 1); + const auto dash = short_name.find("-"); + if (dash != std::string::npos) + short_name = short_name.substr(dash + 1); + + names.insert(names.begin(), short_name); + + return names; } @@ -28,10 +32,8 @@ modorganizer::modorganizer(std::vector names, flags f) } modorganizer::modorganizer(std::vector names, flags f) - : basic_task(make_short_name(names[0])), repo_(names[0]), flags_(f) + : basic_task(make_names(names)), repo_(names[0]), flags_(f) { - for (auto&& n : names) - add_name(std::move(n)); } std::string modorganizer::version() diff --git a/src/tasks/task.cpp b/src/tasks/task.cpp index 683c337..05bd700 100644 --- a/src/tasks/task.cpp +++ b/src/tasks/task.cpp @@ -19,7 +19,6 @@ std::string to_string(task::clean c) case task::clean::reextract: break; case task::clean::reconfigure: break; case task::clean::rebuild: break; - case task::clean::everything: break; } std::vector v; @@ -39,6 +38,26 @@ std::string to_string(task::clean c) return join(v, "|"); } +task::clean make_clean_flags() +{ + task::clean c = task::clean::nothing; + const auto g = conf().global(); + + if (g.redownload()) + c |= task::clean::redownload; + + if (g.reextract()) + c |= task::clean::reextract; + + if (g.reconfigure()) + c |= task::clean::reconfigure; + + if (g.rebuild()) + c |= task::clean::rebuild; + + return c; +} + struct task::thread_context { @@ -62,23 +81,29 @@ task::task(std::vector names) task_manager::instance().register_task(this); } -task::~task() -{ - try - { - join(); - } - catch(bailed&) - { - // ignore - } -} +// anchor +task::~task() = default; bool task::enabled() const { return conf().task(names()).get("enabled"); } +void task::do_clean(clean) +{ + // no-op +} + +void task::do_fetch() +{ + // no-op +} + +void task::do_build_and_install() +{ + // no-op +} + const context& task::cx() const { static const context bad("?"); @@ -96,15 +121,6 @@ const context& task::cx() const return bad; } -void task::add_name(std::string s) -{ - auto itor = std::find(names_.begin(), names_.end(), s); - if (itor != names_.end()) - return; - - names_.push_back(s); -} - const std::string& task::name() const { return names_[0]; @@ -226,22 +242,19 @@ void task::threaded_run(std::string thread_name, std::function f) } } -void task::parallel(std::vector>> v) +void task::parallel(parallel_functions v, std::optional threads) { - std::vector ts; + thread_pool tp(threads); for (auto&& [name, f] : v) { cx().trace(context::generic, "running in parallel: {}", name); - ts.push_back(start_thread([this, name, f] + tp.add([this, name, f] { threaded_run(name, f); - })); + }); } - - for (auto&& t : ts) - t.join(); } conf_task task::task_conf() const @@ -249,10 +262,9 @@ conf_task task::task_conf() const return conf().task(names()); } -git task::make_git(git::ops o) const +git task::make_git() const { - if (o == git::clone_or_pull && task_conf().no_pull()) - o = git::clone; + const auto o = task_conf().no_pull() ? git::clone : git::clone_or_pull; git g(o); @@ -279,6 +291,16 @@ std::string task::make_git_url( return task_conf().git_url_prefix() + org + "/" + repo + ".git"; } +fs::path task::get_source_path() const +{ + return {}; +} + +bool task::get_prebuilt() const +{ + return false; +} + void task::run() { threaded_run(name(), [&] @@ -292,13 +314,9 @@ void task::run() cx().info(context::generic, "running task"); fetch(); - join(); - check_interrupted(); build_and_install(); - join(); - check_interrupted(); }); } @@ -313,12 +331,6 @@ void task::interrupt() t->interrupt(); } -void task::join() -{ - if (thread_.joinable()) - thread_.join(); -} - void task::clean_task() { if (!conf().global().clean()) @@ -348,33 +360,27 @@ void task::fetch() return; } - thread_ = start_thread([&] + clean_task(); + check_interrupted(); + + if (conf().global().fetch()) { - threaded_run(name(), [&] + cx().info(context::generic, "fetching"); + do_fetch(); + + check_interrupted(); + + if (!get_source_path().empty()) { - clean_task(); - check_interrupted(); + cx().debug(context::generic, "patching"); - if (conf().global().fetch()) - { - cx().info(context::generic, "fetching"); - do_fetch(); + run_tool(patcher() + .task(name(), get_prebuilt()) + .root(get_source_path())); + } - check_interrupted(); - - if (!get_source_path().empty()) - { - cx().debug(context::generic, "patching"); - - run_tool(patcher() - .task(name(), get_prebuilt()) - .root(get_source_path())); - } - - check_interrupted(); - } - }); - }); + check_interrupted(); + } } void task::build_and_install() @@ -389,38 +395,12 @@ void task::build_and_install() return; } - thread_ = start_thread([&] - { - threaded_run(name(), [&] - { - check_interrupted(); + check_interrupted(); - cx().info(context::generic, "build and install"); - do_build_and_install(); + cx().info(context::generic, "build and install"); + do_build_and_install(); - check_interrupted(); - }); - }); -} - -task::clean task::make_clean_flags() const -{ - clean c = clean::nothing; - const auto g = conf().global(); - - if (g.redownload()) - c |= clean::redownload; - - if (g.reextract()) - c |= clean::reextract; - - if (g.reconfigure()) - c |= clean::reconfigure; - - if (g.rebuild()) - c |= clean::rebuild; - - return c; + check_interrupted(); } void task::check_interrupted() @@ -461,10 +441,20 @@ void task::run_tool_impl(tool* t) parallel_tasks::parallel_tasks() - : container_task("parallel") + : task("parallel") { } +parallel_tasks::~parallel_tasks() +{ + join(); +} + +bool parallel_tasks::enabled() const +{ + return true; +} + void parallel_tasks::add_task(std::unique_ptr t) { children_.push_back(std::move(t)); diff --git a/src/tasks/task.h b/src/tasks/task.h index f3d7a24..67c8ec8 100644 --- a/src/tasks/task.h +++ b/src/tasks/task.h @@ -1,35 +1,71 @@ #pragma once #include "../utility.h" -#include "../core/context.h" -#include "../tools/tools.h" namespace mob { -class task; class tool; +class conf_task; +class git; + +// base class for all tasks, although tasks will actually inherit from +// basic_task<> below +// class task { public: + // passed to do_clean(), can be any combination depending on the + // configuration (redownload, reextract, etc. in the ini) + // enum class clean { + // do_clean() never gets that nothing = 0x00, + + // whatever downloaded file that was cached must be deleted so it can + // be downloaded again; cached downloads normally inhibit downloads redownload = 0x01, + + // extracting or cloning must be fresh, so basically delete the whole + // source directory; both enums have the same value, but `reclone` makes + // more sense for tools that don't actually extract archives reextract = 0x02, reclone = reextract, + + // building the task must run whatever configuration tool from scratch, + // such as deleting the vsbuild directory for cmake tasks reconfigure = 0x04, - rebuild = 0x08, - everything = redownload+reextract+reconfigure+rebuild + + // the task must be rebuilt from scratch, but without reconfiguration, + // such as `msbuild` with the `Clean` target + // + // some tasks don't have an equivalent for this and might just delete + // the whole source directory, such as openssl + rebuild = 0x08 }; + task(const task&) = delete; task& operator=(const task&) = delete; + + // anchor + // virtual ~task(); + + // whether this task is enabled, just checks conf().task(), but + // parallel_tasks overrides this below to always be true + // virtual bool enabled() const; + + // main task name + // const std::string& name() const; + + // all names for this task + // const std::vector& names() const; // case insensitive, underscores and dashes are equivalent; gets converted @@ -37,15 +73,34 @@ public: // bool name_matches(std::string_view pattern) const; - virtual fs::path get_source_path() const = 0; - virtual std::string get_version() const = 0; - virtual const bool get_prebuilt() const = 0; + // path to the source directory, something like prefix/build/7zip-xx or + // or prefix/build/modorganizer_super/uibase + // + // used for auto patching in fetch(), returns an empty path here + // + virtual fs::path get_source_path() const; + + // whether this task should use the prebuilt version + // + // used for auto patching in fetch(), returns false here + // + virtual bool get_prebuilt() const; + + + // if the task is enabled, calls fetch() and build_and_install() + // virtual void run(); + + // sets the interrupt flag on this task so it's picked up in run() and + // calls interrupt() on all tools currently running + // virtual void interrupt(); - virtual void join(); protected: + using parallel_functions = + std::vector>>; + template task(std::string name, Names&&... names) : task(std::vector{name, std::forward(names)...}) @@ -54,15 +109,40 @@ protected: task(std::vector names); - const context& cx() const; - void add_name(std::string s); + // implemented by derived classes to clean the task depending on the given + // clean flags; no-op in this class + // + virtual void do_clean(clean); + + // implemented by derived classes to fetch (download, clone, etc.) the + // required files to build and/or install the task; no-op in this class + // + virtual void do_fetch(); + + // implemented by derived classes to build and install the task in one step; + // no-op in this class + // + virtual void do_build_and_install(); + + + // returns the task's context + // + // since a task may be running several threads, a list of per-thread context + // objects is kept in contexts_ and the correct one is returned + // + const context& cx() const; + + // throws if the interrupted flag is set + // void check_interrupted(); - virtual void do_clean(clean) {}; - virtual void do_fetch() {} - virtual void do_build_and_install() {} - + // adds the tool to the internal list of active tools so they can be + // interrupted properly and calls run() on it with the task's log context + // + // once run() returns, removes the tool from the list and returns whatever + // result() returns, which may be void + // template auto run_tool(Tool&& t) { @@ -70,45 +150,114 @@ protected: return t.result(); } - void threaded_run(std::string name, std::function f); - void parallel(std::vector>> v); + // runs the given functions in a thread_pool with `threads` as the maximum + // number of threads + // + // calls threaded_run() for every function, which creates a new log context + // for the thread in case multiple tools are run simultaneously + // + // this is the preferred way for tasks to run tools in parallel, such as + // in the translations or gtest tasks + // + void parallel(parallel_functions v, std::optional threads={}); + // returns the conf_task for this task, short for conf().task(names()) + // conf_task task_conf() const; - git make_git(git::ops o=git::clone_or_pull) const; + // returns a git tool suitable for this task, with all the relevant task + // settings set, such as ignore_ts_on_clone, shallow, etc. + // + // if `o` is clone_or_pull and the configuration has no_pull for this task, + // it is changed to clone only + // + git make_git() const; + // returns a git url for the given org and repo, using the git_url_prefix + // for this task + // std::string make_git_url( const std::string& org, const std::string& repo) const; private: + // a struct with the thread id and a context object, defined in task.cpp + // to avoid pulling to many includes + // struct thread_context; - std::vector names_; - std::thread thread_; + // names for this task + const std::vector names_; + + // set when interrupt() is called, checked by check_interrupted(), which + // throws an `interrupted` exception + // std::atomic interrupted_; + // holds a context per thread, added/removed in threaded_run() std::vector> contexts_; mutable std::mutex contexts_mutex_; + // list of active tools, added/removed in run_tool() std::vector tools_; mutable std::mutex tools_mutex_; - clean make_clean_flags() const; + + // called by run_tool, does the actual work + // void run_tool_impl(tool* t); + // called by name_matches() when the pattern is a glob + // bool name_matches_glob(std::string_view pattern) const; + + // called by name_matches() when the pattern is not a glob + // bool name_matches_string(std::string_view pattern) const; + + // called by name_matches_string(), compares the two strings to get the + // same result as name_matches_glob() (case insensitive, dashes/underscores + // are the same, etc.) but without requiring a regex because it's slow as + // frick + // bool strings_match(std::string_view a, std::string_view b) const; - void clean_task(); + // called by run() and parallel(), adds a new context for the current thread + // and calls f() + // + void threaded_run(std::string name, std::function f); + + + // calls clean_task(), then do_fetch() if needed (see --no-fetch-task); + // no-op if the task is disabled + // void fetch(); + + // calls do_build_and_install() if building is enabled + // (see --no-build-task); no-op if the task is disabled + // void build_and_install(); + + // calls do_clean() if needed with the appropriate flags (see + // --no-clean-task); no-op if the task is disabled + // + void clean_task(); + }; MOB_ENUM_OPERATORS(task::clean); +// all tasks except for modorganizer have static functions source_path() and +// prebuilt(), which are used in a variety of places, but `task` also needs to +// know about them +// +// so get_source_path() and get_prebuilt() just forward to the static version +// and basic_task uses CRTP so tasks don't have to implement both +// +// since the modorganizer task is reused for all super projects, it doesn't +// have the static member functions and implement these two functions itself +// template class basic_task : public task { @@ -120,37 +269,28 @@ public: return Task::source_path(); } - std::string get_version() const override - { - return Task::version(); - } - - const bool get_prebuilt() const override + bool get_prebuilt() const override { return Task::prebuilt(); } }; -class container_task : public task -{ -public: - using task::task; - - virtual std::vector children() const = 0; -}; - - -class parallel_tasks : public container_task +// a task that overrides run() to start as many threads as it has children +// and calls run() on all of them +// +class parallel_tasks : public task { public: parallel_tasks(); - bool enabled() const override - { - return true; - } + // joins + // + ~parallel_tasks(); + + // creates a task `Task`, forwards args to constructor and adds it + // template parallel_tasks& add_task(Args&&... args) { @@ -158,6 +298,12 @@ public: return *this; } + // creates a task `Task`, forwards args to constructor and adds it + // + // this overload is convenient for modorganizer tasks to pass the task names + // as an initializer list, which can't be done with the version above + // because `Args` can't be deduced + // template parallel_tasks& add_task(std::initializer_list il, Args&&... args) { @@ -167,32 +313,38 @@ public: return *this; } + // called by the above, adds the task + // void add_task(std::unique_ptr t); - fs::path get_source_path() const override - { - return {}; - } - std::string get_version() const override - { - return {}; - } - - const bool get_prebuilt() const override - { - return false; - } + // returns true, parallel tasks cannot be disabled, but their children can + // + bool enabled() const override; + // starts a thread for every child task and calls run() on it + // void run() override; - void interrupt() override; - void join() override; - std::vector children() const override; + // calls interrupt() on all children tasks + // + void interrupt() override; + + // returns children tasks + // + std::vector children() const; private: + // tasks std::vector> children_; + + // one thread per task std::vector threads_; + + + // joins all threads + // + void join(); }; } // namespace diff --git a/src/tasks/task_manager.cpp b/src/tasks/task_manager.cpp index a7772b5..4ce4b38 100644 --- a/src/tasks/task_manager.cpp +++ b/src/tasks/task_manager.cpp @@ -1,6 +1,7 @@ #include "pch.h" #include "task_manager.h" #include "task.h" +#include "../core/context.h" namespace mob { diff --git a/src/tasks/tasks.h b/src/tasks/tasks.h index 81817c3..0784767 100644 --- a/src/tasks/tasks.h +++ b/src/tasks/tasks.h @@ -1,6 +1,7 @@ #pragma once #include "task.h" +#include "../tools/tools.h" #include "../net.h" #include "../utility.h" #include "../core/conf.h" diff --git a/src/tasks/translations.cpp b/src/tasks/translations.cpp index 664acf3..a4c5d45 100644 --- a/src/tasks/translations.cpp +++ b/src/tasks/translations.cpp @@ -236,24 +236,23 @@ void translations::do_build_and_install() for (auto&& w : ps.warnings()) cx().warning(context::generic, "{}", w); - thread_pool tp; + parallel_functions v; for (auto& p : ps.get()) { for (auto& lg : p.langs) { - tp.add([&] + v.push_back({lg.name + "." + p.name, [&] { - threaded_run(lg.name + "." + p.name, [&] - { - run_tool(lrelease() - .project(p.name) - .sources(lg.ts_files) - .out(dest)); - }); - }); + run_tool(lrelease() + .project(p.name) + .sources(lg.ts_files) + .out(dest)); + }}); } } + + parallel(v); } } // namespace diff --git a/src/utility/threading.cpp b/src/utility/threading.cpp index dd1d15e..61dd029 100644 --- a/src/utility/threading.cpp +++ b/src/utility/threading.cpp @@ -179,8 +179,15 @@ void set_thread_exception_handlers() } -thread_pool::thread_pool(std::size_t count) - : count_(std::max(1, count)) +std::size_t make_thread_count(std::optional count) +{ + static const auto def = std::thread::hardware_concurrency(); + return std::max(1, count.value_or(def)); +} + + +thread_pool::thread_pool(std::optional count) + : count_(make_thread_count(count)) { for (std::size_t i=0; i()); @@ -226,7 +233,7 @@ bool thread_pool::try_add(fun thread_fun) t->running = true; t->thread_fun = thread_fun; - t->thread = std::thread([&] + t->thread = start_thread([&] { t->thread_fun(); t->running = false; diff --git a/src/utility/threading.h b/src/utility/threading.h index ec19cf7..0a58ebe 100644 --- a/src/utility/threading.h +++ b/src/utility/threading.h @@ -28,7 +28,10 @@ class thread_pool public: typedef std::function fun; - thread_pool(std::size_t count=std::thread::hardware_concurrency()); + thread_pool(std::optional count={}); + + // joins + // ~thread_pool(); // non-copyable