Make the minimum items per thread explicit. Found some bugs, optional arguments are evil.

Common/Thread/ParallelLoop.cpp

@@ -58,15 +58,16 @@ WaitableCounter *ParallelRangeLoopWaitable(ThreadManager *threadMan, const std::
 }
 
 void ParallelRangeLoop(ThreadManager *threadMan, const std::function<void(int, int)> &loop, int lower, int upper, int minSize) {
-	if (cpu_info.num_cores == 1) {
-		// "Optimization" for single-core devices.
-		// No point in adding threading overhead, let's just do it inline.
+	if (cpu_info.num_cores == 1 || (minSize >= (upper - lower) && upper > lower)) {
+		// "Optimization" for single-core devices, or minSize larger than the range.
+		// No point in adding threading overhead, let's just do it inline (since this is the blocking variant).
 		loop(lower, upper);
 		return;
 	}
 
-	if (minSize == -1) {
-		minSize = 4;
+	if (minSize < 1) {
+		// There's no obvious value to default to.
+		minSize = 1;
 	}
 
 	WaitableCounter *counter = ParallelRangeLoopWaitable(threadMan, loop, lower, upper, minSize);
@@ -77,6 +78,7 @@ void ParallelRangeLoop(ThreadManager *threadMan, const std::function<void(int, i
 	}
 }
 
+// NOTE: Supports a max of 2GB.
 void ParallelMemcpy(ThreadManager *threadMan, void *dst, const void *src, size_t bytes) {
 	// This threshold can probably be a lot bigger.
 	if (bytes < 512) {
@@ -91,10 +93,10 @@ void ParallelMemcpy(ThreadManager *threadMan, void *dst, const void *src, size_t
 	char *s = (char *)src;
 	ParallelRangeLoop(threadMan, [&](int l, int h) {
 		memmove(d + l, s + l, h - l);
-	}, 0, 128);
+	}, 0, (int)bytes, 128);
 }
 
-
+// NOTE: Supports a max of 2GB.
 void ParallelMemset(ThreadManager *threadMan, void *dst, uint8_t value, size_t bytes) {
 	// This threshold can probably be a lot bigger.
 	if (bytes < 512) {
@@ -108,5 +110,5 @@ void ParallelMemset(ThreadManager *threadMan, void *dst, uint8_t value, size_t b
 	char *d = (char *)dst;
 	ParallelRangeLoop(threadMan, [&](int l, int h) {
 		memset(d + l, value, h - l);
-	}, 0, 128);
+	}, 0, (int)bytes, 128);
 }

Common/Thread/ParallelLoop.h

@@ -37,7 +37,7 @@ public:
 WaitableCounter *ParallelRangeLoopWaitable(ThreadManager *threadMan, const std::function<void(int, int)> &loop, int lower, int upper, int minSize);
 
 // This one optimizes by running the remainder on the calling thread.
-void ParallelRangeLoop(ThreadManager *threadMan, const std::function<void(int, int)> &loop, int lower, int upper, int minSize = -1);
+void ParallelRangeLoop(ThreadManager *threadMan, const std::function<void(int, int)> &loop, int lower, int upper, int minSize);
 
 // Common utilities for large (!) memory copies.
 // Will only fall back to threads if it seems to make sense.

Core/MemMap.cpp

@@ -330,7 +330,7 @@ static void DoMemoryVoid(PointerWrap &p, uint32_t start, uint32_t size) {
 		ParallelRangeLoop(&g_threadManager, [&](int l, int h) {
 			for (int i = l; i < h; i++)
 				_dbg_assert_msg_(d[i] == storage[i], "Savestate verification failure: %d (0x%X) (at %p) != %d (0x%X) (at %p).\n", d[i], d[i], &d[i], storage[i], storage[i], &storage[i]);
-		}, 0, size);
+		}, 0, size, 128);
 		break;
 	}
 	storage += size;

Core/TextureReplacer.cpp

@@ -748,12 +748,13 @@ void ReplacedTexture::Load(int level, void *out, int rowPitch) {
 
 		int w, h, f;
 		uint8_t *image;
+		const int MIN_LINES_PER_THREAD = 4;
 		if (LoadZIMPtr(&zim[0], zimSize, &w, &h, &f, &image)) {
 			ParallelRangeLoop(&g_threadManager, [&](int l, int h) {
 				for (int y = l; y < h; ++y) {
 					memcpy((uint8_t *)out + rowPitch * y, image + w * 4 * y, w * 4);
 				}
-			}, 0, h);
+			}, 0, h, MIN_LINES_PER_THREAD);
 			free(image);
 		}
 

