Change JSON output of nanobench.

bench/nanobench.cpp

 /*
  * Copyright 2014 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include <ctype.h>
 
 #include "Benchmark.h"
 #include "CrashHandler.h"
 #include "ResultsWriter.h"
 #include "Stats.h"
 #include "Timer.h"
 
 #include "SkCanvas.h"
 #include "SkCommandLineFlags.h"
 #include "SkForceLinking.h"
 #include "SkGraphics.h"
 #include "SkString.h"
 #include "SkSurface.h"
 
 #if SK_SUPPORT_GPU
+    #include "gl/GrGLDefines.h"
     #include "GrContextFactory.h"
     GrContextFactory gGrFactory;
 #endif
 
 __SK_FORCE_IMAGE_DECODER_LINKING;
 
 #if SK_DEBUG
     DEFINE_bool(runOnce, true, "Run each benchmark just once?");
 #else
     DEFINE_bool(runOnce, false, "Run each benchmark just once?");
@@ skipping 11 matching lines @@
 DEFINE_double(gpuMs, 5, "Target bench time in millseconds for GPU.");
 DEFINE_int32(gpuFrameLag, 5, "Overestimate of maximum number of frames GPU allows to lag.");
 
 DEFINE_bool(cpu, true, "Master switch for CPU-bound work.");
 DEFINE_bool(gpu, true, "Master switch for GPU-bound work.");
 
 DEFINE_string(outResultsFile, "", "If given, write results here as JSON.");
 DEFINE_bool(resetGpuContext, true, "Reset the GrContext before running each bench.");
 DEFINE_int32(maxCalibrationAttempts, 3,
              "Try up to this many times to guess loops for a bench, or skip the bench.");
+DEFINE_string(key, "", "Space-separated key/value pairs to add to JSON.");
+DEFINE_string(gitHash, "", "Git hash to add to JSON.");
 
 
 static SkString humanize(double ms) {
     if (ms > 1e+3) return SkStringPrintf("%.3gs",  ms/1e3);
     if (ms < 1e-3) return SkStringPrintf("%.3gns", ms*1e6);
 #ifdef SK_BUILD_FOR_WIN
     if (ms < 1)    return SkStringPrintf("%.3gus", ms*1e3);
 #else
     if (ms < 1)    return SkStringPrintf("%.3gµs", ms*1e3);
 #endif
@@ skipping 176 matching lines @@
         GPU_TARGET(nvprmsaa16, GrContextFactory::kNVPR_GLContextType, _8888, 16)
         GPU_TARGET(debug,     GrContextFactory::kDebug_GLContextType, _8888, 0)
         GPU_TARGET(nullgpu,    GrContextFactory::kNull_GLContextType, _8888, 0)
         #if SK_ANGLE
             GPU_TARGET(angle, GrContextFactory::kANGLE_GLContextType, _8888, 0)
         #endif
     }
 #endif
 }
 
 static void fill_static_options(ResultsWriter* log) {

[mtklein, 2014/07/17 13:41:16]

Let's merge fill_static_options and fill_gpu_options into just one fill_options
with a #if-#endif section embedded?

This is also starting to look like a piece of code that'd be useful for other
test programs.  Let's keep this chunk in mind as something to extract later?

[jcgregorio, 2014/07/17 19:13:22]

Actually had to keep fill_gpu_options separate because it's now calling
log->configOption and not just log->option since the values may change per
config.

On 2014/07/17 13:41:16, mtklein wrote:

 #if defined(SK_BUILD_FOR_WIN32)

[jcgregorio, 2014/07/17 13:11:51]

Are there any other values locked up in #defines that would be useful to mirror
into the perf data?

[mtklein, 2014/07/17 13:41:16]

We could probably also provide the CPU architecture (x86, x864-64, ARM v7, ARm
v8).

[jcgregorio, 2014/07/17 19:13:22]

Those should come in via --key from the buildbot information, unless there's
finer grained values we can record here?

On 2014/07/17 13:41:16, mtklein wrote:

[mtklein, 2014/07/17 19:21:36]

We could go arbitrarily fine... e.g., dump /proc/cpuinfo, but in practice we
don't really care any deeper than what comes from the buildbot.

Can't OS come from there too?

[jcgregorio, 2014/07/17 19:32:46]

It does come from there, but it's not clean, it's the value in the buildbot
name, so not just MAC, but Mac10.6, Mac10.7 or Mac10.8. What we record here is
just MAC, so this will give us two ways to filter the data.

On 2014/07/17 19:21:36, mtklein wrote:

     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 defined(SK_DEBUG)
-    log->option("build", "DEBUG");
-#else
-    log->option("build", "RELEASE");
+}
+
+#if SK_SUPPORT_GPU
+static void fill_gpu_options(ResultsWriter* log) {
+    if (!FLAGS_gpu) {
+        return;
+    }
+
+    // Can't get a SkGLContextHelper until we've created a context of that type.
+    gGrFactory.get(GrContextFactory::kNative_GLContextType);

[bsalomon, 2014/07/17 13:35:04]

We don't always want to use the kNative context. We will be running nanobench
with different contexts and we'll want the context info for each. You should
assume that each config we run *may* be using a different context.

Also if we're going to create and destroy the contexts here we don't really need
to use the global GrContextFactory, you can just put one on the stack. It will
destroy its contexts in its destructor.

[jcgregorio, 2014/07/17 19:13:22]

OK, changed it so that these are recorded per config, and use the
SkGLContextHelper that's in Tasks so I don't need to construct or destroy them.

On 2014/07/17 13:35:04, bsalomon wrote:

+    SkGLContextHelper* ctx = gGrFactory.getGLContext(GrContextFactory::kNative_GLContextType);
+    if (NULL != ctx) {
+        const GLubyte* version;
+        SK_GL_RET(*ctx, version, GetString(GR_GL_VERSION));
+        log->option("GL_VERSION", (const char*)(version));
+
+        SK_GL_RET(*ctx, version, GetString(GR_GL_RENDERER));
+        log->option("GL_RENDERER", (const char*) version);
+
+        SK_GL_RET(*ctx, version, GetString(GR_GL_VENDOR));
+        log->option("GL_VENDOR", (const char*) version);
+
+        SK_GL_RET(*ctx, version, GetString(GR_GL_SHADING_LANGUAGE_VERSION));
+        log->option("GL_SHADING_LANGUAGE_VERSION", (const char*) version);
+    }
+    gGrFactory.destroyContexts();
+}
 #endif
-}
 
 int tool_main(int argc, char** argv);
 int tool_main(int argc, char** argv) {
     SetupCrashHandler();
     SkAutoGraphics ag;
     SkCommandLineFlags::Parse(argc, argv);
 
     if (FLAGS_runOnce) {
         FLAGS_samples     = 1;
         FLAGS_gpuFrameLag = 0;
     }
 
     MultiResultsWriter log;
-    SkAutoTDelete<JSONResultsWriter> json;
+    SkAutoTDelete<NanoJSONResultsWriter> json;
     if (!FLAGS_outResultsFile.isEmpty()) {
-        json.reset(SkNEW(JSONResultsWriter(FLAGS_outResultsFile[0])));
+        const char* gitHash = FLAGS_gitHash.isEmpty() ? "" : FLAGS_gitHash[0];

[mtklein, 2014/07/17 13:41:16]

"" -> "unknown-revision" perhaps?

[jcgregorio, 2014/07/17 19:13:22]

Done.

+        json.reset(SkNEW(NanoJSONResultsWriter(FLAGS_outResultsFile[0], gitHash)));
         log.add(json.get());
     }
     CallEnd<MultiResultsWriter> ender(log);

[mtklein, 2014/07/17 13:41:16]

One of log.end() or this line can go away right?  I'm happy either way.

[jcgregorio, 2014/07/17 19:13:22]

Done.

+
+    if (1 == FLAGS_key.count() % 2) {
+        SkDebugf("ERROR: --key must be passed with an even number of arguments.\n");

[mtklein, 2014/07/17 13:41:16]

To be fair, you've written this in a way that we could downgrade this to 

"WARNING: last value of --key will be ignored"

But I do like fail fast.

[jcgregorio, 2014/07/17 19:13:22]

Acknowledged.

+        return 1;
+    }
+    for (int i = 1; i < FLAGS_key.count(); i += 2) {
+        log.key(FLAGS_key[i-1], FLAGS_key[i]);
+    }
     fill_static_options(&log);
 
+#if SK_SUPPORT_GPU
+    fill_gpu_options(&log);
+#endif
+
     const double overhead = estimate_timer_overhead();
     SkAutoTMalloc<double> samples(FLAGS_samples);
 
     if (FLAGS_runOnce) {
         SkDebugf("--runOnce is true; times would only be misleading so we won't print them.\n");
     } else if (FLAGS_verbose) {
         // No header.
     } else if (FLAGS_quiet) {
         SkDebugf("median\tbench\tconfig\n");
     } else {
         SkDebugf("loops\tmin\tmedian\tmean\tmax\tstddev\tsamples\tconfig\tbench\n");
     }
 
     for (const BenchRegistry* r = BenchRegistry::Head(); r != NULL; r = r->next()) {
         SkAutoTDelete<Benchmark> bench(r->factory()(NULL));
         if (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getName())) {
             continue;
         }
-        log.bench(bench->getName(), bench->getSize().fX, bench->getSize().fY);
+        bool loggedBench = false;
 
         SkTDArray<Target*> targets;
         create_targets(bench.get(), &targets);

[mtklein, 2014/07/17 13:41:16]

Ah, gotcha.  Let's do it here as

if (!targets.isEmpty()) {
  log.bench(...);
  bench->preDraw();
}
for (int j = 0; j < targets.count(); j++) {
  ...
}

or, just a little less desirably, as

for (int j = 0; j < targets.count(); j++) {
  if (0 == j) { ... }
  ...
}

Having loggedBench stand alone as a bool had me looking around for the
complicated logic that requires it to exist.  Seems like we can write the
condition more precisely without it.

[jcgregorio, 2014/07/17 19:13:23]

Done.

 
         bench->preDraw();
         for (int j = 0; j < targets.count(); j++) {
             SkCanvas* canvas = targets[j]->surface.get() ? targets[j]->surface->getCanvas() : NULL;
             const char* config = targets[j]->config;
 
             const int loops =
 #if SK_SUPPORT_GPU
                 Benchmark::kGPU_Backend == targets[j]->backend
                 ? gpu_bench(targets[j]->gl, bench.get(), canvas, samples.get())
                 :
 #endif
                  cpu_bench(       overhead, bench.get(), canvas, samples.get());
 
             if (loops == 0) {
                 SkDebugf("Unable to time %s\t%s (overhead %s)\n",
                          bench->getName(), config, humanize(overhead).c_str());
                 continue;
             }
+            if (!loggedBench) {
+                log.bench(bench->getName(), bench->getSize().fX, bench->getSize().fY);
+                loggedBench = true;
+            }
 
             Stats stats(samples.get(), FLAGS_samples);
             log.config(config);
             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 (FLAGS_runOnce) {
@@ skipping 27 matching lines @@
             }
         }
         targets.deleteAll();
 
     #if SK_SUPPORT_GPU
         if (FLAGS_resetGpuContext) {
             gGrFactory.destroyContexts();
         }
     #endif
     }
+    log.end();
 
     return 0;
 }
 
 #if !defined SK_BUILD_FOR_IOS
 int main(int argc, char * const argv[]) {
     return tool_main(argc, (char**) argv);
 }
 #endif