DM tweaks
- Don't print status updates for skipped tasks or count them as pending tasks.
- Refactor DMReporter a bit for better symmetry, be more explicit about
how we read atomics (that is, approximately) in printStatus() (née finish()).
- Remove mutex locking from printStatus().
BUG=skia:
R=halcanary@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/309483003
git-svn-id: http://skia.googlecode.com/svn/trunk@14977 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/dm/DM.cpp b/dm/DM.cpp
index c65e1e7..f71a614 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -182,10 +182,7 @@
}
}
-static void report_failures(const DM::Reporter& reporter) {
- SkTArray<SkString> failures;
- reporter.getFailures(&failures);
-
+static void report_failures(const SkTArray<SkString>& failures) {
if (failures.count() == 0) {
return;
}
@@ -194,6 +191,7 @@
for (int i = 0; i < failures.count(); i++) {
SkDebugf(" %s\n", failures[i].c_str());
}
+ SkDebugf("%d failures.\n", failures.count());
}
template <typename T, typename Registry>
@@ -208,7 +206,7 @@
int tool_main(int argc, char** argv);
int tool_main(int argc, char** argv) {
- SkGraphics::Init();
+ SkAutoGraphics ag;
SkCommandLineFlags::Parse(argc, argv);
#if SK_ENABLE_INST_COUNT
gPrintInstCount = FLAGS_leaks;
@@ -260,11 +258,11 @@
tasks.wait();
SkDebugf("\n");
- report_failures(reporter);
- SkGraphics::Term();
-
- return reporter.failed() > 0;
+ SkTArray<SkString> failures;
+ reporter.getFailures(&failures);
+ report_failures(failures);
+ return failures.count() > 0;
}
#if !defined(SK_BUILD_FOR_IOS) && !defined(SK_BUILD_FOR_NACL)
diff --git a/dm/DMReporter.cpp b/dm/DMReporter.cpp
index 4bb25d3..0bc77bd 100644
--- a/dm/DMReporter.cpp
+++ b/dm/DMReporter.cpp
@@ -1,5 +1,6 @@
#include "DMReporter.h"
+#include "SkDynamicAnnotations.h"
#include "SkCommandLineFlags.h"
#include "OverwriteLine.h"
@@ -8,18 +9,17 @@
namespace DM {
-void Reporter::finish(SkString name, SkMSec timeMs) {
- sk_atomic_inc(&fFinished);
-
+void Reporter::printStatus(SkString name, SkMSec timeMs) const {
if (FLAGS_quiet) {
return;
}
+ // It's okay if these are a little off---they're just for show---so we can read unprotectedly.
+ const int32_t failed = SK_ANNOTATE_UNPROTECTED_READ(fFailed);
+ const int32_t pending = SK_ANNOTATE_UNPROTECTED_READ(fPending) - 1;
+
SkString status;
- status.printf("%s%d tasks left",
- FLAGS_verbose ? "\n" : kSkOverwriteLine,
- this->started() - this->finished());
- const int failed = this->failed();
+ status.printf("%s%d tasks left", FLAGS_verbose ? "\n" : kSkOverwriteLine, pending);
if (failed > 0) {
status.appendf(", %d failed", failed);
}
@@ -29,12 +29,9 @@
SkDebugf("%s", status.c_str());
}
-int32_t Reporter::failed() const {
- SkAutoMutexAcquire reader(&fMutex);
- return fFailures.count();
-}
-
void Reporter::fail(SkString msg) {
+ sk_atomic_inc(&fFailed);
+
SkAutoMutexAcquire writer(&fMutex);
fFailures.push_back(msg);
}
diff --git a/dm/DMReporter.h b/dm/DMReporter.h
index 3f8e953..f7803dc 100644
--- a/dm/DMReporter.h
+++ b/dm/DMReporter.h
@@ -12,20 +12,19 @@
class Reporter : SkNoncopyable {
public:
- Reporter() : fStarted(0), fFinished(0) {}
+ Reporter() : fPending(0), fFailed(0) {}
- void start() { sk_atomic_inc(&fStarted); }
- void finish(SkString name, SkMSec timeMs);
+ void taskCreated() { sk_atomic_inc(&fPending); }
+ void taskDestroyed() { sk_atomic_dec(&fPending); }
void fail(SkString msg);
- int32_t started() const { return fStarted; }
- int32_t finished() const { return fFinished; }
- int32_t failed() const;
+ void printStatus(SkString name, SkMSec timeMs) const;
void getFailures(SkTArray<SkString>*) const;
private:
- int32_t fStarted, fFinished;
+ int32_t fPending; // atomic
+ int32_t fFailed; // atomic, == fFailures.count().
mutable SkMutex fMutex; // Guards fFailures.
SkTArray<SkString> fFailures;
diff --git a/dm/DMTask.cpp b/dm/DMTask.cpp
index 7725780..e32249d 100644
--- a/dm/DMTask.cpp
+++ b/dm/DMTask.cpp
@@ -11,14 +11,18 @@
: fReporter(reporter)
, fTaskRunner(taskRunner)
, fDepth(0) {
- fReporter->start();
+ fReporter->taskCreated();
}
Task::Task(const Task& parent)
: fReporter(parent.fReporter)
, fTaskRunner(parent.fTaskRunner)
, fDepth(parent.depth() + 1) {
- fReporter->start();
+ fReporter->taskCreated();
+}
+
+Task::~Task() {
+ fReporter->taskDestroyed();
}
void Task::fail(const char* msg) {
@@ -34,7 +38,7 @@
}
void Task::finish() {
- fReporter->finish(this->name(), SkTime::GetMSecs() - fStart);
+ fReporter->printStatus(this->name(), SkTime::GetMSecs() - fStart);
}
void Task::spawnChildNext(CpuTask* task) {
@@ -45,11 +49,11 @@
CpuTask::CpuTask(const Task& parent) : Task(parent) {}
void CpuTask::run() {
- this->start();
if (FLAGS_cpu && !this->shouldSkip()) {
+ this->start();
this->draw();
+ this->finish();
}
- this->finish();
SkDELETE(this);
}
@@ -63,11 +67,11 @@
GpuTask::GpuTask(Reporter* reporter, TaskRunner* taskRunner) : Task(reporter, taskRunner) {}
void GpuTask::run(GrContextFactory& factory) {
- this->start();
if (FLAGS_gpu && !this->shouldSkip()) {
+ this->start();
this->draw(&factory);
+ this->finish();
}
- this->finish();
SkDELETE(this);
}
diff --git a/dm/DMTask.h b/dm/DMTask.h
index 96cf810..32bb948 100644
--- a/dm/DMTask.h
+++ b/dm/DMTask.h
@@ -30,7 +30,7 @@
protected:
Task(Reporter* reporter, TaskRunner* taskRunner);
Task(const Task& parent);
- virtual ~Task() {}
+ virtual ~Task();
void start();
void fail(const char* msg = NULL);