Adding the performance and browser benchmarking script.

Adding the performance and browser benchmarking script.
Committed: https://code.google.com/p/dart/source/detail?r=1914

[Emily Fortuna, 2011-11-23 03:08:02]

First stab at adding my benchmarking script so that I can test it on the
performance buildbots. I expect it may still need some refactoring (and this
script will change substantially once it uses the Charts API), so your general
comments are appreciated.

[Siggi Cherem (dart-lang), 2011-11-23 20:00:29]

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
File tools/testing/perf_testing/create_graph.py (right):

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:18: """This script is run hourly to
track performance and correctness progress of
could we trigger it on each revision change?

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:24: DART_INSTALL_LOCATION =
'/Users/efortuna/perfChart/dart'
make it independent - you can derive it from the currently file path (__file__)

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:30: FROG_MEAN = 'frog Mean'
(nit) I'd move all these constants to the top level, make failed_once a field
initialized in __init__ like all other fields.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:48: def
style_and_save_perf_plot(self, chart_title, y_axis_label, size_x, size_y,
(nit): we tend to use a CamelCase style for functions

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:70: @staticmethod
you can also move this definition to the top-level. See 'Execute' in many of the
scripts in testing/...

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:92: os.remove(os.getcwd() +
os.path.join('frog', 'frogsh'))
or 'svn revert'?

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:128: dir_path =
os.path.join(TestRunner.DART_INSTALL_LOCATION, 'tools', 'testing',
80

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:215: TestRunner.__init__(self,
result_folder_name)
use 'super'? (there are some examples in architecture.py, you might need to make
the baseclass (TestRunner) extend from 'object')

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:241: j += 1
what if you have > than 21 benchmarks?

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:242: i=0
Alternatively you could write the index calculations more declaratively:

for i in range(len(self.benchmarks)):

  bench_style[benchmark] = (markers[i % len(markers)], mfc[(i / len(markers)) %
len(mfc])]

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:245: style = bench_style[benchmark]
instead of using style[0] below, you could also use the tuple deconstructor
here:

bench_marker, bench_mfc = bench_style[benchmark]

http://codereview.chromium.org/8670013/diff/1/tools/testing/run_selenium.py
File tools/testing/run_selenium.py (right):

http://codereview.chromium.org/8670013/diff/1/tools/testing/run_selenium.py#n...
tools/testing/run_selenium.py:14: sys.argv[4] = is_perf_test -- a boolean
indicating if this test is a
you might want to use the [optparse] package and make all these parameters into
options, that will make the command on architecture.py easier to understand.

http://codereview.chromium.org/8670013/diff/1/tools/testing/run_selenium.py#n...
tools/testing/run_selenium.py:24: """Tests if the performance test has
completed."""
Tests -> Checks :)

http://codereview.chromium.org/8670013/diff/1/tools/testing/run_selenium.py#n...
tools/testing/run_selenium.py:26: # python webdriver bindings if you call
driver.get_element_by_id.
strange - remind me to chat about this in person :)

[Emily Fortuna, 2011-11-30 00:37:13]

Take another look at this. It has been substantially cleaned up.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
File tools/testing/perf_testing/create_graph.py (right):

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:18: """This script is run hourly to
track performance and correctness progress of
On 2011/11/23 20:00:29, sigmund wrote:
> could we trigger it on each revision change?

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:24: DART_INSTALL_LOCATION =
'/Users/efortuna/perfChart/dart'
On 2011/11/23 20:00:29, sigmund wrote:
> make it independent - you can derive it from the currently file path
(__file__)

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:30: FROG_MEAN = 'frog Mean'
On 2011/11/23 20:00:29, sigmund wrote:
> (nit) I'd move all these constants to the top level, make failed_once a field
> initialized in __init__ like all other fields.

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:48: def
style_and_save_perf_plot(self, chart_title, y_axis_label, size_x, size_y,
On 2011/11/23 20:00:29, sigmund wrote:
> (nit): we tend to use a CamelCase style for functions

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:70: @staticmethod
On 2011/11/23 20:00:29, sigmund wrote:
> you can also move this definition to the top-level. See 'Execute' in many of
the
> scripts in testing/...

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:92: os.remove(os.getcwd() +
os.path.join('frog', 'frogsh'))
On 2011/11/23 20:00:29, sigmund wrote:
> or 'svn revert'?

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:128: dir_path =
os.path.join(TestRunner.DART_INSTALL_LOCATION, 'tools', 'testing',
On 2011/11/23 20:00:29, sigmund wrote:
> 80

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:215: TestRunner.__init__(self,
result_folder_name)
On 2011/11/23 20:00:29, sigmund wrote:
> use 'super'? (there are some examples in architecture.py, you might need to
make
> the baseclass (TestRunner) extend from 'object')

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:241: j += 1
On 2011/11/23 20:00:29, sigmund wrote:
> what if you have > than 21 benchmarks?

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:242: i=0
On 2011/11/23 20:00:29, sigmund wrote:
> Alternatively you could write the index calculations more declaratively:
> 
> for i in range(len(self.benchmarks)):
> 
>   bench_style[benchmark] = (markers[i % len(markers)], mfc[(i / len(markers))
%
> len(mfc])]

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/perf_testing/crea...
tools/testing/perf_testing/create_graph.py:245: style = bench_style[benchmark]
On 2011/11/23 20:00:29, sigmund wrote:
> instead of using style[0] below, you could also use the tuple deconstructor
> here:
> 
> bench_marker, bench_mfc = bench_style[benchmark]

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/run_selenium.py
File tools/testing/run_selenium.py (right):

http://codereview.chromium.org/8670013/diff/1/tools/testing/run_selenium.py#n...
tools/testing/run_selenium.py:14: sys.argv[4] = is_perf_test -- a boolean
indicating if this test is a
On 2011/11/23 20:00:29, sigmund wrote:
> you might want to use the [optparse] package and make all these parameters
into
> options, that will make the command on architecture.py easier to understand.

Done.

http://codereview.chromium.org/8670013/diff/1/tools/testing/run_selenium.py#n...
tools/testing/run_selenium.py:24: """Tests if the performance test has
completed."""
On 2011/11/23 20:00:29, sigmund wrote:
> Tests -> Checks :)

Done.

[Siggi Cherem (dart-lang), 2011-11-30 01:40:21]

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
File tools/testing/perf_testing/create_graph.py (right):

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:48: print line,
you probably want the runcmd to wait until the command is done?

you can do:

output, error = p.communicate();
print output

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:152: def getColor(self):
getColor => GetColor

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:156: self.color_index = 0
how about:

self.color_index = (self.color_index + 1) % len(COLORS)

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:160: #TODO(efortuna)
include explanation?

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:513: def TimeCmd(self, cmd):
this can also be at the top-level

[Emily Fortuna, 2011-11-30 01:57:46]

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
File tools/testing/perf_testing/create_graph.py (right):

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:48: print line,
On 2011/11/30 01:40:21, sigmund wrote:
> you probably want the runcmd to wait until the command is done?
> 
> you can do:
> 
> output, error = p.communicate();
> print output

Done.

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:152: def getColor(self):
On 2011/11/30 01:40:21, sigmund wrote:
> getColor => GetColor

Done.

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:156: self.color_index = 0
On 2011/11/30 01:40:21, sigmund wrote:
> how about:
> 
> self.color_index = (self.color_index + 1) % len(COLORS)

Done.

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:160: #TODO(efortuna)
On 2011/11/30 01:40:21, sigmund wrote:
> include explanation?

Done.

http://codereview.chromium.org/8670013/diff/10001/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:513: def TimeCmd(self, cmd):
On 2011/11/30 01:40:21, sigmund wrote:
> this can also be at the top-level

Done.

[Siggi Cherem (dart-lang), 2011-11-30 02:17:30]

lgtm

http://codereview.chromium.org/8670013/diff/11004/tools/testing/perf_testing/...
File tools/testing/perf_testing/create_graph.py (right):

http://codereview.chromium.org/8670013/diff/11004/tools/testing/perf_testing/...
tools/testing/perf_testing/create_graph.py:46: for line in output:
double check if this works