Switch nanobench to SkJSONWriter

Bug: skia:
Change-Id: I3b014b71695b598c72c569f1466ad3dd7c85cd98
Reviewed-on: https://skia-review.googlesource.com/c/187386
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>

3 files changed, 77 insertions, 140 deletions

diff --git a/bench/ResultsWriter.h b/bench/ResultsWriter.h
index 85c44136fe..41f3ae3b1a 100644
--- a/bench/ResultsWriter.h
+++ b/bench/ResultsWriter.h
@@ -10,53 +10,12 @@
 #ifndef SkResultsWriter_DEFINED
 #define SkResultsWriter_DEFINED
 
-#include "BenchLogger.h"
-#include "SkJSONCPP.h"
-#include "SkOSFile.h"
-#include "SkOSPath.h"
-#include "SkStream.h"
+#include "SkJSONWriter.h"
 #include "SkString.h"
 #include "SkTypes.h"
 
 /**
- * Base class for writing out the bench results.
- *
- * Default implementation does nothing.
- */
-class ResultsWriter : SkNoncopyable {
-public:
-    virtual ~ResultsWriter() {}
-
-    // 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[]) {}
-
-    // 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[]) {}
-
-    // Denote the start of a specific benchmark. Once bench is called,
-    // then config and metric can be called multiple times to record runs.
-    virtual void bench(const char name[], int32_t x, int32_t y) {}
-
-    // Record the specific configuration a bench is run under, such as "8888".
-    virtual void config(const char name[]) {}
-
-    // Record the options for a configuration, such as "GL_RENDERER".
-    virtual void configOption(const char name[], const char* value) {}
-
-    // Record a single test metric.
-    virtual void metric(const char name[], double ms) {}
-
-    // Record a list of test metrics.
-    virtual void metrics(const char name[], const SkTArray<double>& array) {}
-
-    // Flush to storage now please.
-    virtual void flush() {}
-};
-
-/**
- NanoJSONResultsWriter writes the test results out in the following
- format:
+ NanoJSONResultsWriter helps nanobench writes the test results out in the following format:
 
  {
     "key": {
@@ -76,78 +35,23 @@ public:
               },
           ...
 */
