Add --threads to tests binary, to run non-GPU tests on multiple cores.

On my quad-core laptop I can get about a 3x speedup:
  Debug,   --threads 0  40.99s
  Debug,   --threads 8  14.39s
  Release, --threads 0   8.24s
  Release, --threads 8   2.80s

I also removed some unused Test.{h,cpp} APIs and refactored a little to make
things thread-safer.

BUG=
R=borenet@google.com, djsollen@google.com, scroggo@google.com, reed@google.com

Author: mtklein@google.com

Review URL: https://chromiumcodereview.appspot.com/13855007

git-svn-id: http://skia.googlecode.com/svn/trunk@8763 2bbb7eff-a529-9590-31e7-b0007b416f81

4 files changed, 111 insertions, 78 deletions

diff --git a/tests/GrContextFactoryTest.cpp b/tests/GrContextFactoryTest.cpp
index 80f1418057..02b8c287ff 100644
--- a/tests/GrContextFactoryTest.cpp
+++ b/tests/GrContextFactoryTest.cpp
@@ -11,26 +11,28 @@
 #if SK_SUPPORT_GPU
 #include "GrContextFactory.h"
 
-static void test_context_factory(skiatest::Reporter* reporter) {
-    GrContextFactory contextFactory;
+static void test_context_factory(skiatest::Reporter* reporter,
+                                 GrContextFactory* contextFactory) {
+    // Reset in case some other test has been using it first.
+    contextFactory->destroyContexts();
 
     // Before we ask for a context, we expect the GL context to not be there.
     REPORTER_ASSERT(reporter,
-                    NULL == contextFactory.getGLContext(GrContextFactory::kNative_GLContextType));
+                    NULL == contextFactory->getGLContext(GrContextFactory::kNative_GLContextType));
 
     // After we ask for a context, we expect that the GL context to be there.
-    contextFactory.get(GrContextFactory::kNative_GLContextType);
+    contextFactory->get(GrContextFactory::kNative_GLContextType);
     REPORTER_ASSERT(reporter,
-                    contextFactory.getGLContext(GrContextFactory::kNative_GLContextType) != NULL);
+                    contextFactory->getGLContext(GrContextFactory::kNative_GLContextType) != NULL);
 
     // If we did not ask for a context with the particular GL context, we would
     // expect the particular GL context to not be there.
     REPORTER_ASSERT(reporter,
-                    NULL == contextFactory.getGLContext(GrContextFactory::kNull_GLContextType));
+                    NULL == contextFactory->getGLContext(GrContextFactory::kNull_GLContextType));
 }
 
 
 #include "TestClassDef.h"
-DEFINE_TESTCLASS("GrContextFactory", GrContextFactoryClass, test_context_factory);
+DEFINE_GPUTESTCLASS("GrContextFactory", GrContextFactoryClass, test_context_factory);
 
 #endif
diff --git a/tests/Test.cpp b/tests/Test.cpp
index 79d3aad581..408e14f691 100644
--- a/tests/Test.cpp
+++ b/tests/Test.cpp
@@ -7,7 +7,8 @@
  */
 #include "Test.h"
 
-#include "SkTLazy.h"
+#include "SkString.h"
+#include "SkTArray.h"
 
 #if SK_SUPPORT_GPU
 #include "GrContext.h"
@@ -20,45 +21,25 @@ SK_DEFINE_INST_COUNT(skiatest::Reporter)
 
 using namespace skiatest;
 
