diff options
author | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-02-25 20:02:09 +0000 |
---|---|---|
committer | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-02-25 20:02:09 +0000 |
commit | 79e13260cf94427e6ccbfff8242bf85ed4c8187b (patch) | |
tree | decbb335453684fcbe0c9dff854c2b1968226d8a /dm | |
parent | 6bd250a2a340348434b7b16bd4e4b5da0f598e3e (diff) |
Revert of Let DM run unit tests. (https://codereview.chromium.org/178273002/)
Reason for revert:
broke tests
Original issue's description:
> Let DM run unit tests.
> - refactor GYPs and a few flags
> - make GPU tests grab a thread-local GrContextFactory when needed as we do in DM for GMs
> - add a few more UI features to make DM more like tests
>
> I believe this makes the program 'tests' obsolete.
>
> It should be somewhat faster to run the two sets together than running the old binaries serially:
> - serial: tests 20s (3m18s CPU), dm 21s (3m01s CPU)
> - together: 27s (6m21s CPU)
>
> Next up is to incorporate benches. I'm only planning there on a single-pass sanity check, so that won't obsolete the program 'bench' just yet.
>
> Tested: out/Debug/tests && out/Debug/dm && echo ok
> BUG=skia:
>
> Committed: http://code.google.com/p/skia/source/detail?r=13586
R=bsalomon@google.com, mtklein@google.com, tfarina@chromium.org, mtklein@chromium.org
TBR=bsalomon@google.com, mtklein@chromium.org, mtklein@google.com, tfarina@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Author: reed@google.com
Review URL: https://codereview.chromium.org/179403010
git-svn-id: http://skia.googlecode.com/svn/trunk@13587 2bbb7eff-a529-9590-31e7-b0007b416f81
Diffstat (limited to 'dm')
-rw-r--r-- | dm/DM.cpp | 92 | ||||
-rw-r--r-- | dm/DMGpuTask.cpp | 19 | ||||
-rw-r--r-- | dm/DMGpuTask.h | 1 | ||||
-rw-r--r-- | dm/DMReporter.cpp | 16 | ||||
-rw-r--r-- | dm/DMReporter.h | 7 | ||||
-rw-r--r-- | dm/DMTask.cpp | 11 | ||||
-rw-r--r-- | dm/DMTask.h | 2 | ||||
-rw-r--r-- | dm/DMTaskRunner.cpp | 28 | ||||
-rw-r--r-- | dm/DMTaskRunner.h | 12 | ||||
-rw-r--r-- | dm/DMTestTask.cpp | 34 | ||||
-rw-r--r-- | dm/DMTestTask.h | 50 |
11 files changed, 67 insertions, 205 deletions
@@ -7,29 +7,26 @@ #include "SkForceLinking.h" #include "SkGraphics.h" #include "SkString.h" -#include "Test.h" #include "gm.h" -#include "DMCpuTask.h" -#include "DMGpuTask.h" #include "DMReporter.h" #include "DMTask.h" #include "DMTaskRunner.h" -#include "DMTestTask.h" +#include "DMCpuTask.h" +#include "DMGpuTask.h" #include "DMWriteTask.h" #include <string.h> using skiagm::GM; using skiagm::GMRegistry; -using skiatest::Test; -using skiatest::TestRegistry; -DEFINE_int32(threads, -1, "Threads for CPU work. Default NUM_CPUS."); +DEFINE_int32(cpuThreads, -1, "Threads for CPU work. Default NUM_CPUS."); +DEFINE_int32(gpuThreads, 1, "Threads for GPU work."); DEFINE_string2(expectations, r, "", "If a directory, compare generated images against images under this path. " "If a file, compare generated images against JSON expectations at this path."); -DEFINE_string2(resources, i, "resources", "Path to resources directory."); +DEFINE_string(resources, "resources", "Path to resources directory."); DEFINE_string(match, "", "[~][^]substring[$] [...] of GM name to run.\n" "Multiple matches may be separated by spaces.\n" "~ causes a matching GM to always be skipped\n" @@ -40,10 +37,6 @@ DEFINE_string(match, "", "[~][^]substring[$] [...] of GM name to run.\n" "it is skipped unless some list entry starts with ~"); DEFINE_string(config, "565 8888 gpu", "Options: 565 8888 gpu msaa4 msaa16 gpunull gpudebug angle mesa"); // TODO(mtklein): pdf -DEFINE_bool(leaks, false, "Print leaked instance-counted objects at exit?"); - -DEFINE_bool(gms, true, "Run GMs?"); -DEFINE_bool(tests, true, "Run tests?"); __SK_FORCE_IMAGE_DECODER_LINKING; @@ -55,11 +48,11 @@ static SkString lowercase(SkString s) { return s; } -static void kick_off_gms(const SkTDArray<GMRegistry::Factory>& gms, - const SkTArray<SkString>& configs, - const DM::Expectations& expectations, - DM::Reporter* reporter, - DM::TaskRunner* tasks) { +static void kick_off_tasks(const SkTDArray<GMRegistry::Factory>& gms, + const SkTArray<SkString>& configs, + const DM::Expectations& expectations, + DM::Reporter* reporter, + DM::TaskRunner* tasks) { const SkColorType _565 = kRGB_565_SkColorType; const SkColorType _8888 = kPMColor_SkColorType; const GrContextFactory::GLContextType native = GrContextFactory::kNative_GLContextType; @@ -100,14 +93,6 @@ static void kick_off_gms(const SkTDArray<GMRegistry::Factory>& gms, #undef START } -static void kick_off_tests(const SkTDArray<TestRegistry::Factory>& tests, - DM::Reporter* reporter, - DM::TaskRunner* tasks) { - for (int i = 0; i < tests.count(); i++) { - tasks->add(SkNEW_ARGS(DM::TestTask, (reporter, tasks, tests[i]))); - } -} - static void report_failures(const DM::Reporter& reporter) { SkTArray<SkString> failures; reporter.getFailures(&failures); @@ -124,57 +109,40 @@ static void report_failures(const DM::Reporter& reporter) { int tool_main(int argc, char** argv); int tool_main(int argc, char** argv) { -#if SK_ENABLE_INST_COUNT - gPrintInstCount = FLAGS_leaks; -#endif SkGraphics::Init(); + SkCommandLineFlags::Parse(argc, argv); GM::SetResourcePath(FLAGS_resources[0]); - Test::SetResourcePath(FLAGS_resources[0]); - SkTArray<SkString> configs; - SkTDArray<GMRegistry::Factory> gms; - SkAutoTDelete<DM::Expectations> expectations(SkNEW(DM::NoExpectations)); - - if (FLAGS_gms) { - for (int i = 0; i < FLAGS_config.count(); i++) { - SkStrSplit(FLAGS_config[i], ", ", &configs); - } - - for (const GMRegistry* reg = GMRegistry::Head(); reg != NULL; reg = reg->next()) { - SkAutoTDelete<GM> gmForName(reg->factory()(NULL)); - if (!SkCommandLineFlags::ShouldSkip(FLAGS_match, gmForName->shortName())) { - *gms.append() = reg->factory(); - } - } + for (int i = 0; i < FLAGS_config.count(); i++) { + SkStrSplit(FLAGS_config[i], ", ", &configs); + } - if (FLAGS_expectations.count() > 0) { - const char* path = FLAGS_expectations[0]; - if (sk_isdir(path)) { - expectations.reset(SkNEW_ARGS(DM::WriteTask::Expectations, (path))); - } else { - expectations.reset(SkNEW_ARGS(DM::JsonExpectations, (path))); - } + SkTDArray<GMRegistry::Factory> gms; + for (const GMRegistry* reg = GMRegistry::Head(); reg != NULL; reg = reg->next()) { + SkAutoTDelete<GM> gmForName(reg->factory()(NULL)); + if (!SkCommandLineFlags::ShouldSkip(FLAGS_match, gmForName->shortName())) { + *gms.append() = reg->factory(); } } + SkDebugf("%d GMs x %d configs\n", gms.count(), configs.count()); - SkTDArray<TestRegistry::Factory> tests; - if (FLAGS_tests) { - for (const TestRegistry* reg = TestRegistry::Head(); reg != NULL; reg = reg->next()) { - SkAutoTDelete<Test> testForName(reg->factory()(NULL)); - if (!SkCommandLineFlags::ShouldSkip(FLAGS_match, testForName->getName())) { - *tests.append() = reg->factory(); - } + SkAutoTDelete<DM::Expectations> expectations(SkNEW(DM::NoExpectations)); + if (FLAGS_expectations.count() > 0) { + const char* path = FLAGS_expectations[0]; + if (sk_isdir(path)) { + expectations.reset(SkNEW_ARGS(DM::WriteTask::Expectations, (path))); + } else { + expectations.reset(SkNEW_ARGS(DM::JsonExpectations, (path))); } } - SkDebugf("%d GMs x %d configs, %d tests\n", gms.count(), configs.count(), tests.count()); DM::Reporter reporter; - DM::TaskRunner tasks(FLAGS_threads); - kick_off_gms(gms, configs, *expectations, &reporter, &tasks); - kick_off_tests(tests, &reporter, &tasks); + DM::TaskRunner tasks(FLAGS_cpuThreads, FLAGS_gpuThreads); + kick_off_tasks(gms, configs, *expectations, &reporter, &tasks); tasks.wait(); + reporter.updateStatusLine(); SkDebugf("\n"); report_failures(reporter); diff --git a/dm/DMGpuTask.cpp b/dm/DMGpuTask.cpp index 3a4708b8db..f787e2544f 100644 --- a/dm/DMGpuTask.cpp +++ b/dm/DMGpuTask.cpp @@ -18,7 +18,6 @@ GpuTask::GpuTask(const char* name, GrContextFactory::GLContextType contextType, int sampleCount) : Task(reporter, taskRunner) - , fTaskRunner(taskRunner) , fGM(gmFactory(NULL)) , fName(UnderJoin(fGM->shortName(), name)) , fExpectations(expectations) @@ -27,12 +26,24 @@ GpuTask::GpuTask(const char* name, , fSampleCount(sampleCount) {} +static void* new_gr_context_factory() { + return SkNEW(GrContextFactory); +} + +static void delete_gr_context_factory(void* factory) { + SkDELETE((GrContextFactory*) factory); +} + +static GrContextFactory* get_gr_factory() { + return reinterpret_cast<GrContextFactory*>(SkTLS::Get(&new_gr_context_factory, + &delete_gr_context_factory)); +} + void GpuTask::draw() { + GrContext* gr = get_gr_factory()->get(fContextType); // Will be owned by device. SkImageInfo info = SkImageInfo::Make(SkScalarCeilToInt(fGM->width()), SkScalarCeilToInt(fGM->height()), - fColorType, - kPremul_SkAlphaType); - GrContext* gr = fTaskRunner->getGrContextFactory()->get(fContextType); // Owned by surface. + fColorType, kPremul_SkAlphaType); SkAutoTUnref<SkSurface> surface(SkSurface::NewRenderTarget(gr, info, fSampleCount)); SkCanvas* canvas = surface->getCanvas(); diff --git a/dm/DMGpuTask.h b/dm/DMGpuTask.h index 1380e443d4..a3fe52b751 100644 --- a/dm/DMGpuTask.h +++ b/dm/DMGpuTask.h @@ -32,7 +32,6 @@ public: virtual SkString name() const SK_OVERRIDE { return fName; } private: - TaskRunner* fTaskRunner; SkAutoTDelete<skiagm::GM> fGM; const SkString fName; const Expectations& fExpectations; diff --git a/dm/DMReporter.cpp b/dm/DMReporter.cpp index 1ff64c57cc..0fd83e5bb6 100644 --- a/dm/DMReporter.cpp +++ b/dm/DMReporter.cpp @@ -3,27 +3,21 @@ #include "SkCommandLineFlags.h" #include "OverwriteLine.h" -DEFINE_bool2(quiet, q, false, "If true, don't print status updates."); -DEFINE_bool2(verbose, v, false, "If true, print status updates one-per-line."); +DEFINE_bool(quiet, false, "If true, don't print status updates."); namespace DM { -void Reporter::finish(SkString name) { - sk_atomic_inc(&fFinished); - +void Reporter::updateStatusLine() const { if (FLAGS_quiet) { return; } SkString status; - status.printf("%s%d tasks left", - FLAGS_verbose ? "\n" : kSkOverwriteLine, - this->started() - this->finished()); + status.printf("%s%d tasks left", kSkOverwriteLine, this->started() - this->finished()); const int failed = this->failed(); if (failed > 0) { status.appendf(", %d failed", failed); } - status.appendf("\t[%s done]", name.c_str()); SkDebugf(status.c_str()); } @@ -32,9 +26,9 @@ int32_t Reporter::failed() const { return fFailures.count(); } -void Reporter::fail(SkString msg) { +void Reporter::fail(SkString name) { SkAutoMutexAcquire writer(&fMutex); - fFailures.push_back(msg); + fFailures.push_back(name); } void Reporter::getFailures(SkTArray<SkString>* failures) const { diff --git a/dm/DMReporter.h b/dm/DMReporter.h index 2bb4702178..4f4ad432d5 100644 --- a/dm/DMReporter.h +++ b/dm/DMReporter.h @@ -7,6 +7,7 @@ #include "SkTypes.h" // Used to report status changes including failures. All public methods are threadsafe. + namespace DM { class Reporter : SkNoncopyable { @@ -14,13 +15,15 @@ public: Reporter() : fStarted(0), fFinished(0) {} void start() { sk_atomic_inc(&fStarted); } - void finish(SkString name); - void fail(SkString msg); + void finish() { sk_atomic_inc(&fFinished); } + void fail(SkString name); int32_t started() const { return fStarted; } int32_t finished() const { return fFinished; } int32_t failed() const; + void updateStatusLine() const; + void getFailures(SkTArray<SkString>*) const; private: diff --git a/dm/DMTask.cpp b/dm/DMTask.cpp index 2f009f0670..a5c75f0f8a 100644 --- a/dm/DMTask.cpp +++ b/dm/DMTask.cpp @@ -26,7 +26,8 @@ void Task::run() { if (!this->shouldSkip()) { this->draw(); } - fReporter->finish(this->name()); + fReporter->finish(); + fReporter->updateStatusLine(); delete this; } @@ -38,12 +39,8 @@ void Task::spawnChild(Task* task) { } } -void Task::fail(const char* msg) { - SkString failure(this->name()); - if (msg) { - failure.appendf(": %s", msg); - } - fReporter->fail(failure); +void Task::fail() { + fReporter->fail(this->name()); } } // namespace DM diff --git a/dm/DMTask.h b/dm/DMTask.h index 638a939304..0d3ef74cc3 100644 --- a/dm/DMTask.h +++ b/dm/DMTask.h @@ -34,7 +34,7 @@ public: protected: void spawnChild(Task* task); - void fail(const char* msg = NULL); + void fail(); private: // Both unowned. diff --git a/dm/DMTaskRunner.cpp b/dm/DMTaskRunner.cpp index bd53ce615a..22269a4d70 100644 --- a/dm/DMTaskRunner.cpp +++ b/dm/DMTaskRunner.cpp @@ -3,21 +3,10 @@ namespace DM { - -TaskRunner::TaskRunner(int cputhreads) +TaskRunner::TaskRunner(int cputhreads, int gpuThreads) : fMain(cputhreads) - , fGpu(1) { - // Enqueue a task on the GPU thread to create a GrContextFactory. - struct Create : public SkRunnable { - Create(GrContextFactory** ptr) : fPtr(ptr) {} - void run() SK_OVERRIDE { - *fPtr = SkNEW(GrContextFactory); - delete this; - } - GrContextFactory** fPtr; - }; - fGpu.add(SkNEW_ARGS(Create, (&fGrContextFactory))); -} + , fGpu(gpuThreads) + {} void TaskRunner::add(Task* task) { if (task->usesGpu()) { @@ -28,17 +17,6 @@ void TaskRunner::add(Task* task) { } void TaskRunner::wait() { - // Enqueue a task on the GPU thread to destroy the GrContextFactory. - struct Delete : public SkRunnable { - Delete(GrContextFactory* ptr) : fPtr(ptr) {} - void run() SK_OVERRIDE { - delete fPtr; - delete this; - } - GrContextFactory* fPtr; - }; - fGpu.add(SkNEW_ARGS(Delete, (fGrContextFactory))); - // These wait calls block until the threadpool is done. We don't allow // children to spawn new GPU tasks so we can wait for that first knowing // we'll never try to add to it later. Same can't be said of fMain: fGpu diff --git a/dm/DMTaskRunner.h b/dm/DMTaskRunner.h index 8af1b63719..5d7b320d6c 100644 --- a/dm/DMTaskRunner.h +++ b/dm/DMTaskRunner.h @@ -1,12 +1,12 @@ #ifndef DMTaskRunner_DEFINED #define DMTaskRunner_DEFINED -#include "GrContextFactory.h" #include "SkThreadPool.h" #include "SkTypes.h" -// TaskRunner runs Tasks on one of two threadpools depending on the Task's usesGpu() method. This -// lets us drive the GPU from a single thread while parallelizing CPU-bound work. +// TaskRunner runs Tasks on one of two threadpools depending on the Task's usesGpu() method. +// This lets us drive the GPU with a small number of threads (e.g. 2 or 4 can be faster than 1) +// while not swamping it with requests from the full fleet of threads that CPU-bound tasks run on. namespace DM { @@ -14,17 +14,13 @@ class Task; class TaskRunner : SkNoncopyable { public: - explicit TaskRunner(int cputhreads); + TaskRunner(int cputhreads, int gpuThreads); void add(Task* task); void wait(); - // This can only be safely called from a GPU task's draw() method. - GrContextFactory* getGrContextFactory() const { return fGrContextFactory; } - private: SkThreadPool fMain, fGpu; - GrContextFactory* fGrContextFactory; // Created and destroyed on fGpu threadpool. }; } // namespace DM diff --git a/dm/DMTestTask.cpp b/dm/DMTestTask.cpp deleted file mode 100644 index 7a5f8cf9f9..0000000000 --- a/dm/DMTestTask.cpp +++ /dev/null @@ -1,34 +0,0 @@ -#include "DMTestTask.h" -#include "DMUtil.h" -#include "SkCommandLineFlags.h" - -DEFINE_bool2(pathOpsExtended, x, false, "Run extended pathOps tests."); -DEFINE_bool2(pathOpsSingleThread, z, false, "Disallow pathOps tests from using threads."); -DEFINE_bool2(pathOpsVerbose, V, false, "Tell pathOps tests to be verbose."); - -namespace DM { - -TestTask::TestTask(Reporter* reporter, - TaskRunner* taskRunner, - skiatest::TestRegistry::Factory factory) - : Task(reporter, taskRunner) - , fTaskRunner(taskRunner) - , fTest(factory(NULL)) - , fName(UnderJoin("test", fTest->getName())) {} - -void TestTask::draw() { - if (this->usesGpu()) { - fTest->setGrContextFactory(fTaskRunner->getGrContextFactory()); - } - fTest->setReporter(&fTestReporter); - fTest->run(); - if (!fTest->passed()) { - this->fail(fTestReporter.failure()); - } -} - -bool TestTask::TestReporter::allowExtendedTest() const { return FLAGS_pathOpsExtended; } -bool TestTask::TestReporter::allowThreaded() const { return !FLAGS_pathOpsSingleThread; } -bool TestTask::TestReporter::verbose() const { return FLAGS_pathOpsVerbose; } - -} // namespace DM diff --git a/dm/DMTestTask.h b/dm/DMTestTask.h deleted file mode 100644 index a4903ee717..0000000000 --- a/dm/DMTestTask.h +++ /dev/null @@ -1,50 +0,0 @@ -#ifndef DMTestTask_DEFINED -#define DMTestTask_DEFINED - -#include "DMReporter.h" -#include "DMTask.h" -#include "DMTaskRunner.h" -#include "SkString.h" -#include "SkTemplates.h" -#include "Test.h" - -// Runs a unit test. -namespace DM { - -class TestTask : public Task { -public: - TestTask(Reporter*, TaskRunner*, skiatest::TestRegistry::Factory); - - virtual void draw() SK_OVERRIDE; - virtual bool usesGpu() const SK_OVERRIDE { return fTest->isGPUTest(); } - virtual bool shouldSkip() const SK_OVERRIDE { return false; } - virtual SkString name() const SK_OVERRIDE { return fName; } - -private: - class TestReporter : public skiatest::Reporter { - public: - TestReporter() {} - - const char* failure() const { return fFailure.c_str(); } - - private: - virtual bool allowExtendedTest() const SK_OVERRIDE; - virtual bool allowThreaded() const SK_OVERRIDE; - virtual bool verbose() const SK_OVERRIDE; - - virtual void onReportFailed(const SkString& desc) SK_OVERRIDE { - fFailure = desc; - } - - SkString fFailure; - }; - - TaskRunner* fTaskRunner; - TestReporter fTestReporter; - SkAutoTDelete<skiatest::Test> fTest; - const SkString fName; -}; - -} // namespace DM - -#endif // DMTestTask_DEFINED |