-class NanoJSONResultsWriter : public ResultsWriter {
+class NanoJSONResultsWriter : public SkJSONWriter {
 public:
-    explicit NanoJSONResultsWriter(const char filename[])
-        : fFilename(filename)
-        , fRoot()
-        , fResults(fRoot["results"])
-        , fBench(nullptr)
-        , fConfig(nullptr) {}
+    NanoJSONResultsWriter(SkWStream* stream, Mode mode) : SkJSONWriter(stream, mode) {}
 
-    ~NanoJSONResultsWriter() override {
-        this->flush();
+    void beginBench(const char* name, int32_t x, int32_t y) {
+        SkString id = SkStringPrintf("%s_%d_%d", name, x, y);
+        this->beginObject(id.c_str());
     }
 
-    // Added under "key".
-    void key(const char name[], const char value[]) override {
-        fRoot["key"][name] = value;
-    }
-    // Inserted directly into the root.
-    void property(const char name[], const char value[]) override {
-        fRoot[name] = value;
-    }
-    void bench(const char name[], int32_t x, int32_t y) override {
-        SkString id = SkStringPrintf( "%s_%d_%d", name, x, y);
-        fResults[id.c_str()] = Json::Value(Json::objectValue);
-        fBench = &fResults[id.c_str()];
-    }
-    void config(const char name[]) override {
-        SkASSERT(fBench);
-        fConfig = &(*fBench)[name];
-    }
-    void configOption(const char name[], const char* value) override {
-        (*fConfig)["options"][name] = value;
-    }
-    void metric(const char name[], double ms) override {
-        // Don't record if nan, or -nan.
-        if (sk_double_isnan(ms)) {
-            return;
-        }
-        SkASSERT(fConfig);
-        (*fConfig)[name] = ms;
-    }
-    void metrics(const char name[], const SkTArray<double>& array) override {
-        SkASSERT(fConfig);
-        Json::Value value = Json::Value(Json::arrayValue);
-        value.resize(array.count());
-        for (int i = 0; i < array.count(); i++) {
-            // Don't care about nan-ness.
-            value[i] = array[i];
-        }
-        (*fConfig)[name] = std::move(value);
-    }
+    void endBench() { this->endObject(); }
 
-    // Flush to storage now please.
-    void flush() override {
-        SkString dirname = SkOSPath::Dirname(fFilename.c_str());
-        if (!sk_exists(dirname.c_str(), kWrite_SkFILE_Flag)) {
-            if (!sk_mkdir(dirname.c_str())) {
-                SkDebugf("Failed to create directory.");
-            }
+    void appendMetric(const char* name, double value) {
+        // Don't record if nan, or -nan.
+        if (!sk_double_isnan(value)) {
+            this->appendDoubleDigits(name, value, 16);
         }
-        SkFILEWStream stream(fFilename.c_str());
-        stream.writeText(Json::StyledWriter().write(fRoot).c_str());
-        stream.flush();
     }
-
-private:
-    SkString fFilename;
-    Json::Value fRoot;
-    Json::Value& fResults;
-    Json::Value* fBench;
-    Json::Value* fConfig;
 };
 
-
 #endif
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index 8a9e0d6f33..3123b27da1 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -35,6 +35,7 @@
 #include "SkDebugfTracer.h"
 #include "SkEventTracingPriv.h"
 #include "SkGraphics.h"
+#include "SkJSONWriter.h"
 #include "SkLeanWindows.h"
 #include "SkOSFile.h"
 #include "SkOSPath.h"
@@ -211,23 +212,23 @@ struct GPUTarget : public Target {
         }
         return true;
     }
-    void fillOptions(ResultsWriter* log) override {
+    void fillOptions(NanoJSONResultsWriter& log) override {
         const GrGLubyte* version;
         if (this->contextInfo.backend() == GrBackendApi::kOpenGL) {
             const GrGLInterface* gl =
                     static_cast<GrGLGpu*>(this->contextInfo.grContext()->contextPriv().getGpu())
                             ->glInterface();
             GR_GL_CALL_RET(gl, version, GetString(GR_GL_VERSION));
-            log->configOption("GL_VERSION", (const char*)(version));
+            log.appendString("GL_VERSION", (const char*)(version));
 
             GR_GL_CALL_RET(gl, version, GetString(GR_GL_RENDERER));
-            log->configOption("GL_RENDERER", (const char*) version);
+            log.appendString("GL_RENDERER", (const char*) version);
 
             GR_GL_CALL_RET(gl, version, GetString(GR_GL_VENDOR));
-            log->configOption("GL_VENDOR", (const char*) version);
+            log.appendString("GL_VENDOR", (const char*) version);
 
             GR_GL_CALL_RET(gl, version, GetString(GR_GL_SHADING_LANGUAGE_VERSION));
-            log->configOption("GL_SHADING_LANGUAGE_VERSION", (const char*) version);
+            log.appendString("GL_SHADING_LANGUAGE_VERSION", (const char*) version);
         }
     }
 
@@ -1011,23 +1012,27 @@ public:
         return nullptr;
     }
 
-    void fillCurrentOptions(ResultsWriter* log) const {
-        log->configOption("source_type", fSourceType);
-        log->configOption("bench_type",  fBenchType);
+    void fillCurrentOptions(NanoJSONResultsWriter& log) const {
+        log.appendString("source_type", fSourceType);
+        log.appendString("bench_type",  fBenchType);
         if (0 == strcmp(fSourceType, "skp")) {
-            log->configOption("clip",
+            log.appendString("clip",
                     SkStringPrintf("%d %d %d %d", fClip.fLeft, fClip.fTop,
                                                   fClip.fRight, fClip.fBottom).c_str());
             SkASSERT_RELEASE(fCurrentScale < fScales.count());  // debugging paranoia
-            log->configOption("scale", SkStringPrintf("%.2g", fScales[fCurrentScale]).c_str());
+            log.appendString("scale", SkStringPrintf("%.2g", fScales[fCurrentScale]).c_str());
             if (fCurrentUseMPD > 0) {
                 SkASSERT(1 == fCurrentUseMPD || 2 == fCurrentUseMPD);
-                log->configOption("multi_picture_draw", fUseMPDs[fCurrentUseMPD-1] ? "true" : "false");
+                log.appendString("multi_picture_draw",
+                                 fUseMPDs[fCurrentUseMPD-1] ? "true" : "false");
             }
         }
+    }
+
+    void fillCurrentMetrics(NanoJSONResultsWriter& log) const {
         if (0 == strcmp(fBenchType, "recording")) {
-            log->metric("bytes", fSKPBytes);
-            log->metric("ops",   fSKPOps);
+            log.appendMetric("bytes", fSKPBytes);
+            log.appendMetric("ops", fSKPOps);
         }
     }
 
@@ -1124,30 +1129,36 @@ int main(int argc, char** argv) {
         }
     }
 
-    std::unique_ptr<ResultsWriter> log(new ResultsWriter);
+    std::unique_ptr<SkWStream> logStream(new SkNullWStream);
     if (!FLAGS_outResultsFile.isEmpty()) {
 #if defined(SK_RELEASE)
-        log.reset(new NanoJSONResultsWriter(FLAGS_outResultsFile[0]));
+        logStream.reset(new SkFILEWStream(FLAGS_outResultsFile[0]));
 #else
         SkDebugf("I'm ignoring --outResultsFile because this is a Debug build.");
         return 1;
 #endif
     }
+    NanoJSONResultsWriter log(logStream.get(), SkJSONWriter::Mode::kPretty);
+    log.beginObject(); // root
 
     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_properties.count(); i += 2) {
-        log->property(FLAGS_properties[i-1], FLAGS_properties[i]);
+        log.appendString(FLAGS_properties[i-1], FLAGS_properties[i]);
     }
 
     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_key.count(); i += 2) {
-        log->key(FLAGS_key[i-1], FLAGS_key[i]);
+    if (FLAGS_key.count()) {
+        log.beginObject("key");
+        for (int i = 1; i < FLAGS_key.count(); i += 2) {
+            log.appendString(FLAGS_key[i - 1], FLAGS_key[i]);
+        }
+        log.endObject(); // key
     }
 
     const double overhead = estimate_timer_overhead();
@@ -1189,6 +1200,7 @@ int main(int argc, char** argv) {
 
     int runs = 0;
     BenchmarkStream benchStream;
+    log.beginObject("results");
     while (Benchmark* b = benchStream.next()) {
         std::unique_ptr<Benchmark> bench(b);
         if (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getUniqueName())) {
@@ -1196,7 +1208,7 @@ int main(int argc, char** argv) {
         }
 
         if (!configs.empty()) {
-            log->bench(bench->getUniqueName(), bench->getSize().fX, bench->getSize().fY);
+            log.beginBench(bench->getUniqueName(), bench->getSize().fX, bench->getSize().fY);
             bench->delayedSetup();
         }
         for (int i = 0; i < configs.count(); ++i) {
@@ -1280,22 +1292,34 @@ int main(int argc, char** argv) {
             const bool want_plot = !FLAGS_quiet;
 
             Stats stats(samples, want_plot);
-            log->config(config);
-            log->configOption("name", bench->getName());
-            benchStream.fillCurrentOptions(log.get());
-            target->fillOptions(log.get());
-            log->metric("min_ms",    stats.min);
-            log->metrics("samples",    samples);
+            log.beginObject(config);
+
+            log.beginObject("options");
+            log.appendString("name", bench->getName());
+            benchStream.fillCurrentOptions(log);
+            target->fillOptions(log);
+            log.endObject(); // options
+
+            // Metrics
+            log.appendMetric("min_ms", stats.min);
+            log.beginArray("samples");
+            for (double sample : samples) {
+                log.appendDoubleDigits(sample, 16);
+            }
+            log.endArray(); // samples
+            benchStream.fillCurrentMetrics(log);
             if (gpuStatsDump) {
                 // dump to json, only SKPBench currently returns valid keys / values
                 SkASSERT(keys.count() == values.count());
                 for (int i = 0; i < keys.count(); i++) {
-                    log->metric(keys[i].c_str(), values[i]);
+                    log.appendMetric(keys[i].c_str(), values[i]);
                 }
             }
 
+            log.endObject(); // config
+
             if (runs++ % FLAGS_flushEvery == 0) {
-                log->flush();
+                log.flush();
             }
 
             if (kAutoTuneLoops != FLAGS_loops) {
@@ -1360,13 +1384,22 @@ int main(int argc, char** argv) {
             }
             cleanup_run(target);
         }
+        if (!configs.empty()) {
+            log.endBench();
+        }
     }
 
     SkGraphics::PurgeAllCaches();
 
-    log->bench("memory_usage", 0,0);
-    log->config("meta");
-    log->metric("max_rss_mb", sk_tools::getMaxResidentSetSizeMB());
+    log.beginBench("memory_usage", 0, 0);
+    log.beginObject("meta"); // config
+    log.appendS32("max_rss_mb", sk_tools::getMaxResidentSetSizeMB());
+    log.endObject(); // config
+    log.endBench();
+
+    log.endObject(); // results
+    log.endObject(); // root
+    log.flush();
 
     return 0;
 }
diff --git a/bench/nanobench.h b/bench/nanobench.h
index 5b95d59cbf..fe63e04c12 100644
--- a/bench/nanobench.h
+++ b/bench/nanobench.h
@@ -14,9 +14,9 @@
 #include "SkSurface.h"
 #include "SkTypes.h"
 
-class ResultsWriter;
 class SkBitmap;
 class SkCanvas;
+class NanoJSONResultsWriter;
 
 struct Config {
     SkString name;
@@ -68,7 +68,7 @@ struct Target {
     virtual bool capturePixels(SkBitmap* bmp);
 
     /** Writes any config-specific data to the log. */
-    virtual void fillOptions(ResultsWriter*) { }
+    virtual void fillOptions(NanoJSONResultsWriter& log) { }
 
     /** Writes gathered stats using SkDebugf. */
     virtual void dumpStats() {}