add initial support ofr multi-threaded execution

tools/skpdiff/SkDiffContext.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBitmap.h"
 #include "SkImageDecoder.h"
 #include "SkOSFile.h"
+#include "SkRunnable.h"
 #include "SkStream.h"
 #include "SkTDict.h"
+#include "SkThreadPool.h"
 
 #include "SkDiffContext.h"
 #include "SkImageDiffer.h"
 #include "skpdiff_util.h"
 
 // Truncates the number of points of interests in JSON output to not freeze the parser
 static const int kMaxPOI = 100;
 
 SkDiffContext::SkDiffContext() {
     fRecords = NULL;
     fDiffers = NULL;
     fDifferCount = 0;
+    fThreadCount = SkThreadPool::kThreadPerCore;
 }
 
 SkDiffContext::~SkDiffContext() {
     // Delete the record linked list
     DiffRecord* currentRecord = fRecords;
     while (NULL != currentRecord) {
         DiffRecord* nextRecord = currentRecord->fNext;
         SkDELETE(currentRecord);
         currentRecord = nextRecord;
     }
@@ skipping 51 matching lines @@
 
             int poiCount = differ->getPointsOfInterestCount(diffID);
             SkIPoint* poi = differ->getPointsOfInterest(diffID);
             diffData.fPointsOfInterest.append(poiCount, poi);
 
             // Because we are doing everything synchronously for now, we are done with the diff
             // after reading it.
             differ->deleteDiff(diffID);
         }
     }
+
+    // if we get a difference and we want the alpha mask then compute it here.
 }
 
