[test] Add an option to the perf runner to support running with the internal profiler.

[test] Add an option to the perf runner to support running with the internal profiler.

Enhance the perf runner to run with the profiler and print the summary for each d8 run. This automates running the profiler with multiple benchmarks.

BUG=None
LOG=N
TBR=machenbach@chromium.org

Committed: https://crrev.com/b571b83bcdff3077f90bdbc5e30fbdf37579ac59
Cr-Commit-Position: refs/heads/master@{#30720}

[Michael Achenbach, 2015-09-10 07:49:44]

https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py
File tools/run_perf.py (right):

https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py#newco...
tools/run_perf.py:651: if profiler_run:
Suggestion: Could you maybe just use the existing extra_flags list? I.e. make
'--prof' a flag that triggers the additional logic below. The condition here
could be rewritten as:
if '--prof' in self.extra_flags:
  ...

The tool could be started with:
run_perf --extra-flags="--prof --other-flag1 ..."

The check for the d8 executable above in line 459 could be made in a similar
way, e.g.:
extra_flags = extra_flags or []
if self.binary != 'd8' and '--prof' in self.extra_flags:
  # Warn about wrong usage.

[gdeepti1, 2015-09-11 08:22:33]

https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py
File tools/run_perf.py (right):

https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py#newco...
tools/run_perf.py:651: if profiler_run:
On 2015/09/10 07:49:44, Michael Achenbach wrote:
> Suggestion: Could you maybe just use the existing extra_flags list? I.e. make
> '--prof' a flag that triggers the additional logic below. The condition here
> could be rewritten as:
> if '--prof' in self.extra_flags:
>   ...
> 
> The tool could be started with:
> run_perf --extra-flags="--prof --other-flag1 ..."
> 
> The check for the d8 executable above in line 459 could be made in a similar
> way, e.g.:
> extra_flags = extra_flags or []
> if self.binary != 'd8' and '--prof' in self.extra_flags:
>   # Warn about wrong usage.

Currently passing in the --profiler flag to run_perf alters the original output
by printing additional summary from the tick_processor. Output below: 

>>> Running suite: Embenchen/Box2d
>>> Stdout (#1):
EmbenchenBox2d(RunTime): 11392 ms.

Statistical profiling result from v8.log, (10723 ticks, 0 unaccounted, 0
excluded).

 [Summary]:
   ticks  total  nonlib   name
  10153   94.7%   94.9%  JavaScript
    549    5.1%    5.1%  C++
     40    0.4%    0.4%  GC
     21    0.2%          Shared libraries

My initial approach was to pass in the --prof as a part of extra flags, but as
it was modifying the output in a way passing the --prof flag to d8 does not do I
was not sure if it was the right approach. Just to confirm - it is ok to
overload the --prof flag to run and print the output of the tick processor as
well?

[Michael Achenbach, 2015-09-11 09:13:02]

https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py
File tools/run_perf.py (right):

https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py#newco...
tools/run_perf.py:651: if profiler_run:
On 2015/09/11 08:22:33, gdeepti1 wrote:
> On 2015/09/10 07:49:44, Michael Achenbach wrote:
> > Suggestion: Could you maybe just use the existing extra_flags list? I.e.
make
> > '--prof' a flag that triggers the additional logic below. The condition here
> > could be rewritten as:
> > if '--prof' in self.extra_flags:
> >   ...
> > 
> > The tool could be started with:
> > run_perf --extra-flags="--prof --other-flag1 ..."
> > 
> > The check for the d8 executable above in line 459 could be made in a similar
> > way, e.g.:
> > extra_flags = extra_flags or []
> > if self.binary != 'd8' and '--prof' in self.extra_flags:
> >   # Warn about wrong usage.
> 
> Currently passing in the --profiler flag to run_perf alters the original
output
> by printing additional summary from the tick_processor. Output below: 
> 
> >>> Running suite: Embenchen/Box2d
> >>> Stdout (#1):
> EmbenchenBox2d(RunTime): 11392 ms.
> 
> Statistical profiling result from v8.log, (10723 ticks, 0 unaccounted, 0
> excluded).
> 
>  [Summary]:
>    ticks  total  nonlib   name
>   10153   94.7%   94.9%  JavaScript
>     549    5.1%    5.1%  C++
>      40    0.4%    0.4%  GC
>      21    0.2%          Shared libraries
> 
> My initial approach was to pass in the --prof as a part of extra flags, but as
> it was modifying the output in a way passing the --prof flag to d8 does not do
I
> was not sure if it was the right approach. Just to confirm - it is ok to
> overload the --prof flag to run and print the output of the tick processor as
> well? 

Hmm - not sure if I understand. I don't wanna imply any code changes in v8/d8.
Everything there should remain as it is. You should also keep the code below
that does the printing (i.e. line 663). I would just not guard it by an extra
flag "if profiler_run", but by deducing the same thing from the extra_flags,
i.e. "if '--prof' in self.extra_flags".

I.e. when you run "d8 --prof" it does not run the tick-processor, but if you
test d8 through run_perf.py with --extra-flags="--prof", it will run with --prof
_and_ the run_perf script will call the tick processor and print.

[gdeepti1, 2015-09-11 20:05:57]

https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py
File tools/run_perf.py (right):

https://codereview.chromium.org/1327033003/diff/20001/tools/run_perf.py#newco...
tools/run_perf.py:651: if profiler_run:
On 2015/09/11 09:13:02, Michael Achenbach wrote:
> On 2015/09/11 08:22:33, gdeepti1 wrote:
> > On 2015/09/10 07:49:44, Michael Achenbach wrote:
> > > Suggestion: Could you maybe just use the existing extra_flags list? I.e.
> make
> > > '--prof' a flag that triggers the additional logic below. The condition
here
> > > could be rewritten as:
> > > if '--prof' in self.extra_flags:
> > >   ...
> > > 
> > > The tool could be started with:
> > > run_perf --extra-flags="--prof --other-flag1 ..."
> > > 
> > > The check for the d8 executable above in line 459 could be made in a
similar
> > > way, e.g.:
> > > extra_flags = extra_flags or []
> > > if self.binary != 'd8' and '--prof' in self.extra_flags:
> > >   # Warn about wrong usage.
> > 
> > Currently passing in the --profiler flag to run_perf alters the original
> output
> > by printing additional summary from the tick_processor. Output below: 
> > 
> > >>> Running suite: Embenchen/Box2d
> > >>> Stdout (#1):
> > EmbenchenBox2d(RunTime): 11392 ms.
> > 
> > Statistical profiling result from v8.log, (10723 ticks, 0 unaccounted, 0
> > excluded).
> > 
> >  [Summary]:
> >    ticks  total  nonlib   name
> >   10153   94.7%   94.9%  JavaScript
> >     549    5.1%    5.1%  C++
> >      40    0.4%    0.4%  GC
> >      21    0.2%          Shared libraries
> > 
> > My initial approach was to pass in the --prof as a part of extra flags, but
as
> > it was modifying the output in a way passing the --prof flag to d8 does not
do
> I
> > was not sure if it was the right approach. Just to confirm - it is ok to
> > overload the --prof flag to run and print the output of the tick processor
as
> > well? 
> 
> Hmm - not sure if I understand. I don't wanna imply any code changes in v8/d8.
> Everything there should remain as it is. You should also keep the code below
> that does the printing (i.e. line 663). I would just not guard it by an extra
> flag "if profiler_run", but by deducing the same thing from the extra_flags,
> i.e. "if '--prof' in self.extra_flags".
> 
> I.e. when you run "d8 --prof" it does not run the tick-processor, but if you
> test d8 through run_perf.py with --extra-flags="--prof", it will run with
--prof
> _and_ the run_perf script will call the tick processor and print.

Thanks for clarifying, simplified to use --extra-flags instead of an explicit
--profiler flag.

[Michael Achenbach, 2015-09-14 10:24:10]

lgtm with comment:

https://codereview.chromium.org/1327033003/diff/40001/tools/run_perf.py
File tools/run_perf.py (right):

https://codereview.chromium.org/1327033003/diff/40001/tools/run_perf.py#newco...
tools/run_perf.py:460: # TODO(machenbach): This requires +.exe if run on
windows.
Please add one line in the beginning of this method to make it more robust:
extra_flags = extra_flags or []

[gdeepti1, 2015-09-14 19:46:06]

https://codereview.chromium.org/1327033003/diff/40001/tools/run_perf.py
File tools/run_perf.py (right):

https://codereview.chromium.org/1327033003/diff/40001/tools/run_perf.py#newco...
tools/run_perf.py:460: # TODO(machenbach): This requires +.exe if run on
windows.
On 2015/09/14 10:24:10, Michael Achenbach wrote:
> Please add one line in the beginning of this method to make it more robust:
> extra_flags = extra_flags or []

Done.

[gdeepti1, 2015-09-14 19:46:11]

The CQ bit was checked by gdeepti@google.com

[gdeepti1, 2015-09-14 19:46:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
machenbach@chromium.org
Link to the patchset: https://codereview.chromium.org/1327033003/#ps60001
(title: "Michael's comment")

[commit-bot: I haz the power, 2015-09-14 19:46:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1327033003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1327033003/60001

[commit-bot: I haz the power, 2015-09-14 23:14:31]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-14 23:14:46]

Patchset 4 (id:??) landed as
https://crrev.com/b571b83bcdff3077f90bdbc5e30fbdf37579ac59
Cr-Commit-Position: refs/heads/master@{#30720}

[Michael Achenbach, 2015-09-15 07:03:41]

https://codereview.chromium.org/1327033003/diff/60001/tools/run_perf.py
File tools/run_perf.py (right):

https://codereview.chromium.org/1327033003/diff/60001/tools/run_perf.py#newco...
tools/run_perf.py:463: if self.binary != 'd8' and '--prof' in self.extra_flags:
Ah crap - didn't catch this. s/self.extra_flags/extra_flags

https://codereview.chromium.org/1327033003/diff/60001/tools/run_perf.py#newco...
tools/run_perf.py:658: prof_cmd = tick_tools + " --only-summary"
In the else case, tick_tools is read before assignment.

[Michael Achenbach, 2015-09-15 08:14:35]

Clean up in https://codereview.chromium.org/1341213002/