Adding CPU performance test for Youtube video

- Adding CPU performance test to calculate extrapolation of the utilization to play the test video.
- Modify existing performance test by first buffer 100% video, ignore first 10 seconds to get rid of initial dropped frames noise. Then test for next 60 seconds.
- Replaced the test video to Transformers 3 high-motion trailer for better testing.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110117

[ilja, 2011-11-14 05:14:06]

LGTM for formulas with nit.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py
File functional/perf.py (right):

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode558
functional/perf.py:558: """Measure the Youtube video CPU usage. Runs for 60
seconds."""
Measures the Youtube video CPU usage (between 0 and 1) extrapolated to total
frames in the video by taking dropped frames into account. For smooth video
playback this number should be < 0.5..0.6 on a hyperthreaded CPU. Runs for 60
seconds.

[krisr, 2011-11-15 00:42:42]

lgtm

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py
File functional/perf.py (right):

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode540
functional/perf.py:540: for _ in xrange(10):
can we replace this with sleep(10)?

[Nirnimesh, 2011-11-15 02:13:49]

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py
File functional/perf.py (right):

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode502
functional/perf.py:502: """Returns the CPU usage"""
Mention that it get's machine's CPU usage (as opposed to a process's cpu usage)
Mention that it uses /proc/stat

Include a sample dictionary that'll be returned.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode503
functional/perf.py:503: cpu = commands.getoutput("grep \"cpu  \"
/proc/stat").split()
use ' for the outer quote.
Then you don't need to escape the inner "

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode503
functional/perf.py:503: cpu = commands.getoutput("grep \"cpu  \"
/proc/stat").split()
Why grep? Why not read the first line (in python)

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode504
functional/perf.py:504: dict = {'user':int(cpu[1]), 'nice':int(cpu[2]),
'system':int(cpu[3]),
nit: put space after :

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode506
functional/perf.py:506: return dict
merge with previous line. Get rid of |dict|

return {
    'user': int(cpu[1]),
    'nice': int(cpu[2]),
...
}

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode540
functional/perf.py:540: for _ in xrange(10):
What's this sleep for? Add comments

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode565
functional/perf.py:565: time.sleep(1)
mention why you're sleeping.

# Play the video for some time.

http://codereview.chromium.org/8548002/diff/2001/functional/youtube.py
File functional/youtube.py (right):

http://codereview.chromium.org/8548002/diff/2001/functional/youtube.py#newcode50
functional/youtube.py:50: """Returns video info"""
What info? Include sample.

http://codereview.chromium.org/8548002/diff/2001/functional/youtube.py#newcode55
functional/youtube.py:55: return video_data
merge with previous line

http://codereview.chromium.org/8548002/diff/2001/functional/youtube.py#newcode71
functional/youtube.py:71: """Returns Youtube video frames/second
end with . for consistency

[rohitbm, 2011-11-15 03:10:50]

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py
File functional/perf.py (right):

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode502
functional/perf.py:502: """Returns the CPU usage"""
On 2011/11/15 02:13:50, Nirnimesh wrote:
> Mention that it get's machine's CPU usage (as opposed to a process's cpu
usage)
> Mention that it uses /proc/stat
> 
> Include a sample dictionary that'll be returned.

Done.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode503
functional/perf.py:503: cpu = commands.getoutput("grep \"cpu  \"
/proc/stat").split()
On 2011/11/15 02:13:50, Nirnimesh wrote:
> use ' for the outer quote.
> Then you don't need to escape the inner "
Reading /proc/stat file.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode503
functional/perf.py:503: cpu = commands.getoutput("grep \"cpu  \"
/proc/stat").split()
On 2011/11/15 02:13:50, Nirnimesh wrote:
> use ' for the outer quote.
> Then you don't need to escape the inner "

Done.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode504
functional/perf.py:504: dict = {'user':int(cpu[1]), 'nice':int(cpu[2]),
'system':int(cpu[3]),
On 2011/11/15 02:13:50, Nirnimesh wrote:
> nit: put space after :

Done.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode506
functional/perf.py:506: return dict
On 2011/11/15 02:13:50, Nirnimesh wrote:
> merge with previous line. Get rid of |dict|
> 
> return {
>     'user': int(cpu[1]),
>     'nice': int(cpu[2]),
> ...
> }

Done.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode540
functional/perf.py:540: for _ in xrange(10):
On 2011/11/15 00:42:42, krisr wrote:
> can we replace this with sleep(10)?

Done.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode540
functional/perf.py:540: for _ in xrange(10):
On 2011/11/15 02:13:50, Nirnimesh wrote:
> What's this sleep for? Add comments

Done.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode558
functional/perf.py:558: """Measure the Youtube video CPU usage. Runs for 60
seconds."""
On 2011/11/14 05:14:07, ilja wrote:
> Measures the Youtube video CPU usage (between 0 and 1) extrapolated to total
> frames in the video by taking dropped frames into account. For smooth video
> playback this number should be < 0.5..0.6 on a hyperthreaded CPU. Runs for 60
> seconds.

Done. We found that few high motion videos's performance stand between 0.5..1.0,
so mention 1.0 in the comment.

http://codereview.chromium.org/8548002/diff/2001/functional/perf.py#newcode565
functional/perf.py:565: time.sleep(1)
On 2011/11/15 02:13:50, Nirnimesh wrote:
> mention why you're sleeping.
> 
> # Play the video for some time.

Done.

http://codereview.chromium.org/8548002/diff/2001/functional/youtube.py
File functional/youtube.py (right):

http://codereview.chromium.org/8548002/diff/2001/functional/youtube.py#newcode50
functional/youtube.py:50: """Returns video info"""
On 2011/11/15 02:13:50, Nirnimesh wrote:
> What info? Include sample.
This API is undocumented, so do we want to include sample?

http://codereview.chromium.org/8548002/diff/2001/functional/youtube.py#newcode55
functional/youtube.py:55: return video_data
On 2011/11/15 02:13:50, Nirnimesh wrote:
> merge with previous line

Done.

http://codereview.chromium.org/8548002/diff/2001/functional/youtube.py#newcode71
functional/youtube.py:71: """Returns Youtube video frames/second
On 2011/11/15 02:13:50, Nirnimesh wrote:
> end with . for consistency

Done.

[Nirnimesh, 2011-11-15 08:11:29]

LGTM

http://codereview.chromium.org/8548002/diff/12002/functional/perf.py
File functional/perf.py (right):

http://codereview.chromium.org/8548002/diff/12002/functional/perf.py#newcode516
functional/perf.py:516: cpu = file.readline().split()
merge with previous line. you don't need the |file| variable
(BTW |file| is a keyword in python anyway)