+class SkThreadedDiff : public SkRunnable {
+public:
+    SkThreadedDiff() : fDiffContext(NULL) { }
+
+    void setup(SkDiffContext* diffContext, const SkString& baselinePath, const SkString& testPath) {
+        fDiffContext = diffContext;
+        fBaselinePath = baselinePath;
+        fTestPath = testPath;
+    }
+
+    virtual void run() SK_OVERRIDE {
+        fDiffContext->addDiff(fBaselinePath.c_str(), fTestPath.c_str());

[mtklein, 2013/10/23 02:47:12]

I'm crashing all over the place when I run skpdiff with more than one thread.

Are you certain that addDiff is threadsafe?  It doesn't look like any of the
code it calls in skpdiff/ is itself threadsafe.  We'd need to at least guard
fRecords and we definitely can't share fDiffers like we're doing here (maybe we
can just put them on the stack?  thread-local at worst?).

I'd strongly suggest setting the default --threads to 0 until we can get this
cleaned up.

+    }
+
+private:
+    SkDiffContext* fDiffContext;
+    SkString fBaselinePath;
+    SkString fTestPath;
+};
 
 void SkDiffContext::diffDirectories(const char baselinePath[], const char testPath[]) {
     // Get the files in the baseline, we will then look for those inside the test path
     SkTArray<SkString> baselineEntries;
     if (!get_directory(baselinePath, &baselineEntries)) {
         SkDebugf("Unable to open path \"%s\"\n", baselinePath);
         return;
     }
 
-    for (int baselineIndex = 0; baselineIndex < baselineEntries.count(); baselineIndex++) {
-        SkDebugf("[%i/%i] ", baselineIndex, baselineEntries.count());
-        const char* baseFilename = baselineEntries[baselineIndex].c_str();
+    SkThreadPool threadPool(fThreadCount);
+    SkTArray<SkThreadedDiff> runnableDiffs;
+    runnableDiffs.reset(baselineEntries.count());
+
+    for (int x = 0; x < baselineEntries.count(); x++) {
+        const char* baseFilename = baselineEntries[x].c_str();
 
         // Find the real location of each file to compare
         SkString baselineFile = SkOSPath::SkPathJoin(baselinePath, baseFilename);
         SkString testFile = SkOSPath::SkPathJoin(testPath, baseFilename);
 
         // Check that the test file exists and is a file
         if (sk_exists(testFile.c_str()) && !sk_isdir(testFile.c_str())) {
             // Queue up the comparison with the differ
-            this->addDiff(baselineFile.c_str(), testFile.c_str());
+            runnableDiffs[x].setup(this, baselineFile, testFile);
+            threadPool.add(&runnableDiffs[x]);
         } else {
             SkDebugf("Baseline file \"%s\" has no corresponding test file\n", baselineFile.c_str());
         }
     }
+
+    threadPool.wait();

[mtklein, 2013/10/23 02:47:12]

These wait() calls are harmless, but you can cut them out if you want.  The
threadpool destructor will wait too.

 }
 
 
 void SkDiffContext::diffPatterns(const char baselinePattern[], const char testPattern[]) {
     // Get the files in the baseline and test patterns. Because they are in sorted order, it's easy
     // to find corresponding images by matching entry indices.
 
     SkTArray<SkString> baselineEntries;
     if (!glob_files(baselinePattern, &baselineEntries)) {
         SkDebugf("Unable to get pattern \"%s\"\n", baselinePattern);
         return;
     }
 
     SkTArray<SkString> testEntries;
     if (!glob_files(testPattern, &testEntries)) {
         SkDebugf("Unable to get pattern \"%s\"\n", testPattern);
         return;
     }
 
     if (baselineEntries.count() != testEntries.count()) {
         SkDebugf("Baseline and test patterns do not yield corresponding number of files\n");
         return;
     }
 
-    for (int entryIndex = 0; entryIndex < baselineEntries.count(); entryIndex++) {
-        SkDebugf("[%i/%i] ", entryIndex, baselineEntries.count());
-        const char* baselineFilename = baselineEntries[entryIndex].c_str();
-        const char* testFilename     = testEntries    [entryIndex].c_str();
+    SkThreadPool threadPool(fThreadCount);
+    SkTArray<SkThreadedDiff> runnableDiffs;
+    runnableDiffs.reset(baselineEntries.count());
 
-        this->addDiff(baselineFilename, testFilename);
+    for (int x = 0; x < baselineEntries.count(); x++) {
+        runnableDiffs[x].setup(this, baselineEntries[x], testEntries[x]);
+        threadPool.add(&runnableDiffs[x]);
     }
+
+    threadPool.wait();
 }
 
 void SkDiffContext::outputRecords(SkWStream& stream, bool useJSONP) {
     DiffRecord* currentRecord = fRecords;
     if (useJSONP) {
         stream.writeText("var SkPDiffRecords = {\n");
     } else {
         stream.writeText("{\n");
     }
     stream.writeText("    \"records\": [\n");
     while (NULL != currentRecord) {
         stream.writeText("        {\n");
 
+            SkString differenceAbsPath = get_absolute_path(currentRecord->fDifferencePath);
             SkString baselineAbsPath = get_absolute_path(currentRecord->fBaselinePath);
             SkString testAbsPath = get_absolute_path(currentRecord->fTestPath);
 
             // strip off directory structure and find the common part of the filename
             SkString baseName = SkOSPath::SkBasename(baselineAbsPath.c_str());
             SkString testName = SkOSPath::SkBasename(testAbsPath.c_str());
             for (size_t x = 0; x < baseName.size(); ++x) {
                 if (baseName[x] != testName[x]) {
                     baseName.insertUnichar(x, '\n');
                     break;
                 }
             }
 
             stream.writeText("            \"commonName\": \"");
             stream.writeText(baseName.c_str());
             stream.writeText("\",\n");
 
+            stream.writeText("            \"differencePath\": \"");
+            stream.writeText(differenceAbsPath.c_str());
+            stream.writeText("\",\n");
+
             stream.writeText("            \"baselinePath\": \"");
             stream.writeText(baselineAbsPath.c_str());
             stream.writeText("\",\n");
 
             stream.writeText("            \"testPath\": \"");
             stream.writeText(testAbsPath.c_str());
             stream.writeText("\",\n");
 
             stream.writeText("            \"diffs\": [\n");
             for (int diffIndex = 0; diffIndex < currentRecord->fDiffs.count(); diffIndex++) {
@@ skipping 104 matching lines @@
         for (int i = 0; i < cntColumns; i++) {
             SkString str;
             str.printf(", %f", values[i]);
             stream.writeText(str.c_str());
         }
         stream.writeText("\n");
 
         currentRecord = currentRecord->fNext;
     }
 }