Add --properties for things like gitHash that describe the current nanobench run.

--key describes the type of run (describes the line on the chart), --properties
describes the run itself (describes the dot on the chart).

We'll pass --properties gitHash <git hash> build_number <build number>  --key
... to nanobench from the bots.

And... delete a whole lot of dead code.

Example: nanobench --properties gitHash foo build_number 1234 --key bar baz

{
   "build_number" : "1234",
   "gitHash" : "foo",
   "key" : {
      "bar" : "baz"
   },
   "results" : {
....

Friends with https://codereview.chromium.org/491943002
BUG=skia:
R=jcgregorio@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/488213002

3 files changed, 50 insertions, 307 deletions

diff --git a/bench/ResultsWriter.cpp b/bench/ResultsWriter.cpp
deleted file mode 100644
index 0ab8cb1908..0000000000
--- a/bench/ResultsWriter.cpp
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright 2014 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- *
- * Helper functions for result writing operations.
- */
-
-#include "ResultsWriter.h"
-#include "SkString.h"
-#include "SkTArray.h"
-
-Json::Value* SkFindNamedNode(Json::Value* root, const char name[]) {
-    Json::Value* search_results = NULL;
-    for(Json::Value::iterator iter = root->begin();
-            iter!= root->end(); ++iter) {
-        if(SkString(name).equals((*iter)["name"].asCString())) {
-            search_results = &(*iter);
-            break;
-        }
-    }
-
-    if(search_results != NULL) {
-        return search_results;
-    } else {
-        Json::Value* new_val = &(root->append(Json::Value()));
-        (*new_val)["name"] = name;
-        return new_val;
-    }
-}
-
-Json::Value SkMakeBuilderJSON(const SkString &builderName) {
-    static const int kNumKeys = 6;
-    static const char* kKeys[kNumKeys] = {
-        "role", "os", "model", "gpu", "arch", "configuration"};
-    Json::Value builderData;
-
-    if (!builderName.isEmpty()) {
-        SkTArray<SkString> splitBuilder;
-        SkStrSplit(builderName.c_str(), "-", &splitBuilder);
-        SkASSERT(splitBuilder.count() >= kNumKeys);
-        for (int i = 0; i < kNumKeys && i < splitBuilder.count(); ++i) {
-            builderData[kKeys[i]] = splitBuilder[i].c_str();
-        }
-        builderData["builderName"] = builderName.c_str();
-        if (kNumKeys < splitBuilder.count()) {
-            SkString extras;
-            for (int i = kNumKeys; i < splitBuilder.count(); ++i) {
-                extras.append(splitBuilder[i]);
-                if (i != splitBuilder.count() - 1) {
-                    extras.append("-");
-                }
-            }
-            builderData["badParams"] = extras.c_str();
-        }
-    } 
-    return builderData;
-}
diff --git a/bench/ResultsWriter.h b/bench/ResultsWriter.h
index dc45f192f5..56283732ec 100644
--- a/bench/ResultsWriter.h
+++ b/bench/ResultsWriter.h
@@ -20,148 +20,31 @@
 /**
  * Base class for writing out the bench results.
  *
- * TODO(jcgregorio) Add info if tests fail to converge?
+ * Default implementation does nothing.
  */
 class ResultsWriter : SkNoncopyable {
 public:
-    virtual ~ResultsWriter() {};
+    virtual ~ResultsWriter() {}
 
-    // Records one key value pair that makes up a unique identifier for this run.
-    // All keys must be set before calling bench().
-    virtual void key(const char name[], const char value[]) = 0;
+    // Record one key value pair that makes up a unique key for this type of run, e.g.
+    // builder name, machine type, Debug/Release, etc.
+    virtual void key(const char name[], const char value[]) {}
 
-    // Records one option set for this run. All options must be set before
-    // calling bench().
-    virtual void option(const char name[], const char value[]) = 0;
+    // Record one key value pair that describes the run instance, e.g. git hash, build number.
+    virtual void property(const char name[], const char value[]) {}
 
-    // Denotes the start of a specific benchmark. Once bench is called,
+    // Denote the start of a specific benchmark. Once bench is called,
     // then config and timer can be called multiple times to record runs.
-    virtual void bench(const char name[], int32_t x, int32_t y) = 0;
+    virtual void bench(const char name[], int32_t x, int32_t y) {}
 
-    // Records the specific configuration a bench is run under, such as "8888".
-    virtual void config(const char name[]) = 0;
+    // Record the specific configuration a bench is run under, such as "8888".
+    virtual void config(const char name[]) {}
 
-    // Records the options for a configuration, such as "GL_RENDERER".
-    virtual void configOption(const char name[], const char* value) = 0;
+    // Record the options for a configuration, such as "GL_RENDERER".
+    virtual void configOption(const char name[], const char* value) {}
 
-    // Records a single test metric.
-    virtual void timer(const char name[], double ms) = 0;
-
-    // Call when all results are finished.
-    virtual void end() = 0;
-};
-
-/**
- * This ResultsWriter handles writing out the human readable format of the
- * bench results.
- */
-class LoggerResultsWriter : public ResultsWriter {
-public:
-    explicit LoggerResultsWriter(BenchLogger& logger, const char* timeFormat)
-        : fLogger(logger)
-        , fTimeFormat(timeFormat) {
-        fLogger.logProgress("skia bench:");
-    }
-    virtual void key(const char name[], const char value[]) {
-        // Don't log keys to keep microbench output unchanged.
-    }
-    virtual void option(const char name[], const char value[]) {
-        fLogger.logProgress(SkStringPrintf(" %s=%s", name, value));
-    }
-    virtual void bench(const char name[], int32_t x, int32_t y) {
-        fLogger.logProgress(SkStringPrintf(
-            "\nrunning bench [%3d %3d] %40s", x, y, name));
-    }
-    virtual void config(const char name[]) {
-        fLogger.logProgress(SkStringPrintf("   %s:", name));
-    }
-    virtual void configOption(const char name[], const char* value) {
-        // Don't log configOptions to keep microbench output unchanged.
-    }
-    virtual void timer(const char name[], double ms) {
-        fLogger.logProgress(SkStringPrintf("  %s = ", name));
-        fLogger.logProgress(SkStringPrintf(fTimeFormat, ms));
-    }
-    virtual void end() {
-        fLogger.logProgress("\n");
-    }
-private:
-    BenchLogger& fLogger;
-    const char* fTimeFormat;
-};
-
-/**
- * This ResultsWriter handles writing out the results in JSON.
- *
- * The output looks like (except compressed to a single line):
- *
- *  {
- *   "options" : {
- *      "alpha" : "0xFF",
- *      "scale" : "0",
- *      ...
- *      "system" : "UNIX"
- *   },
- *   "results" : [
- *      {
- *      "name" : "Xfermode_Luminosity_640_480",
- *      "results" : [
- *         {
- *            "name": "565",
- *            "cmsecs" : 143.188128906250,
- *            "msecs" : 143.835957031250
- *         },
- *         ...
- */
-
-Json::Value* SkFindNamedNode(Json::Value* root, const char name[]);
-Json::Value SkMakeBuilderJSON(const SkString &buildername);
-
-class JSONResultsWriter : public ResultsWriter {
-public:
-    explicit JSONResultsWriter(const char filename[])
-        : fFilename(filename)
-        , fRoot()
-        , fResults(fRoot["results"])
-        , fBench(NULL)
-        , fConfig(NULL) {
-    }
-    virtual void key(const char name[], const char value[]) {
-    }
-    virtual void option(const char name[], const char value[]) {
-        fRoot["options"][name] = value;
-    }
-    virtual void bench(const char name[], int32_t x, int32_t y) {
-        SkString sk_name(name);
-        sk_name.append("_");
-        sk_name.appendS32(x);
-        sk_name.append("_");
-        sk_name.appendS32(y);
-        Json::Value* bench_node = SkFindNamedNode(&fResults, sk_name.c_str());
-        fBench = &(*bench_node)["results"];
-    }
-    virtual void config(const char name[]) {
-        SkASSERT(NULL != fBench);
-        fConfig = SkFindNamedNode(fBench, name);
-    }
-    virtual void configOption(const char name[], const char* value) {
-    }
-    virtual void timer(const char name[], double ms) {
-        SkASSERT(NULL != fConfig);
-        (*fConfig)[name] = ms;
-    }
-    virtual void end() {
-        SkFILEWStream stream(fFilename.c_str());
-        stream.writeText(Json::FastWriter().write(fRoot).c_str());
-        stream.flush();
-    }
-private:
-
-    SkString fFilename;
-    Json::Value fRoot;
-    Json::Value& fResults;
-    Json::Value* fBench;
-    Json::Value* fConfig;
+    // Record a single test metric.
+    virtual void timer(const char name[], double ms) {}
 };
 
 /**
@@ -175,11 +58,8 @@ private:
       "os": "Android",
       "model": "GalaxyNexus",
     }
-    "options": {
-       "GL_Version": "3.1",
-       ...
-    },
     "gitHash": "d1830323662ae8ae06908b97f15180fd25808894",
+    "build_number": "1234",
     "results" : {
         "Xfermode_Luminosity_640_480" : {
            "8888" : {
@@ -191,19 +71,26 @@ private:
 */
 class NanoJSONResultsWriter : public ResultsWriter {
 public:
-    explicit NanoJSONResultsWriter(const char filename[], const char gitHash[])
+    explicit NanoJSONResultsWriter(const char filename[])
         : fFilename(filename)
         , fRoot()
         , fResults(fRoot["results"])
         , fBench(NULL)
-        , fConfig(NULL) {
-        fRoot["gitHash"] = gitHash;
+        , fConfig(NULL) {}
+
+    ~NanoJSONResultsWriter() {
+        SkFILEWStream stream(fFilename.c_str());
+        stream.writeText(Json::StyledWriter().write(fRoot).c_str());
+        stream.flush();
     }
+
+    // Added under "key".
     virtual void key(const char name[], const char value[]) {
         fRoot["key"][name] = value;
     }
-    virtual void option(const char name[], const char value[]) {
-        fRoot["options"][name] = value;
+    // Inserted directly into the root.
+    virtual void property(const char name[], const char value[]) {
+        fRoot[name] = value;
     }
     virtual void bench(const char name[], int32_t x, int32_t y) {
         SkString id = SkStringPrintf( "%s_%d_%d", name, x, y);
@@ -225,13 +112,8 @@ public:
         SkASSERT(NULL != fConfig);
         (*fConfig)[name] = ms;
     }
-    virtual void end() {
-        SkFILEWStream stream(fFilename.c_str());
-        stream.writeText(Json::FastWriter().write(fRoot).c_str());
-        stream.flush();
-    }
-private:
 
+private:
     SkString fFilename;
     Json::Value fRoot;
     Json::Value& fResults;
@@ -240,64 +122,4 @@ private:
 };
 
 
-/**
- * This ResultsWriter writes out to multiple ResultsWriters.
- */
-class MultiResultsWriter : public ResultsWriter {
-public:
-    MultiResultsWriter() : writers() {
-    };
-    void add(ResultsWriter* writer) {
-      writers.push_back(writer);
-    }
-    virtual void key(const char name[], const char value[]) {
-        for (int i = 0; i < writers.count(); ++i) {
-            writers[i]->key(name, value);
-        }
-    }
-    virtual void option(const char name[], const char value[]) {
-        for (int i = 0; i < writers.count(); ++i) {
-            writers[i]->option(name, value);
-        }
-    }
-    virtual void bench(const char name[], int32_t x, int32_t y) {
-        for (int i = 0; i < writers.count(); ++i) {
-            writers[i]->bench(name, x, y);
-        }
-    }
-    virtual void config(const char name[]) {
-        for (int i = 0; i < writers.count(); ++i) {
-            writers[i]->config(name);
-        }
-    }
-    virtual void configOption(const char name[], const char* value) {
-        for (int i = 0; i < writers.count(); ++i) {
-            writers[i]->configOption(name, value);
-        }
-    }
-    virtual void timer(const char name[], double ms) {
-        for (int i = 0; i < writers.count(); ++i) {
-            writers[i]->timer(name, ms);
-        }
-    }
-    virtual void end() {
-        for (int i = 0; i < writers.count(); ++i) {
-            writers[i]->end();
-        }
-    }
-private:
-    SkTArray<ResultsWriter *> writers;
-};
-
-/**
- * Calls the end() method of T on destruction.
- */
-template <typename T> class CallEnd : SkNoncopyable {
-public:
-    CallEnd(T& obj) : fObj(obj) {}
-    ~CallEnd() { fObj.end(); }
-private:
-    T&  fObj;
-};
-
 #endif
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index fedc9ea29f..6968d6dac2 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -64,11 +64,10 @@ DEFINE_string(outResultsFile, "", "If given, write results here as JSON.");
 DEFINE_int32(maxCalibrationAttempts, 3,
              "Try up to this many times to guess loops for a bench, or skip the bench.");
 DEFINE_int32(maxLoops, 1000000, "Never run a bench more times than this.");
+DEFINE_string(properties, "",
+              "Space-separated key/value pairs to add to JSON identifying this nanobench run.");
 DEFINE_string(key, "",
               "Space-separated key/value pairs to add to JSON identifying this bench config.");
-DEFINE_string(options, "",
-              "Space-separated option/value pairs to add to JSON, logging extra info.");
-DEFINE_string(gitHash, "", "Git hash to add to JSON.");
 
 DEFINE_string(clip, "0,0,1000,1000", "Clip for SKPs.");
 DEFINE_string(scales, "1.0", "Space-separated scales for SKPs.");
@@ -392,20 +391,6 @@ static void create_targets(SkTDArray<Target*>* targets, Benchmark* b,
     }
 }
 
-static void fill_static_options(ResultsWriter* log) {
-#if defined(SK_BUILD_FOR_WIN32)
-    log->option("system", "WIN32");
-#elif defined(SK_BUILD_FOR_MAC)
-    log->option("system", "MAC");
-#elif defined(SK_BUILD_FOR_ANDROID)
-    log->option("system", "ANDROID");
-#elif defined(SK_BUILD_FOR_UNIX)
-    log->option("system", "UNIX");
-#else
-    log->option("system", "other");
-#endif
-}
-
 #if SK_SUPPORT_GPU
 static void fill_gpu_options(ResultsWriter* log, SkGLContextHelper* ctx) {
     const GrGLubyte* version;
@@ -553,30 +538,25 @@ int nanobench_main() {
         }
     }
 
-    MultiResultsWriter log;
-    SkAutoTDelete<NanoJSONResultsWriter> json;
+    SkAutoTDelete<ResultsWriter> log(SkNEW(ResultsWriter));
     if (!FLAGS_outResultsFile.isEmpty()) {
-        const char* gitHash = FLAGS_gitHash.isEmpty() ? "unknown-revision" : FLAGS_gitHash[0];
-        json.reset(SkNEW(NanoJSONResultsWriter(FLAGS_outResultsFile[0], gitHash)));
-        log.add(json.get());
+        log.reset(SkNEW(NanoJSONResultsWriter(FLAGS_outResultsFile[0])));
     }
-    CallEnd<MultiResultsWriter> ender(log);
 
-    if (1 == FLAGS_key.count() % 2) {
-        SkDebugf("ERROR: --key must be passed with an even number of arguments.\n");
+    if (1 == FLAGS_properties.count() % 2) {
+        SkDebugf("ERROR: --properties must be passed with an even number of arguments.\n");
         return 1;
     }
-    for (int i = 1; i < FLAGS_key.count(); i += 2) {
-        log.key(FLAGS_key[i-1], FLAGS_key[i]);
+    for (int i = 1; i < FLAGS_properties.count(); i += 2) {
+        log->property(FLAGS_properties[i-1], FLAGS_properties[i]);
     }
 
-    fill_static_options(&log);
-    if (1 == FLAGS_options.count() % 2) {
-        SkDebugf("ERROR: --options must be passed with an even number of arguments.\n");
+    if (1 == FLAGS_key.count() % 2) {
+        SkDebugf("ERROR: --key must be passed with an even number of arguments.\n");
         return 1;
     }
-    for (int i = 1; i < FLAGS_options.count(); i += 2) {
-        log.option(FLAGS_options[i-1], FLAGS_options[i]);
+    for (int i = 1; i < FLAGS_key.count(); i += 2) {
+        log->key(FLAGS_key[i-1], FLAGS_key[i]);
     }
 
     const double overhead = estimate_timer_overhead();
@@ -608,7 +588,7 @@ int nanobench_main() {
         create_targets(&targets, bench.get(), configs);
 
         if (!targets.isEmpty()) {
-            log.bench(bench->getName(), bench->getSize().fX, bench->getSize().fY);
+            log->bench(bench->getName(), bench->getSize().fX, bench->getSize().fY);
             bench->preDraw();
         }
         for (int j = 0; j < targets.count(); j++) {
@@ -644,18 +624,18 @@ int nanobench_main() {
             }
 
             Stats stats(samples.get(), FLAGS_samples);
-            log.config(config);
-            benchStream.fillCurrentOptions(&log);
+            log->config(config);
+            benchStream.fillCurrentOptions(log.get());
 #if SK_SUPPORT_GPU
             if (Benchmark::kGPU_Backend == targets[j]->config.backend) {
-                fill_gpu_options(&log, targets[j]->gl);
+                fill_gpu_options(log.get(), targets[j]->gl);
             }
 #endif
-            log.timer("min_ms",    stats.min);
-            log.timer("median_ms", stats.median);
-            log.timer("mean_ms",   stats.mean);
-            log.timer("max_ms",    stats.max);
-            log.timer("stddev_ms", sqrt(stats.var));
+            log->timer("min_ms",    stats.min);
+            log->timer("median_ms", stats.median);
+            log->timer("mean_ms",   stats.mean);
+            log->timer("max_ms",    stats.max);
+            log->timer("stddev_ms", sqrt(stats.var));
 
             if (kAutoTuneLoops != FLAGS_loops) {
                 if (targets.count() == 1) {