From 17caf6cae9fc891d371b9e2ef2523f30f0a86955 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 17 Aug 2023 15:14:05 -0700 Subject: [PATCH] improvements to CompilerThreadPool and OpenGLPlatform CompilerThreadPool: - it now supports a thread cleanup function - some initialization is moved to the setup function OpenGLPlatform: - now cleans-up the thread pool threads upon exit --- .../backend/platforms/OpenGLPlatform.h | 6 ++++++ .../include/backend/platforms/PlatformEGL.h | 1 + filament/backend/src/CompilerThreadPool.cpp | 13 ++++++------- filament/backend/src/CompilerThreadPool.h | 16 +++++++++------- .../backend/src/opengl/OpenGLPlatform.cpp | 3 +++ .../src/opengl/ShaderCompilerService.cpp | 14 +++++++++++--- .../src/opengl/platforms/PlatformEGL.cpp | 19 ++++++++++++++++++- 7 files changed, 54 insertions(+), 18 deletions(-) diff --git a/filament/backend/include/backend/platforms/OpenGLPlatform.h b/filament/backend/include/backend/platforms/OpenGLPlatform.h index c41dce4360..3f4488c5f5 100644 --- a/filament/backend/include/backend/platforms/OpenGLPlatform.h +++ b/filament/backend/include/backend/platforms/OpenGLPlatform.h @@ -288,6 +288,12 @@ public: * @see terminate() */ virtual void createContext(bool shared); + + /** + * Detach and destroy the current context if any and releases all resources associated to + * this thread. + */ + virtual void releaseContext() noexcept; }; } // namespace filament diff --git a/filament/backend/include/backend/platforms/PlatformEGL.h b/filament/backend/include/backend/platforms/PlatformEGL.h index 08124909a2..7940054006 100644 --- a/filament/backend/include/backend/platforms/PlatformEGL.h +++ b/filament/backend/include/backend/platforms/PlatformEGL.h @@ -40,6 +40,7 @@ public: PlatformEGL() noexcept; bool isExtraContextSupported() const noexcept override; void createContext(bool shared) override; + void releaseContext() noexcept override; protected: diff --git a/filament/backend/src/CompilerThreadPool.cpp b/filament/backend/src/CompilerThreadPool.cpp index eeb116db3b..591ae063c3 100644 --- a/filament/backend/src/CompilerThreadPool.cpp +++ b/filament/backend/src/CompilerThreadPool.cpp @@ -34,17 +34,14 @@ CompilerThreadPool::~CompilerThreadPool() noexcept { assert_invariant(mQueues[1].empty()); } -void CompilerThreadPool::init(uint32_t threadCount, JobSystem::Priority priority, - ThreadSetup&& threadSetup) noexcept { +void CompilerThreadPool::init(uint32_t threadCount, + ThreadSetup&& threadSetup, ThreadCleanup&& threadCleanup) noexcept { auto setup = std::make_shared(std::move(threadSetup)); + auto cleanup = std::make_shared(std::move(threadCleanup)); for (size_t i = 0; i < threadCount; i++) { - mCompilerThreads.emplace_back([this, priority, setup]() { + mCompilerThreads.emplace_back([this, setup, cleanup]() { SYSTRACE_CONTEXT(); - // give the thread a name - JobSystem::setThreadName("CompilerThreadPool"); - // run at a slightly lower priority than other filament threads - JobSystem::setThreadPriority(priority); (*setup)(); @@ -80,6 +77,8 @@ void CompilerThreadPool::init(uint32_t threadCount, JobSystem::Priority priority job(); } } + + (*cleanup)(); }); } diff --git a/filament/backend/src/CompilerThreadPool.h b/filament/backend/src/CompilerThreadPool.h index 78ce4c7079..fbdff68f69 100644 --- a/filament/backend/src/CompilerThreadPool.h +++ b/filament/backend/src/CompilerThreadPool.h @@ -20,12 +20,13 @@ #include #include -#include +#include +#include #include -#include #include #include +#include #include #include @@ -45,8 +46,9 @@ public: ~CompilerThreadPool() noexcept; using Job = utils::Invocable; using ThreadSetup = utils::Invocable; - void init(uint32_t threadCount, utils::JobSystem::Priority priority, - ThreadSetup&& threadSetup) noexcept; + using ThreadCleanup = utils::Invocable; + void init(uint32_t threadCount, + ThreadSetup&& threadSetup, ThreadCleanup&& threadCleanup) noexcept; void terminate() noexcept; void queue(CompilerPriorityQueue priorityQueue, program_token_t const& token, Job&& job); Job dequeue(program_token_t const& token); @@ -54,9 +56,9 @@ public: private: using Queue = std::deque>; std::vector mCompilerThreads; - std::atomic_bool mExitRequested{false}; - std::mutex mQueueLock; - std::condition_variable mQueueCondition; + bool mExitRequested{ false }; + utils::Mutex mQueueLock; + utils::Condition mQueueCondition; std::array mQueues; // lock must be held for methods below std::pair find(program_token_t const& token); diff --git a/filament/backend/src/opengl/OpenGLPlatform.cpp b/filament/backend/src/opengl/OpenGLPlatform.cpp index 4297479d97..837faecdc4 100644 --- a/filament/backend/src/opengl/OpenGLPlatform.cpp +++ b/filament/backend/src/opengl/OpenGLPlatform.cpp @@ -116,4 +116,7 @@ bool OpenGLPlatform::isExtraContextSupported() const noexcept { void OpenGLPlatform::createContext(bool) { } +void OpenGLPlatform::releaseContext() noexcept { +} + } // namespace filament::backend diff --git a/filament/backend/src/opengl/ShaderCompilerService.cpp b/filament/backend/src/opengl/ShaderCompilerService.cpp index b7b2302234..d3d4cdd065 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.cpp +++ b/filament/backend/src/opengl/ShaderCompilerService.cpp @@ -175,10 +175,18 @@ void ShaderCompilerService::init() noexcept { } mShaderCompilerThreadCount = poolSize; - mCompilerThreadPool.init(mShaderCompilerThreadCount, priority, - [platform = &mDriver.mPlatform]() { + mCompilerThreadPool.init(mShaderCompilerThreadCount, + [&platform = mDriver.mPlatform, priority]() { + // give the thread a name + JobSystem::setThreadName("CompilerThreadPool"); + // run at a slightly lower priority than other filament threads + JobSystem::setThreadPriority(priority); // create a gl context current to this thread - platform->createContext(true); + platform.createContext(true); + }, + [&platform = mDriver.mPlatform]() { + // release context and thread state + platform.releaseContext(); }); } } diff --git a/filament/backend/src/opengl/platforms/PlatformEGL.cpp b/filament/backend/src/opengl/platforms/PlatformEGL.cpp index 59f5a89054..60652b5415 100644 --- a/filament/backend/src/opengl/platforms/PlatformEGL.cpp +++ b/filament/backend/src/opengl/platforms/PlatformEGL.cpp @@ -294,6 +294,22 @@ void PlatformEGL::createContext(bool shared) { mAdditionalContexts.push_back(context); } +void PlatformEGL::releaseContext() noexcept { + EGLContext context = eglGetCurrentContext(); + eglMakeCurrent(mEGLDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + if (context != EGL_NO_CONTEXT) { + eglDestroyContext(mEGLDisplay, context); + } + + mAdditionalContexts.erase( + std::remove_if(mAdditionalContexts.begin(), mAdditionalContexts.end(), + [context](EGLContext c) { + return c == context; + }), mAdditionalContexts.end()); + + eglReleaseThread(); +} + EGLBoolean PlatformEGL::makeCurrent(EGLSurface drawSurface, EGLSurface readSurface) noexcept { if (UTILS_UNLIKELY((drawSurface != mCurrentDrawSurface || readSurface != mCurrentReadSurface))) { mCurrentDrawSurface = drawSurface; @@ -304,7 +320,8 @@ EGLBoolean PlatformEGL::makeCurrent(EGLSurface drawSurface, EGLSurface readSurfa } void PlatformEGL::terminate() noexcept { - eglMakeCurrent(mEGLDisplay, mEGLDummySurface, mEGLDummySurface, EGL_NO_CONTEXT); + // it's always allowed to use EGL_NO_SURFACE, EGL_NO_CONTEXT + eglMakeCurrent(mEGLDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); if (mEGLDummySurface) { eglDestroySurface(mEGLDisplay, mEGLDummySurface); }