Issue 15463003: [Telemetry] Utilize CreatePageSet for benchmarks with one page set.

Issue 15463003: [Telemetry] Utilize CreatePageSet for benchmarks with one page set. (Closed)

Created:
7 years, 7 months ago by tonyg

Modified:
7 years, 7 months ago

Reviewers:
dtu, nduca

CC:
chromium-reviews, chrome-speed-team+watch_google.com

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

[Telemetry] Utilize CreatePageSet for benchmarks with one page set. BUG=None TEST=Ran all modified benchmarks locally on linux NOTRY=True R=dtu@chromium.org, nduca@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201158

Patch Set 2 : Make file_path required #

Created: 7 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+100 lines, -59 lines)			Patch
D	tools/perf/page_sets/dom_perf.json	View		1 chunk	+0 lines, -15 lines	0 comments	Download
D	tools/perf/page_sets/jsgamebench.json	View		1 chunk	+0 lines, -7 lines	0 comments	Download
D	tools/perf/page_sets/kraken.json	View		1 chunk	+0 lines, -7 lines	0 comments	Download
D	tools/perf/page_sets/octane.json	View		1 chunk	+0 lines, -6 lines	0 comments	Download
D	tools/perf/page_sets/robohornetpro.json	View		1 chunk	+0 lines, -7 lines	0 comments	Download
D	tools/perf/page_sets/spaceport.json	View		1 chunk	+0 lines, -6 lines	0 comments	Download
D	tools/perf/page_sets/sunspider.json	View		1 chunk	+0 lines, -7 lines	0 comments	Download
M	tools/perf/perf_tools/dom_perf.py	View	1	2 chunks	+19 lines, -0 lines	0 comments	Download
M	tools/perf/perf_tools/jsgamebench.py	View	1	1 chunk	+13 lines, -0 lines	0 comments	Download
M	tools/perf/perf_tools/kraken.py	View	1	1 chunk	+13 lines, -0 lines	0 comments	Download
M	tools/perf/perf_tools/octane.py	View	1	1 chunk	+12 lines, -0 lines	0 comments	Download
M	tools/perf/perf_tools/robohornetpro.py	View	1	1 chunk	+15 lines, -0 lines	0 comments	Download
M	tools/perf/perf_tools/spaceport.py	View	1	1 chunk	+12 lines, -0 lines	0 comments	Download
M	tools/perf/perf_tools/sunspider.py	View	1	1 chunk	+13 lines, -1 line	0 comments	Download
M	tools/telemetry/telemetry/page/page_set.py	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	tools/telemetry/telemetry/page/page_test_runner.py	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

nduca

lgtm with side comment https://codereview.chromium.org/15463003/diff/1/tools/perf/perf_tools/octane.py File tools/perf/perf_tools/octane.py (right): https://codereview.chromium.org/15463003/diff/1/tools/perf/perf_tools/octane.py#newcode13 tools/perf/perf_tools/octane.py:13: return page_set.PageSet.FromDict({ should we modify ...

7 years, 7 months ago (2013-05-20 18:34:02 UTC) #2

dtu

lgtm https://codereview.chromium.org/15463003/diff/1/tools/perf/perf_tools/octane.py File tools/perf/perf_tools/octane.py (right): https://codereview.chromium.org/15463003/diff/1/tools/perf/perf_tools/octane.py#newcode13 tools/perf/perf_tools/octane.py:13: return page_set.PageSet.FromDict({ On 2013/05/20 18:34:02, nduca wrote: > ...

7 years, 7 months ago (2013-05-20 18:44:31 UTC) #3

tonyg

Committed patchset #2 manually as r201158 (presubmit successful).

7 years, 7 months ago (2013-05-20 22:55:18 UTC) #4

tonyg

7 years, 7 months ago (2013-05-20 22:56:37 UTC) #5

Message was sent while issue was closed.

On 2013/05/20 18:44:31, Dave Tu wrote:
> lgtm
> 
>
https://codereview.chromium.org/15463003/diff/1/tools/perf/perf_tools/octane.py
> File tools/perf/perf_tools/octane.py (right):
> 
>
https://codereview.chromium.org/15463003/diff/1/tools/perf/perf_tools/octane....
> tools/perf/perf_tools/octane.py:13: return page_set.PageSet.FromDict({
> On 2013/05/20 18:34:02, nduca wrote:
> > should we modify the page_runner to explode if the page set returned from
> > CreatePageSet doesn't have a file_path? Its optional now, but it seems
pretty
> > risky if its not given...
> 
> Just make the parameter not-optional in PageSet. I don't think there's really
a
> valid case where it's left out.

Good idea. Done.

Expand Messages | Collapse Messages