-Reporter::Reporter()
-    : fTestCount(0) {
-    this->resetReporting();
-}
-
-void Reporter::resetReporting() {
-    fCurrTest = NULL;
-    fTestCount = 0;
-    sk_bzero(fResultCount, sizeof(fResultCount));
+Reporter::Reporter() : fTestCount(0) {
 }
 
 void Reporter::startTest(Test* test) {
-    SkASSERT(NULL == fCurrTest);
-    fCurrTest = test;
+    this->bumpTestCount();
     this->onStart(test);
-    fTestCount += 1;
-    fCurrTestSuccess = true;    // we're optimistic
 }
 
 void Reporter::report(const char desc[], Result result) {
-    if (NULL == desc) {
-        desc = "<no description>";
-    }
-    this->onReport(desc, result);
-    fResultCount[result] += 1;
-    if (kFailed == result) {
-        fCurrTestSuccess = false;
-    }
+    this->onReport(desc ? desc : "<no description>", result);
 }
 
 void Reporter::endTest(Test* test) {
-    SkASSERT(test == fCurrTest);
     this->onEnd(test);
-    fCurrTest = NULL;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
-Test::Test() : fReporter(NULL) {}
+Test::Test() : fReporter(NULL), fPassed(true) {}
 
 Test::~Test() {
     SkSafeUnref(fReporter);
@@ -75,11 +56,41 @@ const char* Test::getName() {
     return fName.c_str();
 }
 
-bool Test::run() {
+namespace {
+    class LocalReporter : public Reporter {
+    public:
+        LocalReporter() {}
+
+        int failure_size() const { return fFailures.count(); }
+        const char* failure(int i) const { return fFailures[i].c_str(); }
+
+    protected:
+        void onReport(const char desc[], Result result) SK_OVERRIDE {
+            if (kFailed == result) {
+                fFailures.push_back().set(desc);
+            }
+        }
+
+    private:
+        SkTArray<SkString> fFailures;
+    };
+}  // namespace
+
+void Test::run() {
+    // Tell (likely shared) fReporter that this test has started.
     fReporter->startTest(this);
-    this->onRun(fReporter);
+
+    // Run the test into a LocalReporter so we know if it's passed or failed without interference
+    // from other tests that might share fReporter.
+    LocalReporter local;
+    this->onRun(&local);
+    fPassed = local.failure_size() == 0;
+
+    // Now tell fReporter about any failures and wrap up.
+    for (int i = 0; i < local.failure_size(); i++) {
+      fReporter->report(local.failure(i), Reporter::kFailed);
+    }
     fReporter->endTest(this);
-    return fReporter->getCurrSuccess();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/tests/Test.h b/tests/Test.h
index 6b0b90101e..1aa038748f 100644
--- a/tests/Test.h
+++ b/tests/Test.h
@@ -31,13 +31,8 @@ namespace skiatest {
             kLastResult = kFailed
         };
 
-        void resetReporting();
         void bumpTestCount() { sk_atomic_inc(&fTestCount); }
         int countTests() const { return fTestCount; }
-        int countResults(Result r) {
-            SkASSERT((unsigned)r <= kLastResult);
-            return fResultCount[r];
-        }
 
         void startTest(Test*);
         void report(const char testDesc[], Result);
@@ -45,16 +40,6 @@ namespace skiatest {
         virtual bool allowExtendedTest() const { return false; }
         virtual bool allowThreaded() const { return false; }
         // helpers for tests
-        void assertTrue(bool cond, const char desc[]) {
-            if (!cond) {
-                this->report(desc, kFailed);
-            }
-        }
-        void assertFalse(bool cond, const char desc[]) {
-            if (cond) {
-                this->report(desc, kFailed);
-            }
-        }
         void reportFailed(const char desc[]) {
             this->report(desc, kFailed);
         }
@@ -62,9 +47,6 @@ namespace skiatest {
             this->report(desc.c_str(), kFailed);
         }
 
-        bool getCurrSuccess() const {
-            return fCurrTestSuccess;
-        }
 
     protected:
         virtual void onStart(Test*) {}
@@ -72,10 +54,7 @@ namespace skiatest {
         virtual void onEnd(Test*) {}
 
     private:
-        Test* fCurrTest;
-        int fTestCount;
-        int fResultCount[kLastResult+1];
-        bool fCurrTestSuccess;
+        int32_t fTestCount;
 
         typedef SkRefCnt INHERITED;
     };
@@ -89,12 +68,15 @@ namespace skiatest {
         void setReporter(Reporter*);
 
         const char* getName();
-        bool run(); // returns true on success
+        void run();
+        bool passed() const { return fPassed; }
 
         static const SkString& GetTmpDir();
 
         static const SkString& GetResourcePath();
 
+        virtual bool isThreadsafe() const { return true; }
+
     protected:
         virtual void onGetName(SkString*) = 0;
         virtual void onRun(Reporter*) = 0;
@@ -102,6 +84,7 @@ namespace skiatest {
     private:
         Reporter*   fReporter;
         SkString    fName;
+        bool        fPassed;
     };
 
     class GpuTest : public Test{
@@ -109,6 +92,7 @@ namespace skiatest {
         GpuTest() : Test() {}
         static GrContextFactory* GetGrContextFactory();
         static void DestroyContexts();
+        virtual bool isThreadsafe() const { return false; }
     private:
     };
 
diff --git a/tests/skia_test.cpp b/tests/skia_test.cpp
index fd56cc71fb..6330b326a8 100644
--- a/tests/skia_test.cpp
+++ b/tests/skia_test.cpp
@@ -7,6 +7,10 @@
 
 #include "SkCommandLineFlags.h"
 #include "SkGraphics.h"
+#include "SkRunnable.h"
+#include "SkThreadPool.h"
+#include "SkTArray.h"
+#include "SkTemplates.h"
 #include "Test.h"
 #include "SkOSFile.h"
 
@@ -63,14 +67,13 @@ static const char* result2string(Reporter::Result result) {
 class DebugfReporter : public Reporter {
 public:
     DebugfReporter(bool allowExtendedTest, bool allowThreaded)
-        : fIndex(0)
+        : fNextIndex(0)
         , fTotal(0)
         , fAllowExtendedTest(allowExtendedTest)
         , fAllowThreaded(allowThreaded) {
     }
 
-    void setIndexOfTotal(int index, int total) {
-        fIndex = index;
+    void setTotal(int total) {
         fTotal = total;
     }
 
@@ -84,18 +87,22 @@ public:
 
 protected:
     virtual void onStart(Test* test) {
-        SkDebugf("[%d/%d] %s...\n", fIndex+1, fTotal, test->getName());
+        const int index = sk_atomic_inc(&fNextIndex);
+        SkDebugf("[%d/%d] %s...\n", index+1, fTotal, test->getName());
     }
     virtual void onReport(const char desc[], Reporter::Result result) {
         SkDebugf("\t%s: %s\n", result2string(result), desc);
     }
-    virtual void onEnd(Test*) {
-        if (!this->getCurrSuccess()) {
-            SkDebugf("---- FAILED\n");
+
+    virtual void onEnd(Test* test) {
+        if (!test->passed()) {
+          SkDebugf("---- FAILED\n");
         }
     }
+
 private:
-    int fIndex, fTotal;
+    int32_t fNextIndex;
+    int fTotal;
     bool fAllowExtendedTest;
     bool fAllowThreaded;
 };
@@ -132,8 +139,29 @@ DEFINE_string2(match, m, NULL, "substring of test name to run.");
 DEFINE_string2(tmpDir, t, NULL, "tmp directory for tests to use.");
 DEFINE_string2(resourcePath, i, NULL, "directory for test resources.");
 DEFINE_bool2(extendedTest, x, false, "run extended tests for pathOps.");
-DEFINE_bool2(threaded, z, false, "allow tests to use multiple threads.");
+DEFINE_bool2(threaded, z, false, "allow tests to use multiple threads internally.");
 DEFINE_bool2(verbose, v, false, "enable verbose output.");
+DEFINE_int32(threads, 8,
+             "If >0, run threadsafe tests on a threadpool with this many threads.");
+
+// Deletes self when run.
+class SkTestRunnable : public SkRunnable {
+public:
+  // Takes ownership of test.
+  SkTestRunnable(Test* test, int32_t* failCount) : fTest(test), fFailCount(failCount) {}
+
+  virtual void run() {
+      fTest->run();
+      if(!fTest->passed()) {
+          sk_atomic_inc(fFailCount);
+      }
+      SkDELETE(this);
+  }
+
+private:
+    SkAutoTDelete<Test> fTest;
+    int32_t* fFailCount;
+};
 
 int tool_main(int argc, char** argv);
 int tool_main(int argc, char** argv) {
@@ -179,28 +207,36 @@ int tool_main(int argc, char** argv) {
 
     DebugfReporter reporter(FLAGS_extendedTest, FLAGS_threaded);
     Iter iter(&reporter);
-    Test* test;
 
     const int count = Iter::Count();
-    int index = 0;
-    int failCount = 0;
+    reporter.setTotal(count);
+    int32_t failCount = 0;
     int skipCount = 0;
-    while ((test = iter.next()) != NULL) {
-        reporter.setIndexOfTotal(index, count);
+
+    SkAutoTDelete<SkThreadPool> threadpool(SkNEW_ARGS(SkThreadPool, (FLAGS_threads)));
+    SkTArray<Test*> unsafeTests;  // Always passes ownership to an SkTestRunnable
+    for (int i = 0; i < count; i++) {
+        SkAutoTDelete<Test> test(iter.next());
         if (!FLAGS_match.isEmpty() && !strstr(test->getName(), FLAGS_match[0])) {
             ++skipCount;
+        } else if (!test->isThreadsafe()) {
+            unsafeTests.push_back() = test.detach();
         } else {
-            if (!test->run()) {
-                ++failCount;
-            }
+            threadpool->add(SkNEW_ARGS(SkTestRunnable, (test.detach(), &failCount)));
         }
-        SkDELETE(test);
-        index += 1;
     }
 
+    // Run the tests that aren't threadsafe.
+    for (int i = 0; i < unsafeTests.count(); i++) {
+        SkNEW_ARGS(SkTestRunnable, (unsafeTests[i], &failCount))->run();
+    }
+
+    // Blocks until threaded tests finish.
+    threadpool.free();
+
     SkDebugf("Finished %d tests, %d failures, %d skipped.\n",
              count, failCount, skipCount);
-    int testCount = reporter.countTests();
+    const int testCount = reporter.countTests();
     if (FLAGS_verbose && testCount > 0) {
         SkDebugf("Ran %d Internal tests.\n", testCount);
     }