Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(426)

Issue 11779041: [telemetry] Add support for BenchmarkResults that vary from page to page (Closed)

Created:
7 years, 11 months ago by nduca
Modified:
7 years, 11 months ago
Reviewers:
tonyg, hartmanng
CC:
chromium-reviews, chrome-speed-team+watch_google.com, pam+watch_chromium.org, telemetry+watch_chromium.org
Visibility:
Public.

Description

[telemetry] Add support for BenchmarkResults that vary from page to page This patch started out trying to add support for benchmarks that had different outputs for each page. This is required for benchmarks that report [filtered] profiling data, for instance something that simply groups timeline data and reports it. To do csv output in taht situation, we have to hold output until the end of the benchmark. Making this work cleanly in the multi page benchmark code turned out to be easily hacked, but unearthed a pretty hefty amount of deep, branchy code. So, to make this cleaner, I cleaned up the results object. We now have: - PageBenchmarkResults: all results for the benchmark - ValuesForSinglePage: all the results for a page - PageBenchmarkValues: a single value from a benchmark PerfBot benchmark output is managed by PageBenchmarkResults. Csv and Block terminal-block was renamed to block, but with backcompat to terminal-block. Nobody could remember terminal-block. R=tonyg,hartmanng NOTRY=True Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175602

Patch Set 1 #

Patch Set 2 : bugfix #

Total comments: 1

Patch Set 3 : nitfix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+597 lines, -163 lines) Patch
A tools/telemetry/telemetry/block_page_benchmark_results.py View 1 chunk +32 lines, -0 lines 0 comments Download
A tools/telemetry/telemetry/block_page_benchmark_results_unittest.py View 1 chunk +63 lines, -0 lines 0 comments Download
A tools/telemetry/telemetry/csv_page_benchmark_results.py View 1 chunk +79 lines, -0 lines 0 comments Download
A tools/telemetry/telemetry/csv_page_benchmark_results_unittest.py View 1 chunk +106 lines, -0 lines 0 comments Download
M tools/telemetry/telemetry/multi_page_benchmark.py View 2 chunks +10 lines, -156 lines 0 comments Download
M tools/telemetry/telemetry/multi_page_benchmark_runner.py View 1 2 3 chunks +9 lines, -5 lines 0 comments Download
A tools/telemetry/telemetry/page_benchmark_results.py View 1 chunk +128 lines, -0 lines 0 comments Download
A tools/telemetry/telemetry/page_benchmark_results_unittest.py View 1 chunk +107 lines, -0 lines 0 comments Download
A tools/telemetry/telemetry/page_benchmark_value.py View 1 chunk +32 lines, -0 lines 0 comments Download
M tools/telemetry/telemetry/page_runner.py View 1 chunk +3 lines, -1 line 0 comments Download
A tools/telemetry/telemetry/perf_tests_helper.py View 1 1 chunk +25 lines, -0 lines 0 comments Download
M tools/telemetry/telemetry/websocket.py View 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
nduca
7 years, 11 months ago (2013-01-08 12:37:25 UTC) #1
hartmanng
LGTM with nit https://codereview.chromium.org/11779041/diff/3001/tools/telemetry/telemetry/multi_page_benchmark_runner.py File tools/telemetry/telemetry/multi_page_benchmark_runner.py (right): https://codereview.chromium.org/11779041/diff/3001/tools/telemetry/telemetry/multi_page_benchmark_runner.py#newcode18 tools/telemetry/telemetry/multi_page_benchmark_runner.py:18: from telemetry import block_page_benchmark_results The rest ...
7 years, 11 months ago (2013-01-08 16:04:10 UTC) #2
tonyg
LGTM too. Beautiful cleanup!
7 years, 11 months ago (2013-01-08 17:07:08 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nduca@chromium.org/11779041/5001
7 years, 11 months ago (2013-01-08 21:14:56 UTC) #4
commit-bot: I haz the power
7 years, 11 months ago (2013-01-08 22:18:48 UTC) #5
Message was sent while issue was closed.
Change committed as 175602

Powered by Google App Engine
This is Rietveld 408576698