Merge pull request #1688 from turtleli/gsdx-thread

gsdx: Use std::thread and std::function for GSJobQueue

plugins/GSdx/CMakeLists.txt

@@ -104,7 +104,6 @@ set(GSdxSources
     GSTextureOGL.cpp
     GSTextureNull.cpp
     GSTextureSW.cpp
-    GSThread.cpp
     GSUtil.cpp
     GSVector.cpp
     GSVertexTrace.cpp
@@ -164,6 +163,7 @@ set(GSdxHeaders
     GSTextureCacheOGL.h
     GSTextureNull.h
     GSThread.h
+    GSThread_CXX11.h
     GSUtil.h
     GSVector.h
     GSVertex.h

plugins/GSdx/GSCapture.cpp

@@ -492,7 +492,7 @@ bool GSCapture::BeginCapture(float fps, GSVector2i recomendedResolution, float a
 	m_size.y = theApp.GetConfigI("CaptureHeight");
 
 	for(int i = 0; i < m_threads; i++) {
-		m_workers.push_back(std::unique_ptr<GSPng::Worker>(new GSPng::Worker()));
+		m_workers.push_back(std::unique_ptr<GSPng::Worker>(new GSPng::Worker(&GSPng::Process)));
 	}
 #endif
 
@@ -555,10 +555,6 @@ bool GSCapture::EndCapture()
 	}
 
 #elif defined(__unix__)
-	// XXX Might not be necessary to wait
-	for(size_t i = 0; i < m_workers.size(); i++) {
-		m_workers[i]->Wait();
-	}
 	m_workers.clear();
 
 	m_frame = 0;

plugins/GSdx/GSPng.cpp

@@ -140,7 +140,7 @@ namespace GSPng {
             _aligned_free(m_image);
     }
 
-    void Worker::Process(shared_ptr<Transaction>& item)
+    void Process(std::shared_ptr<Transaction>& item)
     {
         Save(item->m_fmt, item->m_file, item->m_image, item->m_w, item->m_h, item->m_pitch, item->m_compression);
     }

plugins/GSdx/GSPng.h

@@ -52,14 +52,7 @@ namespace GSPng {
 
     bool Save(GSPng::Format fmt, const string& file, uint8* image, int w, int h, int pitch, int compression, bool rb_swapped = false);
 
-	class Worker : public GSJobQueue<shared_ptr<Transaction>, 16 >
+    void Process(std::shared_ptr<Transaction> &item);
-	{
-		public:
-			Worker() {};
-			virtual ~Worker() {};
 
-			void Process(shared_ptr<Transaction>& item);
+    using Worker = GSJobQueue<std::shared_ptr<Transaction>, 16>;
-
-			int GetPixels(bool reset) {return 0;}
-	};
 }

plugins/GSdx/GSRasterizer.cpp

@@ -1158,11 +1158,6 @@ GSRasterizerList::GSRasterizerList(int threads, GSPerfMon* perfmon)
 
 GSRasterizerList::~GSRasterizerList()
 {
-	for(auto i = m_workers.begin(); i != m_workers.end(); i++)
-	{
-		delete *i;
-	}
-
 	_aligned_free(m_scanline);
 }
 
@@ -1213,33 +1208,8 @@ int GSRasterizerList::GetPixels(bool reset)
 
 	for(size_t i = 0; i < m_workers.size(); i++)
 	{
-		pixels += m_workers[i]->GetPixels(reset);
+		pixels += m_r[i]->GetPixels(reset);
 	}
 
 	return pixels;
 }
-
-// GSRasterizerList::GSWorker
-
-GSRasterizerList::GSWorker::GSWorker(GSRasterizer* r)
-	: GSJobQueue<shared_ptr<GSRasterizerData>, 65536>()
-	, m_r(r)
-{
-}
-
-GSRasterizerList::GSWorker::~GSWorker()
-{
-	Wait();
-
-	delete m_r;
-}
-
-int GSRasterizerList::GSWorker::GetPixels(bool reset)
-{
-	return m_r->GetPixels(reset);
-}
-
-void GSRasterizerList::GSWorker::Process(shared_ptr<GSRasterizerData>& item)
-{
-	m_r->Draw(item.get());
-}

plugins/GSdx/GSRasterizer.h

@@ -181,23 +181,12 @@ public:
 class GSRasterizerList : public IRasterizer
 {
 protected:
-	class GSWorker : public GSJobQueue<shared_ptr<GSRasterizerData>, 65536 >
+	using GSWorker = GSJobQueue<std::shared_ptr<GSRasterizerData>, 65536>;
-	{
-		GSRasterizer* m_r;
-
-	public:
-		GSWorker(GSRasterizer* r);
-		virtual ~GSWorker();
-
-		int GetPixels(bool reset);
-
-		// GSJobQueue
-
-		void Process(shared_ptr<GSRasterizerData>& item);
-	};
 
 	GSPerfMon* m_perfmon;
-	vector<GSWorker*> m_workers;
+	// Worker threads depend on the rasterizers, so don't change the order.
+	std::vector<std::unique_ptr<GSRasterizer>> m_r;
+	std::vector<std::unique_ptr<GSWorker>> m_workers;
 	uint8* m_scanline;
 	int m_thread_height;
 
@@ -214,17 +203,18 @@ public:
 		{
 			return new GSRasterizer(new DS(), 0, 1, perfmon);
 		}
-		else
+
+		GSRasterizerList* rl = new GSRasterizerList(threads, perfmon);
+
+		for(int i = 0; i < threads; i++)
 		{
-			GSRasterizerList* rl = new GSRasterizerList(threads, perfmon);
+			rl->m_r.push_back(std::unique_ptr<GSRasterizer>(new GSRasterizer(new DS(), i, threads, perfmon)));
-
+			auto &r = *rl->m_r[i];
-			for(int i = 0; i < threads; i++)
+			rl->m_workers.push_back(std::unique_ptr<GSWorker>(new GSWorker(
-			{
+				[&r](std::shared_ptr<GSRasterizerData> &item) { r.Draw(item.get()); })));
-				rl->m_workers.push_back(new GSWorker(new GSRasterizer(new DS(), i, threads, perfmon)));
-			}
-
-			return rl;
 		}
+
+		return rl;
 	}
 
 	// IRasterizer

plugins/GSdx/GSThread.cpp

@@ -1,129 +0,0 @@
-/*
- *	Copyright (C) 2007-2009 Gabest
- *	http://www.gabest.org
- *
- *  This Program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2, or (at your option)
- *  any later version.
- *
- *  This Program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with GNU Make; see the file COPYING.  If not, write to
- *  the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA USA.
- *  http://www.gnu.org/copyleft/gpl.html
- *
- */
-
-#include "stdafx.h"
-#include "GSThread_CXX11.h"
-
-GSThread::GSThread()
-{
-    #ifdef _WIN32
-
-	m_ThreadId = 0;
-	m_hThread = NULL;
-
-    #else
-
-    #endif
-}
-
-GSThread::~GSThread()
-{
-}
-
-#ifdef _WIN32
-
-DWORD WINAPI GSThread::StaticThreadProc(void* lpParam)
-{
-	((GSThread*)lpParam)->ThreadProc();
-
-	return 0;
-}
-
-#else
-
-void* GSThread::StaticThreadProc(void* param)
-{
-	((GSThread*)param)->ThreadProc();
-#ifndef _STD_THREAD_ // exit is done implicitly by std::thread
-	pthread_exit(NULL);
-#endif
-	return NULL;
-}
-
-#endif
-
-void GSThread::CreateThread()
-{
-    #ifdef _WIN32
-
-	m_hThread = ::CreateThread(NULL, 0, StaticThreadProc, (void*)this, 0, &m_ThreadId);
-
-	#else
-    
-    #ifdef _STD_THREAD_
-    t = new thread(StaticThreadProc,(void*)this);
-    #else
-    pthread_attr_init(&m_thread_attr);
-    pthread_create(&m_thread, &m_thread_attr, StaticThreadProc, (void*)this);
-    #endif
-
-	#endif
-}
-
-void GSThread::CloseThread()
-{
-    #ifdef _WIN32
-
-	if(m_hThread != NULL)
-	{
-		if(WaitForSingleObject(m_hThread, 5000) != WAIT_OBJECT_0)
-		{
-			printf("GSdx: WARNING: GSThread Thread did not close itself in time. Assuming hung. Terminating.\n");
-			TerminateThread(m_hThread, 1);
-		}
-
-		CloseHandle(m_hThread);
-
-		m_hThread = NULL;
-		m_ThreadId = 0;
-	}
-
-    #else
-    // Should be tested on windows too one day, native handle should be disabled there though, or adapted to windows thread
-    #ifdef _STD_THREAD_
-    
-    #define _NATIVE_HANDLE_ // Using std::thread native handle, allows to just use posix stuff.
-    #ifdef _NATIVE_HANDLE_ // std::thread join seems to be bugged, have to test it again every now and then, it did work at some point(gcc 5), seems there is bug in system lib...
-    pthread_t m_thread = t->native_handle();
-    void *ret = NULL;
-    pthread_join(m_thread, &ret);
-    /* We are sure thread is dead, not so bad.
-     * Still no way to to delete that crap though... Really, wtf is this standard??
-     * I guess we will have to wait that someone debug either the implementation or change standard.
-     * There should be a moderate memory leak for now, I am trying to find a way to fix it.
-     * 3kinox
-     */
-    #else
-    if(t->joinable())
-    {
-        t->join();
-    }
-    delete(t);
-    #endif
-    #else
-    void* ret = NULL;
-
-    pthread_join(m_thread, &ret);
-    pthread_attr_destroy(&m_thread_attr);
-    #endif
-    #endif
-}
-

plugins/GSdx/GSThread_CXX11.h

@@ -24,81 +24,11 @@
 #include "GSdx.h"
 #include "boost_spsc_queue.hpp"
 
-class IGSThread
+template<class T, int CAPACITY> class GSJobQueue final
 {
-protected:
+private:
-	virtual void ThreadProc() = 0;
+	std::thread m_thread;
-};
+	std::function<void(T&)> m_func;
-
-// let us use std::thread for now, comment out the definition to go back to pthread
-// There are currently some bugs/limitations to std::thread (see various comment)
-// For the moment let's keep pthread but uses new std object (mutex, cond_var)
-//#define _STD_THREAD_
-
-#ifdef _WIN32
-
-class GSThread : public IGSThread
-{
-    DWORD m_ThreadId;
-    HANDLE m_hThread;
-
-	static DWORD WINAPI StaticThreadProc(void* lpParam);
-
-protected:
-	void CreateThread();
-	void CloseThread();
-
-public:
-	GSThread();
-	virtual ~GSThread();
-};
-
-#else
-
-#ifdef _STD_THREAD_
-#include <thread>
-#else
-#include <pthread.h>
-#endif
-
-class GSThread : public IGSThread
-{
-    #ifdef _STD_THREAD_
-    std::thread *t;
-    #else
-    pthread_attr_t m_thread_attr;
-    pthread_t m_thread;
-    #endif
-    static void* StaticThreadProc(void* param);
-
-protected:
-	void CreateThread();
-	void CloseThread();
-
-public:
-	GSThread();
-	virtual ~GSThread();
-};
-
-#endif
-
-template<class T> class IGSJobQueue : public GSThread
-{
-public:
-	IGSJobQueue() {}
-	virtual ~IGSJobQueue() {}
-
-	virtual bool IsEmpty() const = 0;
-	virtual void Push(const T& item) = 0;
-	virtual void Wait() = 0;
-
-	virtual void Process(T& item) = 0;
-	virtual int GetPixels(bool reset) = 0;
-};
-
-template<class T, int CAPACITY> class GSJobQueue : public IGSJobQueue<T>
-{
-protected:
 	std::atomic<int16_t> m_count;
 	std::atomic<bool> m_exit;
 	ringbuffer_base<T, CAPACITY> m_queue;
@@ -139,19 +69,22 @@ protected:
 	}
 
 public:
-	GSJobQueue() :
+	GSJobQueue(std::function<void(T&)> func) :
+		m_func(func),
 		m_count(0),
 		m_exit(false)
 	{
-		this->CreateThread();
+		m_thread = std::thread(&GSJobQueue::ThreadProc, this);
 	}
 
-	virtual ~GSJobQueue() {
+	~GSJobQueue()
+	{
 		m_exit = true;
 		do {
 			m_notempty.notify_one();
 		} while (m_exit);
-		this->CloseThread();
+
+		m_thread.join();
 	}
 
 	bool IsEmpty() const {
@@ -185,6 +118,6 @@ public:
 	}
 
 	void operator() (T& item) {
-		this->Process(item);
+		m_func(item);
 	}
 };

plugins/GSdx/GSdx.vcxproj

@@ -195,7 +195,6 @@
     <ClCompile Include="GSTextureFX9.cpp" />
     <ClCompile Include="GSTextureNull.cpp" />
     <ClCompile Include="GSTextureSW.cpp" />
-    <ClCompile Include="GSThread.cpp" />
     <ClCompile Include="GSUtil.cpp" />
     <ClCompile Include="GSVector.cpp" />
     <ClCompile Include="GSVertexList.cpp" />
@@ -218,6 +217,7 @@
     </ClCompile>
   </ItemGroup>
   <ItemGroup>
+    <ClInclude Include="boost_spsc_queue.hpp" />
     <ClInclude Include="config.h" />
     <ClInclude Include="GLLoader.h" />
     <ClInclude Include="GLState.h" />
@@ -278,6 +278,7 @@
     <ClInclude Include="GSTextureNull.h" />
     <ClInclude Include="GSTextureSW.h" />
     <ClInclude Include="GSThread.h" />
+    <ClInclude Include="GSThread_CXX11.h" />
     <ClInclude Include="GSUniformBufferOGL.h" />
     <ClInclude Include="GSUtil.h" />
     <ClInclude Include="GSVector.h" />

plugins/GSdx/GSdx.vcxproj.filters

@@ -171,9 +171,6 @@
     <ClCompile Include="GSTextureNull.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="GSThread.cpp">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="GSUtil.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
@@ -566,6 +563,12 @@
     <ClInclude Include="PSX\GPUVertex.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="boost_spsc_queue.hpp">
+      <Filter>Header Files</Filter>
+    </ClInclude>
+    <ClInclude Include="GSThread_CXX11.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <None Include="res\logo10.bmp">

plugins/GSdx/stdafx.h

@@ -107,6 +107,7 @@ typedef int64 sint64;
 #include <atomic>
 #include <mutex>
 #include <condition_variable>
+#include <functional>
 
 using namespace std;