Index: source/ps/Future.h =================================================================== --- source/ps/Future.h +++ source/ps/Future.h @@ -26,7 +26,6 @@ #include #include -template class PackagedTask; namespace FutureSharedStateDetail @@ -68,12 +67,29 @@ { }; +/** + * Abstract base-class of SharedState only exposes the interface for PackagedTask: The function call + * operator. + */ +class ErasedSharedState +{ +protected: + ErasedSharedState() = default; + ~ErasedSharedState() = default; + + ErasedSharedState(const ErasedSharedState&) = delete; + ErasedSharedState& operator = (const ErasedSharedState&) = delete; + +public: + virtual void operator()() = 0; +}; + /** * The shared state between futures and packaged state. * Holds all relevant data. */ template -class SharedState : public SharedStateResult +class SharedState final : public SharedStateResult, public ErasedSharedState { static constexpr bool VoidResult = std::is_same_v; public: @@ -93,6 +109,33 @@ SharedState(const SharedState&) = delete; SharedState(SharedState&&) = delete; + void operator()() final + { + Status expected = Status::PENDING; + if (!m_Status.compare_exchange_strong(expected, Status::STARTED)) + return; + + if constexpr (VoidResult) + m_Func(); + else + { + // To avoid UB, explicitly placement-new the value. + new (&SharedStateResult::m_Result.m_Result) ResultType{m_Func()}; + SharedStateResult::m_HasResult = true; + } + + // Because we might have threads waiting on us, we need to make sure that they either: + // - don't wait on our condition variable + // - receive the notification when we're done. + // This requires locking the mutex (@see Wait). + { + std::lock_guard lock(m_Mutex); + m_Status = Status::DONE; + } + + m_ConditionVariable.notify_all(); + } + bool IsDoneOrCanceled() const { return m_Status == Status::DONE || m_Status == Status::CANCELED; @@ -167,9 +210,6 @@ template class Future { - template - friend class PackagedTask; - static constexpr bool VoidResult = std::is_same_v; using Status = FutureSharedStateDetail::Status; @@ -185,7 +225,7 @@ * Make the future wait for the result of @a func. */ template - PackagedTask Wrap(T&& func); + PackagedTask Wrap(T&& func); /** * Move the result out of the future, and invalidate the future. @@ -261,66 +301,35 @@ }; /** - * Corresponds somewhat to std::packaged_task. - * Like packaged_task, this holds a function acting as a promise. - * This type is mostly just the shared state and the call operator, - * handling the promise & continuation logic. + * A lightweight handle to a task can be used for storing in a task queue. */ -template class PackagedTask { - static constexpr bool VoidResult = std::is_same_v; public: - PackagedTask() = delete; - PackagedTask(std::shared_ptr::SharedState> ss) : m_SharedState(std::move(ss)) {} + PackagedTask() = default; + explicit PackagedTask(std::shared_ptr ss) : + m_SharedState(std::move(ss)) + {} void operator()() { - typename Future::Status expected = Future::Status::PENDING; - if (!m_SharedState->m_Status.compare_exchange_strong(expected, Future::Status::STARTED)) - return; - - if constexpr (VoidResult) - m_SharedState->m_Func(); - else - { - // To avoid UB, explicitly placement-new the value. - new (&m_SharedState->m_Result) ResultType{std::move(m_SharedState->m_Func())}; - m_SharedState->m_HasResult = true; - } - - // Because we might have threads waiting on us, we need to make sure that they either: - // - don't wait on our condition variable - // - receive the notification when we're done. - // This requires locking the mutex (@see Wait). - { - std::lock_guard lock(m_SharedState->m_Mutex); - m_SharedState->m_Status = Future::Status::DONE; - } - - m_SharedState->m_ConditionVariable.notify_all(); + (*m_SharedState)(); // We no longer need the shared state, drop it immediately. m_SharedState.reset(); } - void Cancel() - { - m_SharedState->Cancel(); - m_SharedState.reset(); - } - protected: - std::shared_ptr::SharedState> m_SharedState; + std::shared_ptr m_SharedState; }; template template -PackagedTask Future::Wrap(T&& func) +PackagedTask Future::Wrap(T&& func) { static_assert(std::is_convertible_v, ResultType>, "The return type of the wrapped function cannot be converted to the type of the Future."); m_SharedState = std::make_shared(std::move(func)); - return PackagedTask(m_SharedState); + return PackagedTask{m_SharedState}; } #endif // INCLUDED_FUTURE Index: source/ps/TaskManager.h =================================================================== --- source/ps/TaskManager.h +++ source/ps/TaskManager.h @@ -74,7 +74,7 @@ private: TaskManager(size_t numberOfWorkers); - void DoPushTask(std::function&& task, TaskPriority priority); + void DoPushTask(PackagedTask&& task, TaskPriority priority); class Impl; std::unique_ptr m; Index: source/ps/TaskManager.cpp =================================================================== --- source/ps/TaskManager.cpp +++ source/ps/TaskManager.cpp @@ -61,7 +61,7 @@ class Thread; -using QueueItem = std::function; +using QueueItem = PackagedTask; /** * Light wrapper around std::thread. Ensures Join has been called. @@ -141,13 +141,13 @@ * Takes ownership of @a task. * May be called from any thread. */ - void PushTask(std::function&& task, TaskPriority priority); + void PushTask(QueueItem&& task, TaskPriority priority); protected: void ClearQueue(); template - bool PopTask(std::function& taskOut); + bool PopTask(QueueItem& taskOut); // Back reference (keep this first). TaskManager& m_TaskManager; @@ -208,12 +208,12 @@ return m->m_Workers.size(); } -void TaskManager::DoPushTask(std::function&& task, TaskPriority priority) +void TaskManager::DoPushTask(QueueItem&& task, TaskPriority priority) { m->PushTask(std::move(task), priority); } -void TaskManager::Impl::PushTask(std::function&& task, TaskPriority priority) +void TaskManager::Impl::PushTask(QueueItem&& task, TaskPriority priority) { std::mutex& mutex = priority == TaskPriority::NORMAL ? m_GlobalMutex : m_GlobalLowPriorityMutex; std::deque& queue = priority == TaskPriority::NORMAL ? m_GlobalQueue : m_GlobalLowPriorityQueue; @@ -229,7 +229,7 @@ } template -bool TaskManager::Impl::PopTask(std::function& taskOut) +bool TaskManager::Impl::PopTask(QueueItem& taskOut) { std::mutex& mutex = Priority == TaskPriority::NORMAL ? m_GlobalMutex : m_GlobalLowPriorityMutex; std::deque& queue = Priority == TaskPriority::NORMAL ? m_GlobalQueue : m_GlobalLowPriorityQueue; @@ -289,7 +289,7 @@ g_Profiler2.RegisterCurrentThread(name); - std::function task; + QueueItem task; bool hasTask = false; std::unique_lock lock(m_Mutex, std::defer_lock); while (!m_Kill) Index: source/ps/tests/test_Future.h =================================================================== --- source/ps/tests/test_Future.h +++ source/ps/tests/test_Future.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2021 Wildfire Games. +/* Copyright (C) 2022 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -30,7 +30,7 @@ int counter = 0; { Future noret; - std::function task = noret.Wrap([&counter]() mutable { counter++; }); + PackagedTask task = noret.Wrap([&counter]() mutable { counter++; }); task(); TS_ASSERT_EQUALS(counter, 1); } @@ -38,7 +38,7 @@ { Future noret; { - std::function task = noret.Wrap([&counter]() mutable { counter++; }); + PackagedTask task = noret.Wrap([&counter]() mutable { counter++; }); // Auto-cancels the task. } } @@ -49,7 +49,7 @@ { { Future future; - std::function task = future.Wrap([]() { return 1; }); + PackagedTask task = future.Wrap([]() { return 1; }); task(); TS_ASSERT_EQUALS(future.Get(), 1); } @@ -57,7 +57,7 @@ // Convertible type. { Future future; - std::function task = future.Wrap([]() -> u8 { return 1; }); + PackagedTask task = future.Wrap([]() -> u8 { return 1; }); task(); TS_ASSERT_EQUALS(future.Get(), 1); } @@ -80,14 +80,14 @@ TS_ASSERT_EQUALS(destroyed, 0); { Future future; - std::function task = future.Wrap([]() { return 1; }); + PackagedTask task = future.Wrap([]() { return 1; }); task(); TS_ASSERT_EQUALS(future.Get().value, 1); } TS_ASSERT_EQUALS(destroyed, 1); { Future future; - std::function task = future.Wrap([]() { return 1; }); + PackagedTask task = future.Wrap([]() { return 1; }); } TS_ASSERT_EQUALS(destroyed, 1); /** @@ -117,7 +117,7 @@ c = new (&functionStorage) std::function{}; *c = []() { return 7; }; - std::function task = f->Wrap(std::move(*c)); + PackagedTask task = f->Wrap(std::move(*c)); future = std::move(*f); function = std::move(*c); @@ -129,7 +129,7 @@ memset(&functionStorage, 0xFF, sizeof(decltype(functionStorage))); // Let's move the packaged task while at it. - std::function task2 = std::move(task); + PackagedTask task2 = std::move(task); task2(); TS_ASSERT_EQUALS(future.Get(), 7); }