Teach `benchmark.mojo` to save the collected trace file to disk. This patch adds `--save-traces` argument to mojo_benchmark which saves the traces collected in each benchmark run to disk. A follow-up patch will need teach the runner to move these files to a better location on the host machine (including from an Android device). Fixes #417. R=qsr@chromium.org Review URL: https://codereview.chromium.org/1347063002 .
diff --git a/apps/benchmark/README.md b/apps/benchmark/README.md index a8f6563..10da93e 100644 --- a/apps/benchmark/README.md +++ b/apps/benchmark/README.md
@@ -8,12 +8,17 @@ ## Arguments -The benchmarking app expects the following arguments: +The benchmarking app **requires** the following arguments: - `--app=<app_url>` - url of the application to be benchmarked - `--duration=<duration_seconds>` - duration of the benchmark in seconds -any other arguments are assumed to be descriptions of measurements to be +The following arguments are **optional**: + + - `--trace-output=<output_file_path>` - local file path at which the collected trace + will be written + +Any other arguments are assumed to be descriptions of measurements to be conducted on the collected trace data. Each measurement has to be of form: `<measurement_type>/<trace_event_category>/<trace_event_name>`. @@ -28,4 +33,4 @@ Devtools offers [a helper script](../../mojo/devtools/common/mojo_benchmark) allowing to run a list of benchmarks in controlled caching conditions, both -on Linux and Android. +on **Android** and **Linux**.
diff --git a/apps/benchmark/benchmark_app.cc b/apps/benchmark/benchmark_app.cc index d7a5b60..66f742b 100644 --- a/apps/benchmark/benchmark_app.cc +++ b/apps/benchmark/benchmark_app.cc
@@ -12,6 +12,7 @@ #include "apps/benchmark/run_args.h" #include "apps/benchmark/trace_collector_client.h" #include "base/bind.h" +#include "base/files/file_util.h" #include "base/macros.h" #include "base/memory/scoped_ptr.h" #include "base/strings/string_split.h" @@ -83,6 +84,17 @@ // TraceCollectorClient::Receiver: void OnTraceCollected(std::string trace_data) override { + if (args_.write_output_file) { + // Write the trace file regardless of whether it can be parsed (or whether + // the measurements succeed), as it can be useful to debug failures. + base::File trace_file( + args_.output_file_path, + base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); + trace_file.WriteAtCurrentPos(trace_data.data(), trace_data.size()); + printf("wrote trace file at: %s\n", + args_.output_file_path.value().c_str()); + } + // Parse trace events. std::vector<Event> events; if (!GetEvents(trace_data, &events)) {
diff --git a/apps/benchmark/run_args.cc b/apps/benchmark/run_args.cc index 1791470..aa21f9f 100644 --- a/apps/benchmark/run_args.cc +++ b/apps/benchmark/run_args.cc
@@ -68,6 +68,13 @@ } result->duration = base::TimeDelta::FromSeconds(duration_int); + result->write_output_file = false; + if (command_line.HasSwitch("trace-output")) { + result->write_output_file = true; + result->output_file_path = + base::FilePath(command_line.GetSwitchValueASCII("trace-output")); + } + // All regular arguments (not switches, ie. not preceded by "--") describe // measurements. for (const std::string& measurement_spec : command_line.GetArgs()) {
diff --git a/apps/benchmark/run_args.h b/apps/benchmark/run_args.h index a9ada52..73e83d6 100644 --- a/apps/benchmark/run_args.h +++ b/apps/benchmark/run_args.h
@@ -9,6 +9,7 @@ #include <vector> #include "apps/benchmark/measurements.h" +#include "base/files/file_path.h" #include "base/time/time.h" namespace benchmark { @@ -18,6 +19,8 @@ std::string app; base::TimeDelta duration; std::vector<Measurement> measurements; + bool write_output_file; + base::FilePath output_file_path; RunArgs(); ~RunArgs();
diff --git a/mojo/devtools/common/mojo_benchmark b/mojo/devtools/common/mojo_benchmark index 756b44b..a8191f5 100755 --- a/mojo/devtools/common/mojo_benchmark +++ b/mojo/devtools/common/mojo_benchmark
@@ -8,6 +8,7 @@ import argparse import logging import sys +import time from devtoolslib import shell_arguments from devtoolslib import shell_config @@ -52,7 +53,7 @@ def _run_benchmark(shell, shell_args, name, app, duration_seconds, measurements, - cold_start, verbose): + cold_start, verbose, save_traces): """Runs `benchmark.mojo` in shell with correct arguments, parses and presents the benchmark results. """ @@ -60,6 +61,13 @@ benchmark_args = [] benchmark_args.append('--app=' + app) benchmark_args.append('--duration=' + str(duration_seconds)) + if save_traces: + trace_output_file = 'benchmark-%s-%s-%s.trace' % ( + name.replace(' ', '_'), + 'cold_start' if cold_start else 'warm_start', + time.strftime('%Y%m%d%H%M%S')) + benchmark_args.append('--trace-output=' + trace_output_file) + for measurement in measurements: benchmark_args.append(measurement) @@ -100,6 +108,8 @@ description=_DESCRIPTION) parser.add_argument('benchmark_list_file', type=file, help='a file listing benchmarks to run') + parser.add_argument('--save-traces', action='store_true', + help='save the traces produced by benchmarks to disk') # Common shell configuration arguments. shell_config.add_shell_arguments(parser) @@ -125,9 +135,11 @@ shell_args = benchmark_spec.get('shell-args', []) + common_shell_args measurements = benchmark_spec['measurements'] _run_benchmark(shell, shell_args, name, app, duration, measurements, - cold_start=True, verbose=script_args.verbose) + cold_start=True, verbose=script_args.verbose, + save_traces=script_args.save_traces) _run_benchmark(shell, shell_args, name, app, duration, measurements, - cold_start=False, verbose=script_args.verbose) + cold_start=False, verbose=script_args.verbose, + save_traces=script_args.save_traces) return 0 if succeeded else 1