GPU/Common/TextureScalerCommon.cpp

@@ -606,24 +606,26 @@ bool TextureScalerCommon::Scale(u32* &data, u32 &dstFmt, int &width, int &height
 	return false;
 }
 
+const int MIN_LINES_PER_THREAD = 4;
+
 void TextureScalerCommon::ScaleXBRZ(int factor, u32* source, u32* dest, int width, int height) {
 	xbrz::ScalerCfg cfg;
-	ParallelRangeLoop(&g_threadManager, std::bind(&xbrz::scale, factor, source, dest, width, height, xbrz::ColorFormat::ARGB, cfg, std::placeholders::_1, std::placeholders::_2), 0, height);
+	ParallelRangeLoop(&g_threadManager, std::bind(&xbrz::scale, factor, source, dest, width, height, xbrz::ColorFormat::ARGB, cfg, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 }
 
 void TextureScalerCommon::ScaleBilinear(int factor, u32* source, u32* dest, int width, int height) {
 	bufTmp1.resize(width * height * factor);
 	u32 *tmpBuf = bufTmp1.data();
-	ParallelRangeLoop(&g_threadManager, std::bind(&bilinearH, factor, source, tmpBuf, width, std::placeholders::_1, std::placeholders::_2), 0, height);
-	ParallelRangeLoop(&g_threadManager, std::bind(&bilinearV, factor, tmpBuf, dest, width, 0, height, std::placeholders::_1, std::placeholders::_2), 0, height);
+	ParallelRangeLoop(&g_threadManager, std::bind(&bilinearH, factor, source, tmpBuf, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
+	ParallelRangeLoop(&g_threadManager, std::bind(&bilinearV, factor, tmpBuf, dest, width, 0, height, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 }
 
 void TextureScalerCommon::ScaleBicubicBSpline(int factor, u32* source, u32* dest, int width, int height) {
-	ParallelRangeLoop(&g_threadManager,std::bind(&scaleBicubicBSpline, factor, source, dest, width, height, std::placeholders::_1, std::placeholders::_2), 0, height);
+	ParallelRangeLoop(&g_threadManager,std::bind(&scaleBicubicBSpline, factor, source, dest, width, height, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 }
 
 void TextureScalerCommon::ScaleBicubicMitchell(int factor, u32* source, u32* dest, int width, int height) {
-	ParallelRangeLoop(&g_threadManager,std::bind(&scaleBicubicMitchell, factor, source, dest, width, height, std::placeholders::_1, std::placeholders::_2), 0, height);
+	ParallelRangeLoop(&g_threadManager,std::bind(&scaleBicubicMitchell, factor, source, dest, width, height, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 }
 
 void TextureScalerCommon::ScaleHybrid(int factor, u32* source, u32* dest, int width, int height, bool bicubic) {
@@ -640,8 +642,8 @@ void TextureScalerCommon::ScaleHybrid(int factor, u32* source, u32* dest, int wi
 	bufTmp2.resize(width*height*factor*factor);
 	bufTmp3.resize(width*height*factor*factor);
 
-	ParallelRangeLoop(&g_threadManager,std::bind(&generateDistanceMask, source, bufTmp1.data(), width, height, std::placeholders::_1, std::placeholders::_2), 0, height);
-	ParallelRangeLoop(&g_threadManager,std::bind(&convolve3x3, bufTmp1.data(), bufTmp2.data(), KERNEL_SPLAT, width, height, std::placeholders::_1, std::placeholders::_2), 0, height);
+	ParallelRangeLoop(&g_threadManager,std::bind(&generateDistanceMask, source, bufTmp1.data(), width, height, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
+	ParallelRangeLoop(&g_threadManager,std::bind(&convolve3x3, bufTmp1.data(), bufTmp2.data(), KERNEL_SPLAT, width, height, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 	ScaleBilinear(factor, bufTmp2.data(), bufTmp3.data(), width, height);
 	// mask C is now in bufTmp3
 
@@ -654,13 +656,13 @@ void TextureScalerCommon::ScaleHybrid(int factor, u32* source, u32* dest, int wi
 
 	// Now we can mix it all together
 	// The factor 8192 was found through practical testing on a variety of textures
-	ParallelRangeLoop(&g_threadManager,std::bind(&mix, dest, bufTmp2.data(), bufTmp3.data(), 8192, width*factor, std::placeholders::_1, std::placeholders::_2), 0, height*factor);
+	ParallelRangeLoop(&g_threadManager,std::bind(&mix, dest, bufTmp2.data(), bufTmp3.data(), 8192, width*factor, std::placeholders::_1, std::placeholders::_2), 0, height*factor, MIN_LINES_PER_THREAD);
 }
 
 void TextureScalerCommon::DePosterize(u32* source, u32* dest, int width, int height) {
 	bufTmp3.resize(width*height);
-	ParallelRangeLoop(&g_threadManager,std::bind(&deposterizeH, source, bufTmp3.data(), width, std::placeholders::_1, std::placeholders::_2), 0, height);
-	ParallelRangeLoop(&g_threadManager,std::bind(&deposterizeV, bufTmp3.data(), dest, width, height, std::placeholders::_1, std::placeholders::_2), 0, height);
-	ParallelRangeLoop(&g_threadManager,std::bind(&deposterizeH, dest, bufTmp3.data(), width, std::placeholders::_1, std::placeholders::_2), 0, height);
-	ParallelRangeLoop(&g_threadManager,std::bind(&deposterizeV, bufTmp3.data(), dest, width, height, std::placeholders::_1, std::placeholders::_2), 0, height);
+	ParallelRangeLoop(&g_threadManager,std::bind(&deposterizeH, source, bufTmp3.data(), width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
+	ParallelRangeLoop(&g_threadManager,std::bind(&deposterizeV, bufTmp3.data(), dest, width, height, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
+	ParallelRangeLoop(&g_threadManager,std::bind(&deposterizeH, dest, bufTmp3.data(), width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
+	ParallelRangeLoop(&g_threadManager,std::bind(&deposterizeV, bufTmp3.data(), dest, width, height, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 }

GPU/Common/TextureScalerCommon.h

@@ -22,6 +22,8 @@
 
 #include <vector>
 
+static const int MIN_TEXSCALE_LINES_PER_THREAD = 4;
+
 class TextureScalerCommon {
 public:
 	TextureScalerCommon();

GPU/D3D11/TextureScalerD3D11.cpp

@@ -40,15 +40,15 @@ void TextureScalerD3D11::ConvertTo8888(u32 format, u32* source, u32* &dest, int
 		break;
 
 	case DXGI_FORMAT_B4G4R4A4_UNORM:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert4444_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert4444_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_TEXSCALE_LINES_PER_THREAD);
 		break;
 
 	case DXGI_FORMAT_B5G6R5_UNORM:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert565_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert565_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_TEXSCALE_LINES_PER_THREAD);
 		break;
 
 	case DXGI_FORMAT_B5G5R5A1_UNORM:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert5551_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert5551_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_TEXSCALE_LINES_PER_THREAD);
 		break;
 
 	default:

GPU/Directx9/TextureScalerDX9.cpp

@@ -42,15 +42,15 @@ void TextureScalerDX9::ConvertTo8888(u32 format, u32* source, u32* &dest, int wi
 		break;
 
 	case D3DFMT_A4R4G4B4:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert4444_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert4444_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_TEXSCALE_LINES_PER_THREAD);
 		break;
 
 	case D3DFMT_R5G6B5:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert565_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert565_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_TEXSCALE_LINES_PER_THREAD);
 		break;
 
 	case D3DFMT_A1R5G5B5:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert5551_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert5551_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_TEXSCALE_LINES_PER_THREAD);
 		break;
 
 	default:

GPU/GLES/TextureScalerGLES.cpp

@@ -42,15 +42,15 @@ void TextureScalerGLES::ConvertTo8888(u32 format, u32* source, u32* &dest, int w
 		break;
 
 	case Draw::DataFormat::R4G4B4A4_UNORM_PACK16:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert4444_gl, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert4444_gl, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, 1);
 		break;
 
 	case Draw::DataFormat::R5G6B5_UNORM_PACK16:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert565_gl, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert565_gl, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, 1);
 		break;
 
 	case Draw::DataFormat::R5G5B5A1_UNORM_PACK16:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert5551_gl, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert5551_gl, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, 1);
 		break;
 
 	default:

GPU/Software/Rasterizer.cpp

@@ -1322,29 +1322,32 @@ void DrawTriangle(const VertexData& v0, const VertexData& v1, const VertexData&
 	// 32 because we do two pixels at once, and we don't want overlap.
 	int rangeY = (maxY - minY) / 32 + 1;
 	int rangeX = (maxX - minX) / 32 + 1;
+
+	const int MIN_LINES_PER_THREAD = 4;
+
 	if (rangeY >= 12 && rangeX >= rangeY * 4) {
 		if (gstate.isModeClear()) {
 			auto bound = [&](int a, int b) -> void {
 				DrawTriangleSlice<true>(v0, v1, v2, minX, minY, maxX, maxY, false, a, b);
 			};
-			ParallelRangeLoop(&g_threadManager, bound, 0, rangeX);
+			ParallelRangeLoop(&g_threadManager, bound, 0, rangeX, MIN_LINES_PER_THREAD);
 		} else {
 			auto bound = [&](int a, int b) -> void {
 				DrawTriangleSlice<false>(v0, v1, v2, minX, minY, maxX, maxY, false, a, b);
 			};
-			ParallelRangeLoop(&g_threadManager, bound, 0, rangeX);
+			ParallelRangeLoop(&g_threadManager, bound, 0, rangeX, MIN_LINES_PER_THREAD);
 		}
 	} else if (rangeY >= 12 && rangeX >= 12) {
 		if (gstate.isModeClear()) {
 			auto bound = [&](int a, int b) -> void {
 				DrawTriangleSlice<true>(v0, v1, v2, minX, minY, maxX, maxY, true, a, b);
 			};
-			ParallelRangeLoop(&g_threadManager, bound, 0, rangeY);
+			ParallelRangeLoop(&g_threadManager, bound, 0, rangeY, MIN_LINES_PER_THREAD);
 		} else {
 			auto bound = [&](int a, int b) -> void {
 				DrawTriangleSlice<false>(v0, v1, v2, minX, minY, maxX, maxY, true, a, b);
 			};
-			ParallelRangeLoop(&g_threadManager, bound, 0, rangeY);
+			ParallelRangeLoop(&g_threadManager, bound, 0, rangeY, MIN_LINES_PER_THREAD);
 		}
 	} else {
 		if (gstate.isModeClear()) {

GPU/Vulkan/TextureScalerVulkan.cpp

@@ -42,21 +42,23 @@ u32 TextureScalerVulkan::Get8888Format() {
 }
 
 void TextureScalerVulkan::ConvertTo8888(u32 format, u32* source, u32* &dest, int width, int height) {
+	const int MIN_LINES_PER_THREAD = 4;
+
 	switch (format) {
 	case VULKAN_8888_FORMAT:
 		dest = source; // already fine
 		break;
 
 	case VULKAN_4444_FORMAT:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert4444_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert4444_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 		break;
 
 	case VULKAN_565_FORMAT:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert565_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert565_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 		break;
 
 	case VULKAN_1555_FORMAT:
-		ParallelRangeLoop(&g_threadManager, std::bind(&convert5551_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height);
+		ParallelRangeLoop(&g_threadManager, std::bind(&convert5551_dx9, (u16*)source, dest, width, std::placeholders::_1, std::placeholders::_2), 0, height, MIN_LINES_PER_THREAD);
 		break;
 
 	default:

unittest/TestThreadManager.cpp

@@ -31,22 +31,26 @@ bool TestMailbox() {
 
 void rangeFunc(int lower, int upper) {
 	sleep_ms(30);
-	printf("range %d-%d (thread %d)\n", lower, upper, GetCurrentThreadIdForDebug());
+	printf(" - range %d-%d (thread %d)\n", lower, upper, GetCurrentThreadIdForDebug());
 }
 
 // This always passes unless something is badly broken, the interesting thing is the
 // logged output.
 bool TestParallelLoop(ThreadManager *threadMan) {
-	printf("tester thread ID: %d", GetCurrentThreadIdForDebug());
+	printf("tester thread ID: %d\n", GetCurrentThreadIdForDebug());
 
+	printf("waitable test\n");
 	WaitableCounter *waitable = ParallelRangeLoopWaitable(threadMan, rangeFunc, 0, 7, 1);
 	// Can do stuff here if we like.
 	waitable->WaitAndRelease();
 	// Now it's done.
 
-	ParallelRangeLoop(threadMan, rangeFunc, 0, 65);
-
-	// Try a few synchronous loops (can be slightly more efficient) with various ranges.
+	// Try a loop with stragglers.
+	printf("blocking test #1\n");
+	ParallelRangeLoop(threadMan, rangeFunc, 0, 65, 1);
+	// Try a loop with a relatively large minimum size.
+	printf("blocking test #2\n");
+	ParallelRangeLoop(threadMan, rangeFunc, 0, 100, 40);
 	return true;
 }