SkThreadPool ~~> SkTaskGroup
SkTaskGroup is like SkThreadPool except the threads stay in
one global pool. Each SkTaskGroup itself is tiny (4 bytes)
and its wait() method applies only to tasks add()ed to that
instance, not the whole thread pool.
This means we don't need to bring up new thread pools when
tests themselves want to use multithreading (e.g. pathops,
quilt). We just create a new SkTaskGroup and wait for that
to complete. This should be more efficient, and allow us
to expand where we use threads to really latency sensitive
places. E.g. we can probably now use these in nanobench
for CPU .skp rendering.
Now that all threads are sharing the same pool, I think we
can remove most of the custom mechanism pathops tests use
to control threading. They'll just ride on the global pool
with all other tests now.
This (temporarily?) removes the GPU multithreading feature
from DM, which we don't use.
On my desktop, DM runs a little faster (57s -> 55s) in
Debug, and a lot faster in Release (36s -> 24s). The bots
show speedups of similar proportions, cutting more than a
minute off the N4/Release and Win7/Debug runtimes.
BUG=skia:
Committed: https://skia.googlesource.com/skia/+/9c7207b5dc71dc5a96a2eb107d401133333d5b6f
R=caryclark@google.com, bsalomon@google.com, bungeman@google.com, mtklein@google.com, reed@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/531653002
diff --git a/tests/OnceTest.cpp b/tests/OnceTest.cpp
index 389d257..192abaa 100644
--- a/tests/OnceTest.cpp
+++ b/tests/OnceTest.cpp
@@ -6,7 +6,7 @@
*/
#include "SkOnce.h"
-#include "SkThreadPool.h"
+#include "SkTaskGroup.h"
#include "Test.h"
static void add_five(int* x) {
@@ -42,7 +42,7 @@
};
DEF_TEST(SkOnce_Multithreaded, r) {
- const int kTasks = 16, kThreads = 4;
+ const int kTasks = 16;
// Make a bunch of tasks that will race to be the first to add six to x.
Racer racers[kTasks];
@@ -54,11 +54,11 @@
}
// Let them race.
- SkThreadPool pool(kThreads);
+ SkTaskGroup tg;
for (int i = 0; i < kTasks; i++) {
- pool.add(&racers[i]);
+ tg.add(&racers[i]);
}
- pool.wait();
+ tg.wait();
// Only one should have done the +=.
REPORTER_ASSERT(r, 6 == x);
diff --git a/tests/PathOpsExtendedTest.cpp b/tests/PathOpsExtendedTest.cpp
index 05d0004..f90b9fe 100644
--- a/tests/PathOpsExtendedTest.cpp
+++ b/tests/PathOpsExtendedTest.cpp
@@ -14,8 +14,8 @@
#include "SkPaint.h"
#include "SkRTConf.h"
#include "SkStream.h"
+#include "SkTaskGroup.h"
#include "SkThread.h"
-#include "SkThreadPool.h"
#ifdef SK_BUILD_FOR_MAC
#include <sys/sysctl.h>
@@ -542,7 +542,7 @@
SK_DECLARE_STATIC_MUTEX(gMutex);
-int initializeTests(skiatest::Reporter* reporter, const char* test) {
+void initializeTests(skiatest::Reporter* reporter, const char* test) {
#if 0 // doesn't work yet
SK_CONF_SET("images.jpeg.suppressDecoderWarnings", true);
SK_CONF_SET("images.png.suppressDecoderWarnings", true);
@@ -566,7 +566,6 @@
}
}
}
- return reporter->allowThreaded() ? SkThreadPool::kThreadPerCore : 1;
}
void outputProgress(char* ramStr, const char* pathStr, SkPath::FillType pathFillType) {
diff --git a/tests/PathOpsExtendedTest.h b/tests/PathOpsExtendedTest.h
index 5f3413c..a854410 100644
--- a/tests/PathOpsExtendedTest.h
+++ b/tests/PathOpsExtendedTest.h
@@ -36,7 +36,7 @@
const char* pathStr);
extern bool testSimplify(skiatest::Reporter* reporter, const SkPath& path, const char* filename);
-int initializeTests(skiatest::Reporter* reporter, const char* testName);
+void initializeTests(skiatest::Reporter* reporter, const char* testName);
void outputProgress(char* ramStr, const char* pathStr, SkPath::FillType );
void outputProgress(char* ramStr, const char* pathStr, SkPathOp op);
diff --git a/tests/PathOpsOpCubicThreadedTest.cpp b/tests/PathOpsOpCubicThreadedTest.cpp
index 889ade0..751ccc5 100644
--- a/tests/PathOpsOpCubicThreadedTest.cpp
+++ b/tests/PathOpsOpCubicThreadedTest.cpp
@@ -67,8 +67,8 @@
}
DEF_TEST(PathOpsOpCubicsThreaded, reporter) {
- int threadCount = initializeTests(reporter, "cubicOp");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "cubicOp");
+ PathOpsThreadedTestRunner testRunner(reporter);
for (int a = 0; a < 6; ++a) { // outermost
for (int b = a + 1; b < 7; ++b) {
for (int c = 0 ; c < 6; ++c) {
diff --git a/tests/PathOpsOpLoopThreadedTest.cpp b/tests/PathOpsOpLoopThreadedTest.cpp
index 71efff3..3f316d1 100755
--- a/tests/PathOpsOpLoopThreadedTest.cpp
+++ b/tests/PathOpsOpLoopThreadedTest.cpp
@@ -62,8 +62,8 @@
}
DEF_TEST(PathOpsOpLoopsThreaded, reporter) {
- int threadCount = initializeTests(reporter, "cubicOp");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "cubicOp");
+ PathOpsThreadedTestRunner testRunner(reporter);
for (int a = 0; a < 6; ++a) { // outermost
for (int b = a + 1; b < 7; ++b) {
for (int c = 0 ; c < 6; ++c) {
@@ -81,7 +81,7 @@
}
DEF_TEST(PathOpsOpLoops, reporter) {
- (void) initializeTests(reporter, "cubicOp");
+ initializeTests(reporter, "cubicOp");
PathOpsThreadState state;
state.fReporter = reporter;
SkBitmap bitmap;
diff --git a/tests/PathOpsOpRectThreadedTest.cpp b/tests/PathOpsOpRectThreadedTest.cpp
index 3d07d74..1b6e4e8 100644
--- a/tests/PathOpsOpRectThreadedTest.cpp
+++ b/tests/PathOpsOpRectThreadedTest.cpp
@@ -74,8 +74,8 @@
}
DEF_TEST(PathOpsRectsThreaded, reporter) {
- int threadCount = initializeTests(reporter, "testOp");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "testOp");
+ PathOpsThreadedTestRunner testRunner(reporter);
for (int a = 0; a < 6; ++a) { // outermost
for (int b = a + 1; b < 7; ++b) {
for (int c = 0 ; c < 6; ++c) {
diff --git a/tests/PathOpsQuadLineIntersectionThreadedTest.cpp b/tests/PathOpsQuadLineIntersectionThreadedTest.cpp
index fd7581f..7e33b7b 100644
--- a/tests/PathOpsQuadLineIntersectionThreadedTest.cpp
+++ b/tests/PathOpsQuadLineIntersectionThreadedTest.cpp
@@ -111,8 +111,8 @@
}
DEF_TEST(PathOpsQuadLineIntersectionThreaded, reporter) {
- int threadCount = initializeTests(reporter, "testQuadLineIntersect");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "testQuadLineIntersect");
+ PathOpsThreadedTestRunner testRunner(reporter);
for (int a = 0; a < 16; ++a) {
for (int b = 0 ; b < 16; ++b) {
for (int c = 0 ; c < 16; ++c) {
diff --git a/tests/PathOpsSimplifyDegenerateThreadedTest.cpp b/tests/PathOpsSimplifyDegenerateThreadedTest.cpp
index 5cd3c35..8e8c58b 100755
--- a/tests/PathOpsSimplifyDegenerateThreadedTest.cpp
+++ b/tests/PathOpsSimplifyDegenerateThreadedTest.cpp
@@ -68,8 +68,8 @@
}
DEF_TEST(PathOpsSimplifyDegeneratesThreaded, reporter) {
- int threadCount = initializeTests(reporter, "testDegenerates");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "testDegenerates");
+ PathOpsThreadedTestRunner testRunner(reporter);
for (int a = 0; a < 16; ++a) {
int ax = a & 0x03;
int ay = a >> 2;
diff --git a/tests/PathOpsSimplifyQuadThreadedTest.cpp b/tests/PathOpsSimplifyQuadThreadedTest.cpp
index dbbec3e..3c92cca 100644
--- a/tests/PathOpsSimplifyQuadThreadedTest.cpp
+++ b/tests/PathOpsSimplifyQuadThreadedTest.cpp
@@ -74,8 +74,8 @@
}
DEF_TEST(PathOpsSimplifyQuadsThreaded, reporter) {
- int threadCount = initializeTests(reporter, "testQuads");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "testQuads");
+ PathOpsThreadedTestRunner testRunner(reporter);
int a = 0;
for (; a < 16; ++a) {
for (int b = a ; b < 16; ++b) {
diff --git a/tests/PathOpsSimplifyQuadralateralsThreadedTest.cpp b/tests/PathOpsSimplifyQuadralateralsThreadedTest.cpp
index afa9200..f8e9a6e 100755
--- a/tests/PathOpsSimplifyQuadralateralsThreadedTest.cpp
+++ b/tests/PathOpsSimplifyQuadralateralsThreadedTest.cpp
@@ -76,8 +76,8 @@
}
DEF_TEST(PathOpsSimplifyQuadralateralsThreaded, reporter) {
- int threadCount = initializeTests(reporter, "testQuadralaterals");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "testQuadralaterals");
+ PathOpsThreadedTestRunner testRunner(reporter);
for (int a = 0; a < 16; ++a) {
for (int b = a ; b < 16; ++b) {
for (int c = b ; c < 16; ++c) {
diff --git a/tests/PathOpsSimplifyRectThreadedTest.cpp b/tests/PathOpsSimplifyRectThreadedTest.cpp
index 9e6a5ea..52a78ec 100644
--- a/tests/PathOpsSimplifyRectThreadedTest.cpp
+++ b/tests/PathOpsSimplifyRectThreadedTest.cpp
@@ -187,8 +187,8 @@
}
DEF_TEST(PathOpsSimplifyRectsThreaded, reporter) {
- int threadCount = initializeTests(reporter, "testLine");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "testLine");
+ PathOpsThreadedTestRunner testRunner(reporter);
for (int a = 0; a < 8; ++a) { // outermost
for (int b = a ; b < 8; ++b) {
for (int c = b ; c < 8; ++c) {
diff --git a/tests/PathOpsSimplifyTrianglesThreadedTest.cpp b/tests/PathOpsSimplifyTrianglesThreadedTest.cpp
index b5d6508..ee0ca2b 100755
--- a/tests/PathOpsSimplifyTrianglesThreadedTest.cpp
+++ b/tests/PathOpsSimplifyTrianglesThreadedTest.cpp
@@ -73,8 +73,8 @@
}
DEF_TEST(PathOpsSimplifyTrianglesThreaded, reporter) {
- int threadCount = initializeTests(reporter, "testTriangles");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "testTriangles");
+ PathOpsThreadedTestRunner testRunner(reporter);
for (int a = 0; a < 15; ++a) {
int ax = a & 0x03;
int ay = a >> 2;
diff --git a/tests/PathOpsSkpClipTest.cpp b/tests/PathOpsSkpClipTest.cpp
index cdc3c1f..0769141 100755
--- a/tests/PathOpsSkpClipTest.cpp
+++ b/tests/PathOpsSkpClipTest.cpp
@@ -20,8 +20,8 @@
#include "SkString.h"
#include "SkTArray.h"
#include "SkTDArray.h"
+#include "SkTaskGroup.h"
#include "SkTemplates.h"
-#include "SkThreadPool.h"
#include "SkTime.h"
__SK_FORCE_IMAGE_DECODER_LINKING;
@@ -90,14 +90,14 @@
}
if (filename) {
path.appendf("%s%s", PATH_SLASH, filename);
- if (!sk_exists(path.c_str())) {
+ if (!sk_exists(path.c_str())) {
SkDebugf("could not read %s\n", path.c_str());
return SkString();
}
}
return path;
}
-
+
static void make_recursive_dir(const SkString& path) {
if (sk_exists(path.c_str())) {
return;
@@ -129,7 +129,7 @@
make_recursive_dir(path);
return path;
}
-
+
static SkString get_sum_path(const char* dirName) {
SkString path;
SkASSERT(dirName);
@@ -166,12 +166,12 @@
fTestStep = kCompareBits;
fScale = 1;
}
-
+
void init(int dirNo, const SkString& filename) {
fDirNo = dirNo;
strcpy(fFilename, filename.c_str());
fTestStep = kCompareBits;
- fScale = 1;
+ fScale = 1;
}
SkString status() {
@@ -204,7 +204,7 @@
}
void testOne();
-
+
char fFilename[kMaxLength];
TestStep fTestStep;
int fDirNo;
@@ -245,13 +245,8 @@
};
struct TestRunner {
- TestRunner(int threadCount)
- : fNumThreads(threadCount) {
- }
-
~TestRunner();
void render();
- int fNumThreads;
SkTDArray<class TestRunnable*> fRunnables;
};
@@ -300,9 +295,9 @@
}
void TestRunner::render() {
- SkThreadPool pool(fNumThreads);
+ SkTaskGroup tg;
for (int index = 0; index < fRunnables.count(); ++ index) {
- pool.add(fRunnables[index]);
+ tg.add(fRunnables[index]);
}
}
@@ -531,18 +526,12 @@
DEFINE_string2(skp, s, NULL, "skp to test");
DEFINE_bool2(single, z, false, "run tests on a single thread internally.");
DEFINE_int32(testIndex, 0, "override local test index (PathOpsSkpClipOneOff only).");
-DEFINE_int32(threads, SkThreadPool::kThreadPerCore,
- "Run threadsafe tests on a threadpool with this many threads.");
DEFINE_bool2(verbose, v, false, "enable verbose output.");
static bool verbose() {
return FLAGS_verbose;
}
-static int getThreadCount() {
- return FLAGS_single ? 1 : FLAGS_threads;
-}
-
class Dirs {
public:
Dirs() {
@@ -616,7 +605,7 @@
}
return NULL;
}
-
+
void set(const SkCommandLineFlags::StringArray& names) {
fNames = &names;
}
@@ -626,7 +615,7 @@
const SkCommandLineFlags::StringArray* fNames;
} gNames;
-static bool buildTestDir(int dirNo, int firstDirNo,
+static bool buildTestDir(int dirNo, int firstDirNo,
SkTDArray<TestResult>* tests, SkTDArray<SortByName*>* sorted) {
SkString dirName = get_out_path(dirNo, outStatusDir);
if (!dirName.size()) {
@@ -792,8 +781,7 @@
}
}
}
- int threadCount = getThreadCount();
- TestRunner testRunner(threadCount);
+ TestRunner testRunner;
for (int index = 0; index < state.fPixelWorst.count(); ++index) {
const TestResult& result = state.fPixelWorst[index];
SkString filename(result.fFilename);
@@ -865,8 +853,7 @@
DEF_TEST(PathOpsSkpClipThreaded) {
gDirs.setDefault();
initTest();
- int threadCount = getThreadCount();
- TestRunner testRunner(threadCount);
+ TestRunner testRunner;
int dirNo;
gDirs.reset();
while ((dirNo = gDirs.next()) > 0) {
@@ -889,7 +876,7 @@
}
encodeFound(state);
}
-
+
static bool buildTests(SkTDArray<TestResult>* tests, SkTDArray<SortByName*>* sorted) {
int firstDirNo = gDirs.first();
int dirNo;
@@ -912,8 +899,7 @@
if (!buildTests(tests.get(), sorted.get())) {
return;
}
- int threadCount = getThreadCount();
- TestRunner testRunner(threadCount);
+ TestRunner testRunner;
int dirNo;
gDirs.reset();
while ((dirNo = gDirs.next()) > 0) {
diff --git a/tests/PathOpsThreadedCommon.cpp b/tests/PathOpsThreadedCommon.cpp
index ac4cd6b..0adde91 100644
--- a/tests/PathOpsThreadedCommon.cpp
+++ b/tests/PathOpsThreadedCommon.cpp
@@ -7,7 +7,7 @@
#include "PathOpsExtendedTest.h"
#include "PathOpsThreadedCommon.h"
-#include "SkThreadPool.h"
+#include "SkTaskGroup.h"
PathOpsThreadedTestRunner::~PathOpsThreadedTestRunner() {
for (int index = 0; index < fRunnables.count(); index++) {
@@ -16,8 +16,8 @@
}
void PathOpsThreadedTestRunner::render() {
- SkThreadPool pool(fNumThreads);
+ SkTaskGroup tg;
for (int index = 0; index < fRunnables.count(); ++ index) {
- pool.add(fRunnables[index]);
+ tg.add(fRunnables[index]);
}
}
diff --git a/tests/PathOpsThreadedCommon.h b/tests/PathOpsThreadedCommon.h
index a638cd2..124921e 100644
--- a/tests/PathOpsThreadedCommon.h
+++ b/tests/PathOpsThreadedCommon.h
@@ -33,17 +33,13 @@
class PathOpsThreadedTestRunner {
public:
- PathOpsThreadedTestRunner(skiatest::Reporter* reporter, int threadCount)
- : fNumThreads(threadCount)
- , fReporter(reporter) {
- }
+ PathOpsThreadedTestRunner(skiatest::Reporter* reporter) : fReporter(reporter) {}
~PathOpsThreadedTestRunner();
void render();
public:
- int fNumThreads;
SkTDArray<PathOpsThreadedRunnable*> fRunnables;
skiatest::Reporter* fReporter;
};
diff --git a/tests/PathOpsTightBoundsTest.cpp b/tests/PathOpsTightBoundsTest.cpp
index 09f9622..cea3752 100644
--- a/tests/PathOpsTightBoundsTest.cpp
+++ b/tests/PathOpsTightBoundsTest.cpp
@@ -35,8 +35,8 @@
}
DEF_TEST(PathOpsTightBoundsLines, reporter) {
- int threadCount = initializeTests(reporter, "tightBoundsLines");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "tightBoundsLines");
+ PathOpsThreadedTestRunner testRunner(reporter);
int outerCount = reporter->allowExtendedTest() ? 100 : 1;
for (int index = 0; index < outerCount; ++index) {
for (int idx2 = 0; idx2 < 10; ++idx2) {
@@ -110,8 +110,8 @@
}
DEF_TEST(PathOpsTightBoundsQuads, reporter) {
- int threadCount = initializeTests(reporter, "tightBoundsQuads");
- PathOpsThreadedTestRunner testRunner(reporter, threadCount);
+ initializeTests(reporter, "tightBoundsQuads");
+ PathOpsThreadedTestRunner testRunner(reporter);
int outerCount = reporter->allowExtendedTest() ? 100 : 1;
for (int index = 0; index < outerCount; ++index) {
for (int idx2 = 0; idx2 < 10; ++idx2) {
diff --git a/tests/SkpSkGrTest.cpp b/tests/SkpSkGrTest.cpp
index c1883a9..c882654 100644
--- a/tests/SkpSkGrTest.cpp
+++ b/tests/SkpSkGrTest.cpp
@@ -23,7 +23,7 @@
#include "SkString.h"
#include "SkTArray.h"
#include "SkTDArray.h"
-#include "SkThreadPool.h"
+#include "SkTaskGroup.h"
#include "SkTime.h"
#include "Test.h"
@@ -125,14 +125,12 @@
};
struct SkpSkGrThreadedTestRunner {
- SkpSkGrThreadedTestRunner(skiatest::Reporter* reporter, int threadCount)
- : fNumThreads(threadCount)
- , fReporter(reporter) {
+ SkpSkGrThreadedTestRunner(skiatest::Reporter* reporter)
+ : fReporter(reporter) {
}
~SkpSkGrThreadedTestRunner();
void render();
- int fNumThreads;
SkTDArray<SkpSkGrThreadedRunnable*> fRunnables;
skiatest::Reporter* fReporter;
};
@@ -164,9 +162,9 @@
}
void SkpSkGrThreadedTestRunner::render() {
- SkThreadPool pool(fNumThreads);
+ SkTaskGroup tg;
for (int index = 0; index < fRunnables.count(); ++ index) {
- pool.add(fRunnables[index]);
+ tg.add(fRunnables[index]);
}
}
@@ -676,8 +674,7 @@
if (!initTest()) {
return;
}
- int threadCount = reporter->allowThreaded() ? 3 : 1;
- SkpSkGrThreadedTestRunner testRunner(reporter, threadCount);
+ SkpSkGrThreadedTestRunner testRunner(reporter);
for (int dirIndex = 1; dirIndex <= 100; ++dirIndex) {
SkString pictDir = make_in_dir_name(dirIndex);
if (pictDir.size() == 0) {
diff --git a/tests/Test.cpp b/tests/Test.cpp
index 20afd45..d0147e1 100644
--- a/tests/Test.cpp
+++ b/tests/Test.cpp
@@ -75,10 +75,6 @@
return fReporter->allowExtendedTest();
}
- virtual bool allowThreaded() const SK_OVERRIDE {
- return fReporter->allowThreaded();
- }
-
virtual void bumpTestCount() SK_OVERRIDE {
fReporter->bumpTestCount();
}
diff --git a/tests/Test.h b/tests/Test.h
index 72b0bee..6c85b32 100644
--- a/tests/Test.h
+++ b/tests/Test.h
@@ -32,7 +32,6 @@
void endTest(Test*);
virtual bool allowExtendedTest() const { return false; }
- virtual bool allowThreaded() const { return false; }
virtual bool verbose() const { return false; }
virtual void bumpTestCount() { sk_atomic_inc(&fTestCount); }
diff --git a/tests/skia_test.cpp b/tests/skia_test.cpp
index 97ac29b..0058215 100644
--- a/tests/skia_test.cpp
+++ b/tests/skia_test.cpp
@@ -12,8 +12,8 @@
#include "SkGraphics.h"
#include "SkOSFile.h"
#include "SkTArray.h"
+#include "SkTaskGroup.h"
#include "SkTemplates.h"
-#include "SkThreadPool.h"
#include "SkTime.h"
#include "Test.h"
@@ -54,7 +54,6 @@
explicit DebugfReporter(int total) : fDone(0), fTotal(total) {}
virtual bool allowExtendedTest() const SK_OVERRIDE { return FLAGS_extendedTest; }
- virtual bool allowThreaded() const SK_OVERRIDE { return !FLAGS_single; }
virtual bool verbose() const SK_OVERRIDE { return FLAGS_veryVerbose; }
protected:
@@ -175,7 +174,8 @@
int32_t failCount = 0;
int skipCount = 0;
- SkThreadPool threadpool(FLAGS_threads);
+ SkTaskGroup::Enabler enabled(FLAGS_threads);
+ SkTaskGroup cpuTests;
SkTArray<Test*> gpuTests; // Always passes ownership to an SkTestRunnable
DebugfReporter reporter(toRun);
@@ -186,7 +186,7 @@
} else if (test->isGPUTest()) {
gpuTests.push_back() = test.detach();
} else {
- threadpool.add(SkNEW_ARGS(SkTestRunnable, (test.detach(), &failCount)));
+ cpuTests.add(SkNEW_ARGS(SkTestRunnable, (test.detach(), &failCount)));
}
}
@@ -204,7 +204,7 @@
}
// Block until threaded tests finish.
- threadpool.wait();
+ cpuTests.wait();
if (FLAGS_verbose) {
SkDebugf("\nFinished %d tests, %d failures, %d skipped. (%d internal tests)",