In skia_test.cc, atomics -> mutex.
These guys are not heavily contended nor speed critical. No need for atomics,
plus this makes tsan stop complaining (correctly) about reading fNextIndex
unsafely in onEnd.
I took a look at failCount/fFailCount, which I think is safely atomic and quite
conveniently so: It's never read until all the threads which could possibly
increment it have terminated (except for the one where it was created,
obviously). We could guard it with a mutex too, but maybe we can let this one
slide.
BUG=
R=bungeman@google.com
Author: mtklein@google.com
Review URL: https://codereview.chromium.org/25357002
git-svn-id: http://skia.googlecode.com/svn/trunk@11561 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/tests/skia_test.cpp b/tests/skia_test.cpp
index a93b307..620cdac 100644
--- a/tests/skia_test.cpp
+++ b/tests/skia_test.cpp
@@ -84,20 +84,23 @@
protected:
virtual void onStart(Test* test) {
- const int index = sk_atomic_inc(&fNextIndex)+1;
- const int pending = sk_atomic_inc(&fPending)+1;
- SkDebugf("[%3d/%3d] (%d) %s\n", index, fTotal, pending, test->getName());
+ SkAutoMutexAcquire lock(fStartEndMutex);
+ fNextIndex++;
+ fPending++;
+ SkDebugf("[%3d/%3d] (%d) %s\n", fNextIndex, fTotal, fPending, test->getName());
}
+
virtual void onReportFailed(const SkString& desc) {
SkDebugf("\tFAILED: %s\n", desc.c_str());
}
virtual void onEnd(Test* test) {
+ SkAutoMutexAcquire lock(fStartEndMutex);
if (!test->passed()) {
SkDebugf("---- %s FAILED\n", test->getName());
}
- sk_atomic_dec(&fPending);
+ fPending--;
if (fNextIndex == fTotal) {
// Just waiting on straggler tests. Shame them by printing their name and runtime.
SkDebugf(" (%d) %5.1fs %s\n",
@@ -106,8 +109,11 @@
}
private:
+ SkMutex fStartEndMutex; // Guards fNextIndex and fPending.
int32_t fNextIndex;
int32_t fPending;
+
+ // Once the tests get going, these are logically const.
int fTotal;
bool fAllowExtendedTest;
bool fAllowThreaded;