From 5af5345048ecf67be8832d8342de0a9f65bf6d29 Mon Sep 17 00:00:00 2001 From: Dynamitos Date: Mon, 28 Mar 2022 21:18:57 +0200 Subject: [PATCH] Still memory leaks in threadpool all --- external/boost | 2 +- external/ktx | 2 +- external/slang | 2 +- src/Engine/ThreadPool.cpp | 51 +++++++++++------ src/Engine/ThreadPool.h | 97 ++++++++++++++++++++------------- src/Engine/Window/SceneView.cpp | 24 ++++---- src/Engine/main.cpp | 2 +- test/CMakeLists.txt | 4 +- test/Engine/EngineTest.h | 6 +- 9 files changed, 113 insertions(+), 77 deletions(-) diff --git a/external/boost b/external/boost index 4d8de5a..238fe94 160000 --- a/external/boost +++ b/external/boost @@ -1 +1 @@ -Subproject commit 4d8de5a695430fec0856bf667b6c73855428d83b +Subproject commit 238fe946c1d304f6e065a313610c2ef999239637 diff --git a/external/ktx b/external/ktx index b8d462b..a2ccc90 160000 --- a/external/ktx +++ b/external/ktx @@ -1 +1 @@ -Subproject commit b8d462b01cf3d4fdaf6e08ce4042575e25b04260 +Subproject commit a2ccc90effc8a5831a271ea033a9e38e26e63a59 diff --git a/external/slang b/external/slang index 2e1a84a..858c7c5 160000 --- a/external/slang +++ b/external/slang @@ -1 +1 @@ -Subproject commit 2e1a84add57efd9f8a50a88d0569a48ae4b6d834 +Subproject commit 858c7c57b125afed9b5b2329d6b02477284e4803 diff --git a/src/Engine/ThreadPool.cpp b/src/Engine/ThreadPool.cpp index 21d212b..29e5262 100644 --- a/src/Engine/ThreadPool.cpp +++ b/src/Engine/ThreadPool.cpp @@ -11,30 +11,46 @@ List Seele::promises; //} Event::Event(const std::string &name, const std::source_location &location) - : name(name) - , location(location) + : state(std::make_shared()) { + state->name = name; + state->location = location; } Event::Event(const std::source_location &location) - : name(location.function_name()) - , location(location) + : state(std::make_shared()) { + state->name = location.function_name(); + state->location = location; +} + +Event::Event(const Event& other) +{ + std::scoped_lock lock(other.eventLock); + state = other.state; } Event::Event(Event&& other) - : name(std::move(other.name)) - , location(std::move(other.location)) { + std::scoped_lock lock(other.eventLock); + state = std::move(other.state); } +Event& Event::operator=(const Event& other) +{ + if(this != &other) + { + std::scoped_lock lock(eventLock, other.eventLock); + state = other.state; + } + return *this; +} Event& Event::operator=(Event&& other) { if(this != &other) { std::scoped_lock lock(eventLock, other.eventLock); - name = std::move(other.name); - location = std::move(other.location); + state = std::move(other.state); } return *this; } @@ -42,26 +58,26 @@ Event& Event::operator=(Event&& other) void Event::raise() { std::scoped_lock lock(eventLock); - data = true; - if(waitingJobs.size() > 0) + state->data = true; + if(state->waitingJobs.size() > 0) { - getGlobalThreadPool().scheduleBatch(waitingJobs); + getGlobalThreadPool().scheduleBatch(state->waitingJobs); } - if(waitingMainJobs.size() > 0) + if(state->waitingMainJobs.size() > 0) { - getGlobalThreadPool().scheduleBatch(waitingMainJobs); + getGlobalThreadPool().scheduleBatch(state->waitingMainJobs); } } void Event::reset() { std::scoped_lock lock(eventLock); - data = false; + state->data = false; } bool Event::await_ready() { eventLock.lock(); - bool result = data; + bool result = state->data; if(result) { eventLock.unlock(); @@ -72,14 +88,14 @@ bool Event::await_ready() void Event::await_suspend(std::coroutine_handle> h) { //h.promise().enqueue(this); - waitingJobs.add(JobBase(&h.promise())); + state->waitingJobs.add(JobBase(&h.promise())); eventLock.unlock(); } void Event::await_suspend(std::coroutine_handle> h) { //h.promise().enqueue(this); - waitingMainJobs.add(JobBase(&h.promise())); + state->waitingMainJobs.add(JobBase(&h.promise())); eventLock.unlock(); } @@ -117,6 +133,7 @@ void ThreadPool::waitIdle() std::unique_lock lock(numIdlingLock); if(numIdling == workers.size()) { + assert(promises.size() == 0); return; } numIdlingIncr.wait(lock); diff --git a/src/Engine/ThreadPool.h b/src/Engine/ThreadPool.h index f78b7d7..2ac5342 100644 --- a/src/Engine/ThreadPool.h +++ b/src/Engine/ThreadPool.h @@ -19,35 +19,35 @@ public: //Event(nullptr_t); Event(const std::string& name, const std::source_location& location = std::source_location::current()); Event(const std::source_location& location = std::source_location::current()); - Event(const Event& other) = delete; + Event(const Event& other); Event(Event&& other); ~Event() = default; - Event& operator=(const Event& other) = delete; + Event& operator=(const Event& other); Event& operator=(Event&& other); auto operator<=>(const Event& other) const { - return name <=> other.name; + return state->name <=> other.state->name; } bool operator==(const Event& other) const { - return name == other.name; + return state->name == other.state->name; } operator bool() { std::scoped_lock lock(eventLock); - return data; + return state->data; } friend std::ostream& operator<<(std::ostream& stream, const Event& event) { stream - << event.location.file_name() + << event.state->location.file_name() << "(" - << event.location.line() + << event.state->location.line() << ":" - << event.location.column() + << event.state->location.column() << "): " - << event.location.function_name(); + << event.state->location.function_name(); return stream; } @@ -58,12 +58,16 @@ public: void await_suspend(std::coroutine_handle> h); constexpr void await_resume() {} private: - std::string name; - std::source_location location; - std::mutex eventLock; - bool data = false; - Array> waitingJobs; - Array> waitingMainJobs; + mutable std::mutex eventLock; + struct EventState + { + std::string name; + std::source_location location; + bool data = false; + Array> waitingJobs; + Array> waitingMainJobs; + }; + std::shared_ptr state; friend class ThreadPool; }; @@ -81,14 +85,14 @@ struct JobPromiseBase DONE }; JobPromiseBase(const std::source_location& location = std::source_location::current()) - : pad0(0x7472617453) + : pad0(12345)//0x7472617453) , handle(std::coroutine_handle>::from_promise(*this)) , waitingFor(nullptr) , continuation(nullptr) , numRefs(0) , finishedEvent(location) , state(State::READY) - , pad1(0x646E45) + , pad1(12345)//0x646E45) { if constexpr(!MainJob) { @@ -119,6 +123,7 @@ struct JobPromiseBase void resume() { + validate(); if(!handle || handle.done() || executing()) { return; @@ -128,6 +133,7 @@ struct JobPromiseBase } void setContinuation(JobPromiseBase* cont) { + validate(); assert(cont->ready()); continuation = cont; cont->state = State::SCHEDULED; @@ -136,32 +142,39 @@ struct JobPromiseBase } bool done() { + validate(); return state == State::DONE; } bool scheduled() { + validate(); return state == State::SCHEDULED; } bool waiting() { + validate(); return state == State::WAITING; } bool executing() { + validate(); return state == State::EXECUTING; } bool ready() { + validate(); return state == State::READY; } void enqueue(Event* event); bool schedule(); void addRef() { + validate(); numRefs++; } void removeRef() { + validate(); numRefs--; if(numRefs == 0) { @@ -171,6 +184,11 @@ struct JobPromiseBase } } } + void validate() + { + assert(pad0 == 12345); + assert(pad1 == 12345); + } uint64 pad0; std::coroutine_handle handle; Event* waitingFor; @@ -199,7 +217,10 @@ public: JobBase(const JobBase& other) { promise = other.promise; - promise->addRef(); + if(promise != nullptr) + { + promise->addRef(); + } } JobBase(JobBase&& other) { @@ -223,7 +244,10 @@ public: promise->removeRef(); } promise = other.promise; - promise->addRef(); + if(promise != nullptr) + { + promise->addRef(); + } } return *this; } @@ -245,15 +269,15 @@ public: promise->resume(); } template - inline JobBase&& then(Callable callable) + inline JobBase then(Callable callable) { return then(callable()); } - JobBase&& then(JobBase&& continuation) + JobBase then(JobBase continuation) { promise->setContinuation(continuation.promise); - promise->schedule(); - return std::move(continuation); + continuation.promise->state = JobPromiseBase::State::SCHEDULED; + return continuation; } bool done() { @@ -267,9 +291,11 @@ public: return promise->finishedEvent; } static JobBase all() = delete; + template requires std::same_as, JobBase> static JobBase all(Iterable collection); + template static JobBase all(Iterable collection) { @@ -278,10 +304,11 @@ public: co_await it; } } + template - static JobBase all(Awaitable&&... jobs) + static JobBase all(Awaitable... jobs) { - return JobBase::all(Array{jobs...}); + return JobBase::all(std::initializer_list{jobs...}); } template requires std::invocable> @@ -299,7 +326,7 @@ using Promise = JobPromiseBase; class ThreadPool { public: - ThreadPool(uint32 threadCount = 1);//std::thread::hardware_concurrency()); + ThreadPool(uint32 threadCount = std::thread::hardware_concurrency()); virtual ~ThreadPool(); void waitIdle(); void scheduleJob(Job job); @@ -313,7 +340,7 @@ public: { //job.promise->addRef(); job.promise->state = JobPromiseBase::State::SCHEDULED; - mainJobs.add(job); + mainJobs.add(std::move(job)); } mainJobCV.notify_one(); } @@ -326,7 +353,7 @@ public: { //job.promise->addRef(); job.promise->state = JobPromiseBase::State::SCHEDULED; - jobQueue.add(job); + jobQueue.add(std::move(job)); } jobQueueCV.notify_all(); } @@ -360,12 +387,14 @@ inline JobBase JobPromiseBase::get_return_object() noexcept template inline auto JobPromiseBase::initial_suspend() noexcept { + validate(); return std::suspend_always{}; } template inline auto JobPromiseBase::final_suspend() noexcept { + validate(); state = State::DONE; finishedEvent.raise(); if(continuation) @@ -375,20 +404,10 @@ inline auto JobPromiseBase::final_suspend() noexcept } return std::suspend_always{}; } -//template -//inline void JobPromiseBase::enqueue(Event* event) -//{ -// if(!handle || handle.done() || waiting() || scheduled()) -// { -// return; -// } -// state = State::WAITING; -// waitingFor = event; -// getGlobalThreadPool().enqueueWaiting(event, std::move(JobBase(this))); -//} template inline bool JobPromiseBase::schedule() { + validate(); if(!handle || done() || !ready()) { return false; diff --git a/src/Engine/Window/SceneView.cpp b/src/Engine/Window/SceneView.cpp index 1acc24c..e0ea849 100644 --- a/src/Engine/Window/SceneView.cpp +++ b/src/Engine/Window/SceneView.cpp @@ -20,13 +20,13 @@ Seele::SceneView::SceneView(Gfx::PGraphics graphics, PWindow owner, const Viewpo { scene = new Scene(graphics); scene->addActor(activeCamera); - AssetRegistry::importFile("/home/dynamitos/TestSeeleProject/Assets/Ayaka/Avatar_Girl_Sword_Ayaka_Tex_Body_Diffuse.png"); - AssetRegistry::importFile("/home/dynamitos/TestSeeleProject/Assets/Ayaka/Avatar_Girl_Sword_Ayaka_Tex_Body_Lightmap.png"); - AssetRegistry::importFile("/home/dynamitos/TestSeeleProject/Assets/Ayaka/Avatar_Girl_Sword_Ayaka_Tex_Face_Diffuse.png"); - AssetRegistry::importFile("/home/dynamitos/TestSeeleProject/Assets/Ayaka/Avatar_Girl_Sword_Ayaka_Tex_Hair_Diffuse.png"); - AssetRegistry::importFile("/home/dynamitos/TestSeeleProject/Assets/Ayaka/Avatar_Girl_Sword_Ayaka_Tex_Hair_Lightmap.png"); - AssetRegistry::importFile("/home/dynamitos/TestSeeleProject/Assets/Ayaka/Avatar_Girl_Tex_FaceLightmap.png"); - AssetRegistry::importFile("/home/dynamitos/TestSeeleProject/Assets/Ayaka/Ayaka.fbx"); + AssetRegistry::importFile("C:\\Users\\Dynamitos\\TestSeeleProject\\Assets\\Ayaka\\Avatar_Girl_Sword_Ayaka_Tex_Body_Diffuse.png"); + AssetRegistry::importFile("C:\\Users\\Dynamitos\\TestSeeleProject\\Assets\\Ayaka\\Avatar_Girl_Sword_Ayaka_Tex_Body_Lightmap.png"); + AssetRegistry::importFile("C:\\Users\\Dynamitos\\TestSeeleProject\\Assets\\Ayaka\\Avatar_Girl_Sword_Ayaka_Tex_Face_Diffuse.png"); + AssetRegistry::importFile("C:\\Users\\Dynamitos\\TestSeeleProject\\Assets\\Ayaka\\Avatar_Girl_Sword_Ayaka_Tex_Hair_Diffuse.png"); + AssetRegistry::importFile("C:\\Users\\Dynamitos\\TestSeeleProject\\Assets\\Ayaka\\Avatar_Girl_Sword_Ayaka_Tex_Hair_Lightmap.png"); + AssetRegistry::importFile("C:\\Users\\Dynamitos\\TestSeeleProject\\Assets\\Ayaka\\Avatar_Girl_Tex_FaceLightmap.png"); + AssetRegistry::importFile("C:\\Users\\Dynamitos\\TestSeeleProject\\Assets\\Ayaka\\Ayaka.fbx"); PPrimitiveComponent ayaka = new PrimitiveComponent(AssetRegistry::findMesh("Ayaka")); ayaka->setRelativeLocation(Vector(0, 0, 0)); ayaka->setRelativeScale(Vector(10, 10, 10)); @@ -34,12 +34,12 @@ Seele::SceneView::SceneView(Gfx::PGraphics graphics, PWindow owner, const Viewpo //AssetRegistry::importFile("D:\\Private\\Programming\\Unreal Engine\\Assets\\Ely\\Ely.fbx"); //AssetRegistry::importFile("D:\\Private\\Programming\\Unreal Engine\\Assets\\Cube\\cube.obj"); - //AssetRegistry::importFile("D:\\Private\\Programming\\Unreal Engine\\Assets\\Plane\\plane.fbx"); - //PPrimitiveComponent plane = new PrimitiveComponent(AssetRegistry::findMesh("plane")); - //plane->setRelativeScale(Vector(100, 100, 100)); - //scene->addPrimitiveComponent(plane); + AssetRegistry::importFile("D:\\Private\\Programming\\Unreal Engine\\Assets\\Plane\\plane.fbx"); + PPrimitiveComponent plane = new PrimitiveComponent(AssetRegistry::findMesh("plane")); + plane->setRelativeScale(Vector(100, 100, 100)); + scene->addPrimitiveComponent(plane); - for(uint32 i = 0; i < 100000; ++i) + for(uint32 i = 0; i < 10; ++i) { PMyComponent myComp = new MyComponent(); PMyOtherComponent myOtherComp = new MyOtherComponent(); diff --git a/src/Engine/main.cpp b/src/Engine/main.cpp index 51f7ca7..4c29747 100644 --- a/src/Engine/main.cpp +++ b/src/Engine/main.cpp @@ -9,7 +9,7 @@ using namespace Seele; int main() { PWindowManager windowManager = new WindowManager(); - AssetRegistry::init("/home/dynamitos/TestSeeleProject/"); + AssetRegistry::init("C:\\Users\\Dynamitos\\TestSeeleProject"); WindowCreateInfo mainWindowInfo; mainWindowInfo.title = "SeeleEngine"; mainWindowInfo.width = 1280; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0275b54..404de08 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -7,7 +7,9 @@ include_directories(${Boost_INCLUDE_DIRS}) add_subdirectory(Engine/) target_link_libraries(Seele_unit_tests ${Boost_LIBRARIES}) -target_compile_definitions(Seele_unit_tests PRIVATE -DBOOST_TEST_DYN_LINK) +if(UNIX) + target_compile_definitions(Seele_unit_tests PRIVATE -DBOOST_TEST_DYN_LINK) +endif() target_precompile_headers(Seele_unit_tests PRIVATE diff --git a/test/Engine/EngineTest.h b/test/Engine/EngineTest.h index e90ce6b..df9cb74 100644 --- a/test/Engine/EngineTest.h +++ b/test/Engine/EngineTest.h @@ -1,5 +1,5 @@ #pragma once -//#include +#include namespace Seele { @@ -7,16 +7,14 @@ namespace Seele { GlobalFixture() { - //_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); + _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); } ~GlobalFixture(); void setup() { - //Fibers::JobQueue::initJobQueues(); } void teardown() { - //Fibers::JobQueue::cleanupJobQueues(); } }; }; \ No newline at end of file