From 4bf94458d5d7abafb154e914620dc758d26719c2 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 14 Apr 2023 10:24:10 +0100 Subject: Global worker instance --- lib/worker.cpp | 12 ++++++------ lib/worker.h | 35 ++++++++++------------------------- test/Jamfile.jam | 1 + test/test-worker.cpp | 7 +++++++ 4 files changed, 24 insertions(+), 31 deletions(-) create mode 100644 test/test-worker.cpp diff --git a/lib/worker.cpp b/lib/worker.cpp index fd255c7..cf59f56 100644 --- a/lib/worker.cpp +++ b/lib/worker.cpp @@ -1,9 +1,10 @@ #include "worker.h" -#if __cpp_lib_semaphore -# include "work.h" -# include -# include -# include +#include "work.h" +#include +#include +#include + +Worker Worker::instance; Worker::Worker() : todoLen {0} { @@ -45,4 +46,3 @@ Worker::worker() j->doWork(); } } -#endif diff --git a/lib/worker.h b/lib/worker.h index 7cd06f9..136c50e 100644 --- a/lib/worker.h +++ b/lib/worker.h @@ -1,19 +1,17 @@ #pragma once +#include +#include +#include +#include +#include +#include #include -class Work; - -#if __cpp_lib_semaphore -# include -# include -# include -# include -# include -# include -# include +#include +class Work; class Worker { -public: +private: Worker(); ~Worker(); @@ -42,19 +40,6 @@ private: ToDo todo; std::counting_semaphore<16> todoLen; std::mutex todoMutex; -}; - -#else -class Worker { -public: - template - void - addWork(Params &&... params) - requires std::is_base_of_v - { - T(std::forward(params)...).doWork(); - } + static Worker instance; }; - -#endif diff --git a/test/Jamfile.jam b/test/Jamfile.jam index 390880d..482b388 100644 --- a/test/Jamfile.jam +++ b/test/Jamfile.jam @@ -57,6 +57,7 @@ run test-glContextBhvr.cpp ; run test-assetFactory.cpp : -- : [ sequence.insertion-sort [ glob-tree $(res) : *.* ] fixtures/rgb.txt ] : test ; run perf-assetFactory.cpp : : : benchmark test test-assetFactory ; run perf-persistence.cpp : : : benchmark test test-persistence ; +run test-worker.cpp ; compile test-static-enumDetails.cpp ; compile test-static-stream_support.cpp ; explicit perf-assetFactory ; diff --git a/test/test-worker.cpp b/test/test-worker.cpp new file mode 100644 index 0000000..3c5ed7e --- /dev/null +++ b/test/test-worker.cpp @@ -0,0 +1,7 @@ +#define BOOST_TEST_MODULE test_worker + +#include "testHelpers.h" +#include +#include + +BOOST_AUTO_TEST_CASE(exists) { } -- cgit v1.2.3 From 1cbf18ff8463946aa5a09ccf1d6873b222b912b6 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 14 Apr 2023 10:26:51 +0100 Subject: Simplify worker with jthread Moves thread collection to bottom of class so threads are joined before job storage is destroyed. --- lib/work.h | 12 ------------ lib/worker.cpp | 5 +---- lib/worker.h | 4 ++-- 3 files changed, 3 insertions(+), 18 deletions(-) delete mode 100644 lib/work.h diff --git a/lib/work.h b/lib/work.h deleted file mode 100644 index c780e13..0000000 --- a/lib/work.h +++ /dev/null @@ -1,12 +0,0 @@ -#pragma once - -#include - -class Work { -public: - virtual ~Work() = default; - NO_COPY(Work); - NO_MOVE(Work); - - virtual void doWork() = 0; -}; diff --git a/lib/worker.cpp b/lib/worker.cpp index cf59f56..4f1352d 100644 --- a/lib/worker.cpp +++ b/lib/worker.cpp @@ -9,16 +9,13 @@ Worker Worker::instance; Worker::Worker() : todoLen {0} { std::generate_n(std::back_inserter(threads), std::thread::hardware_concurrency(), [this]() { - return std::thread {&Worker::worker, this}; + return std::jthread {&Worker::worker, this}; }); } Worker::~Worker() { todoLen.release(std::thread::hardware_concurrency()); - std::for_each(threads.begin(), threads.end(), [](auto & th) { - th.join(); - }); } void diff --git a/lib/worker.h b/lib/worker.h index 136c50e..96593d9 100644 --- a/lib/worker.h +++ b/lib/worker.h @@ -33,13 +33,13 @@ private: private: void worker(); - using Threads = std::vector; + using Threads = std::vector; using ToDo = std::deque; - Threads threads; ToDo todo; std::counting_semaphore<16> todoLen; std::mutex todoMutex; + Threads threads; static Worker instance; }; -- cgit v1.2.3 From aa77548bd637ffd1a7461136e6206906dc4c61c7 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 14 Apr 2023 11:54:45 +0100 Subject: New WorkItem/job/promise/future based interface --- lib/worker.cpp | 3 +- lib/worker.h | 87 ++++++++++++++++++++++++++++++++++++++++++++++------ test/test-worker.cpp | 39 ++++++++++++++++++++++- 3 files changed, 116 insertions(+), 13 deletions(-) diff --git a/lib/worker.cpp b/lib/worker.cpp index 4f1352d..7e7f296 100644 --- a/lib/worker.cpp +++ b/lib/worker.cpp @@ -1,5 +1,4 @@ #include "worker.h" -#include "work.h" #include #include #include @@ -19,7 +18,7 @@ Worker::~Worker() } void -Worker::addWork(WorkPtr j) +Worker::addWorkPtr(WorkPtr j) { std::lock_guard lck {todoMutex}; todoLen.release(); diff --git a/lib/worker.h b/lib/worker.h index 96593d9..5356606 100644 --- a/lib/worker.h +++ b/lib/worker.h @@ -1,6 +1,8 @@ #pragma once #include +#include +#include #include #include #include @@ -9,28 +11,93 @@ #include #include -class Work; class Worker { +public: + class WorkItem { + public: + WorkItem() = default; + virtual ~WorkItem() = default; + NO_MOVE(WorkItem); + NO_COPY(WorkItem); + + virtual void doWork() = 0; + }; + + template class WorkItemT : public WorkItem { + public: + T + get() + { + return future.get(); + } + + protected: + std::promise promise; + std::future future {promise.get_future()}; + friend Worker; + }; + + template + static auto + addWork(Params &&... params) + { + return instance.addWorkImpl(std::forward(params)...); + } + template using WorkPtrT = std::shared_ptr>; + private: + template class WorkItemTImpl : public WorkItemT { + public: + WorkItemTImpl(Params &&... params) : params {std::forward(params)...} { } + + private: + void + doWork() override + { + try { + if constexpr (std::is_void_v) { + std::apply( + [](auto &&... p) { + return std::invoke(p...); + }, + params); + WorkItemT::promise.set_value(); + } + else { + WorkItemT::promise.set_value(std::apply( + [](auto &&... p) { + return std::invoke(p...); + }, + params)); + } + } + catch (...) { + WorkItemT::promise.set_exception(std::current_exception()); + } + } + + std::tuple params; + }; + Worker(); ~Worker(); NO_COPY(Worker); NO_MOVE(Worker); - using WorkPtr = std::unique_ptr; + using WorkPtr = std::shared_ptr; - template - void - addWork(Params &&... params) - requires std::is_base_of_v + template + auto + addWorkImpl(Params &&... params) { - addWork(std::make_unique(std::forward(params)...)); + using T = decltype(std::invoke(std::forward(params)...)); + auto work = std::make_shared>(std::forward(params)...); + addWorkPtr(work); + return work; } - void addWork(WorkPtr w); - -private: + void addWorkPtr(WorkPtr w); void worker(); using Threads = std::vector; diff --git a/test/test-worker.cpp b/test/test-worker.cpp index 3c5ed7e..c542020 100644 --- a/test/test-worker.cpp +++ b/test/test-worker.cpp @@ -2,6 +2,43 @@ #include "testHelpers.h" #include +#include #include +#include -BOOST_AUTO_TEST_CASE(exists) { } +uint32_t +workCounter() +{ + static std::atomic_uint32_t n; + usleep(1000); + return n++; +} + +BOOST_AUTO_TEST_CASE(basic_slow_counter) +{ + std::vector> ps; + for (int i {}; i < 30; ++i) { + ps.push_back(Worker::addWork(workCounter)); + } + std::set out; + std::transform(ps.begin(), ps.end(), std::inserter(out, out.end()), [](auto && p) { + return p->get(); + }); + BOOST_REQUIRE_EQUAL(out.size(), ps.size()); + BOOST_CHECK_EQUAL(*out.begin(), 0); + BOOST_CHECK_EQUAL(*out.rbegin(), ps.size() - 1); +} + +BOOST_AUTO_TEST_CASE(basic_error_handler) +{ + auto workitem = Worker::addWork([]() { + throw std::runtime_error {"test"}; + }); + BOOST_CHECK_THROW(workitem->get(), std::runtime_error); +} + +BOOST_AUTO_TEST_CASE(basic_void_work) +{ + auto workitem = Worker::addWork([]() {}); + BOOST_CHECK_NO_THROW(workitem->get()); +} -- cgit v1.2.3 From e671adba5a57e1d4e848eb4d6de0f480e7b3709a Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 14 Apr 2023 12:29:24 +0100 Subject: Simplify doWork, add tests for various interface uses --- lib/worker.h | 12 ++---------- test/test-worker.cpp | 47 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/lib/worker.h b/lib/worker.h index 5356606..1bc7c14 100644 --- a/lib/worker.h +++ b/lib/worker.h @@ -56,19 +56,11 @@ private: { try { if constexpr (std::is_void_v) { - std::apply( - [](auto &&... p) { - return std::invoke(p...); - }, - params); + std::apply(std::invoke, params); WorkItemT::promise.set_value(); } else { - WorkItemT::promise.set_value(std::apply( - [](auto &&... p) { - return std::invoke(p...); - }, - params)); + WorkItemT::promise.set_value(std::apply(std::invoke, params)); } } catch (...) { diff --git a/test/test-worker.cpp b/test/test-worker.cpp index c542020..319261b 100644 --- a/test/test-worker.cpp +++ b/test/test-worker.cpp @@ -14,6 +14,19 @@ workCounter() return n++; } +void +workVoid() +{ + usleep(1000); +} + +void +workFail() +{ + usleep(1000); + throw std::runtime_error {"test"}; +} + BOOST_AUTO_TEST_CASE(basic_slow_counter) { std::vector> ps; @@ -31,14 +44,40 @@ BOOST_AUTO_TEST_CASE(basic_slow_counter) BOOST_AUTO_TEST_CASE(basic_error_handler) { - auto workitem = Worker::addWork([]() { - throw std::runtime_error {"test"}; - }); + auto workitem = Worker::addWork(workFail); BOOST_CHECK_THROW(workitem->get(), std::runtime_error); } BOOST_AUTO_TEST_CASE(basic_void_work) { - auto workitem = Worker::addWork([]() {}); + auto workitem = Worker::addWork(workVoid); BOOST_CHECK_NO_THROW(workitem->get()); } + +BOOST_AUTO_TEST_CASE(lambda_void) +{ + BOOST_CHECK_NO_THROW(Worker::addWork([]() {})->get()); + BOOST_CHECK_NO_THROW(Worker::addWork([](int) {}, 0)->get()); + BOOST_CHECK_NO_THROW(Worker::addWork([](int, int) {}, 0, 0)->get()); +} + +BOOST_AUTO_TEST_CASE(lambda_value) +{ + BOOST_CHECK_EQUAL(1, Worker::addWork([]() { + return 1; + })->get()); + BOOST_CHECK_EQUAL(2, + Worker::addWork( + [](int i) { + return i; + }, + 2) + ->get()); + BOOST_CHECK_EQUAL(3, + Worker::addWork( + [](int i, int j) { + return i + j; + }, + 1, 2) + ->get()); +} -- cgit v1.2.3 From f8ee56f6dc8cf7e92c4bfc5930d32b14f634141c Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 14 Apr 2023 13:13:47 +0100 Subject: Current thread partakes in work effort while waiting This will prevent deadlock if the work pool is otherwise busy by ensuring work is always being done --- lib/worker.cpp | 21 +++++++++++++++++++++ lib/worker.h | 26 ++++++++++++++++++++++---- test/test-worker.cpp | 21 +++++++++++++++++++++ 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/lib/worker.cpp b/lib/worker.cpp index 7e7f296..45fb6df 100644 --- a/lib/worker.cpp +++ b/lib/worker.cpp @@ -42,3 +42,24 @@ Worker::worker() j->doWork(); } } + +void +Worker::assist() +{ + auto job = [this]() { + using namespace std::chrono_literals; + if (todoLen.try_acquire_for(100us)) { + if (std::lock_guard lck {todoMutex}; todo.size()) { + WorkPtr x = std::move(todo.front()); + if (x) { + todo.pop_front(); + } + return x; + } + } + return WorkPtr {}; + }; + if (auto j = job()) { + j->doWork(); + } +} diff --git a/lib/worker.h b/lib/worker.h index 1bc7c14..d9a5a6f 100644 --- a/lib/worker.h +++ b/lib/worker.h @@ -14,12 +14,20 @@ class Worker { public: class WorkItem { - public: - WorkItem() = default; + protected: + WorkItem(Worker * worker) : worker {worker} { } virtual ~WorkItem() = default; NO_MOVE(WorkItem); NO_COPY(WorkItem); + void + assist() const + { + worker->assist(); + } + Worker * worker; + + public: virtual void doWork() = 0; }; @@ -28,10 +36,16 @@ public: T get() { + using namespace std::chrono_literals; + while (future.wait_for(0s) == std::future_status::timeout) { + assist(); + } return future.get(); } protected: + WorkItemT(Worker * worker) : WorkItem {worker} { } + std::promise promise; std::future future {promise.get_future()}; friend Worker; @@ -48,7 +62,10 @@ public: private: template class WorkItemTImpl : public WorkItemT { public: - WorkItemTImpl(Params &&... params) : params {std::forward(params)...} { } + WorkItemTImpl(Worker * worker, Params &&... params) : + WorkItemT {worker}, params {std::forward(params)...} + { + } private: void @@ -84,13 +101,14 @@ private: addWorkImpl(Params &&... params) { using T = decltype(std::invoke(std::forward(params)...)); - auto work = std::make_shared>(std::forward(params)...); + auto work = std::make_shared>(this, std::forward(params)...); addWorkPtr(work); return work; } void addWorkPtr(WorkPtr w); void worker(); + void assist(); using Threads = std::vector; using ToDo = std::deque; diff --git a/test/test-worker.cpp b/test/test-worker.cpp index 319261b..76b7138 100644 --- a/test/test-worker.cpp +++ b/test/test-worker.cpp @@ -81,3 +81,24 @@ BOOST_AUTO_TEST_CASE(lambda_value) 1, 2) ->get()); } + +BOOST_AUTO_TEST_CASE(recursive, *boost::unit_test::timeout(5)) +{ + auto recurse = []() { + std::vector> ps; + for (int i {}; i < 30; ++i) { + ps.push_back(Worker::addWork(workCounter)); + } + return std::accumulate(ps.begin(), ps.end(), 0U, [](auto && out, auto && p) { + return out += p->get(); + }); + }; + std::vector> ps; + for (int i {}; i < 30; ++i) { + ps.push_back(Worker::addWork(recurse)); + } + std::set out; + std::transform(ps.begin(), ps.end(), std::inserter(out, out.end()), [](auto && p) { + return p->get(); + }); +} -- cgit v1.2.3 From 1cdd7753d37bcf8f626298c3df97a02cc73f266c Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 14 Apr 2023 14:48:59 +0100 Subject: Load texture images in Worker --- assetFactory/assetFactory.cpp | 18 +++++++++++------- assetFactory/assimp.cpp | 20 ++++++++------------ assetFactory/textureFragment.cpp | 4 +++- assetFactory/textureFragment.h | 3 ++- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/assetFactory/assetFactory.cpp b/assetFactory/assetFactory.cpp index 46b4642..e27e575 100644 --- a/assetFactory/assetFactory.cpp +++ b/assetFactory/assetFactory.cpp @@ -99,21 +99,25 @@ AssetFactory::createTexutre() const { if (!textureFragments.empty() && !texture) { // * layout images - std::vector imageSizes; + std::map> images; std::transform( - textureFragments.begin(), textureFragments.end(), std::back_inserter(imageSizes), [](const auto & tf) { - return TexturePacker::Image {tf.second->image->width, tf.second->image->height}; + textureFragments.begin(), textureFragments.end(), std::inserter(images, images.end()), [](auto && tf) { + return decltype(images)::value_type {tf.first, tf.second->image->get()}; }); + std::vector imageSizes; + std::transform(images.begin(), images.end(), std::back_inserter(imageSizes), [](const auto & i) { + return TexturePacker::Image {i.second->width, i.second->height}; + }); const auto [layout, outSize] = TexturePacker {imageSizes}.pack(); // * create texture texture = std::make_shared(outSize.x, outSize.y, layout.size()); - std::transform(textureFragments.begin(), textureFragments.end(), + std::transform(images.begin(), images.end(), std::inserter(textureFragmentPositions, textureFragmentPositions.end()), - [position = layout.begin(), size = imageSizes.begin(), this](const auto & tf) mutable { - const auto m = texture->add(*position, *size, tf.second->image->data.data()); + [position = layout.begin(), size = imageSizes.begin(), this](const auto & i) mutable { + const auto m = texture->add(*position, *size, i.second->data.data()); position++; size++; - return decltype(textureFragmentPositions)::value_type {tf.first, m}; + return decltype(textureFragmentPositions)::value_type {i.first, m}; }); } } diff --git a/assetFactory/assimp.cpp b/assetFactory/assimp.cpp index 878e7e7..bab052e 100644 --- a/assetFactory/assimp.cpp +++ b/assetFactory/assimp.cpp @@ -102,18 +102,14 @@ AssImp::postLoad() return AssetFactory::Shapes::value_type {m->mName.C_Str(), std::make_shared(scene, m)}; }); const auto textures = AIRANGE(scene, Textures); - auto textureFutures = textures * [](const aiTexture * t) { - return std::async(std::launch::async, [t]() { - auto texture = std::make_shared(); - texture->id = texture->path = t->mFilename.C_Str(); - texture->image = std::make_unique( - std::span {reinterpret_cast(t->pcData), t->mWidth}, STBI_rgb_alpha); - return texture; - }); - }; - std::transform(textureFutures.begin(), textureFutures.end(), - std::inserter(mf->textureFragments, mf->textureFragments.end()), [](auto && textureFuture) { - auto texture = textureFuture.get(); + std::transform(textures.begin(), textures.end(), + std::inserter(mf->textureFragments, mf->textureFragments.end()), [](auto && t) { + auto texture = std::make_shared(); + texture->id = texture->path = t->mFilename.C_Str(); + texture->image = Worker::addWork([t]() { + return std::make_unique( + std::span {reinterpret_cast(t->pcData), t->mWidth}, STBI_rgb_alpha); + }); return AssetFactory::TextureFragments::value_type {texture->id, texture}; }); } diff --git a/assetFactory/textureFragment.cpp b/assetFactory/textureFragment.cpp index d153688..0a4ec1d 100644 --- a/assetFactory/textureFragment.cpp +++ b/assetFactory/textureFragment.cpp @@ -11,5 +11,7 @@ TextureFragment::persist(Persistence::PersistenceStore & store) void TextureFragment::postLoad() { - image = std::make_unique(Resource::mapPath(path), STBI_rgb_alpha); + image = Worker::addWork([this]() { + return std::make_unique(Resource::mapPath(path), STBI_rgb_alpha); + }); } diff --git a/assetFactory/textureFragment.h b/assetFactory/textureFragment.h index d03affd..75fe96e 100644 --- a/assetFactory/textureFragment.h +++ b/assetFactory/textureFragment.h @@ -3,12 +3,13 @@ #include "gfx/image.h" #include "persistence.h" #include "stdTypeDefs.hpp" +#include "worker.h" class TextureFragment : public Persistence::Persistable, public StdTypeDefs { public: std::string id; std::string path; - std::unique_ptr image; + Worker::WorkPtrT> image; private: friend Persistence::SelectionPtrBase; -- cgit v1.2.3