Add --parse-all option in order to benchmark and measure the scanner/parser performance.

Add --parse-all option in order to benchmark and measure the scanner/parser performance.

R=hausner@google.com

Committed: https://github.com/dart-lang/sdk/commit/12f86594ecc32d7fa0c513fcc960d9adaee13428

[hausner, 2016-09-09 16:31:07]

hausner@google.com changed reviewers:
+ hausner@google.com

[hausner, 2016-09-09 16:31:08]

LGTM w/comments.

https://codereview.chromium.org/2327693002/diff/1/runtime/bin/main.cc
File runtime/bin/main.cc (right):

https://codereview.chromium.org/2327693002/diff/1/runtime/bin/main.cc#newcode811
runtime/bin/main.cc:811: #if 0
Maybe remove this entirely in a separate change?

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/compiler.cc#newc...
runtime/vm/compiler.cc:1403: INC_STAT(thread, num_functions_compiled, 1);
I know this is just a temporary thing, but it might make sense not to increase
these counters here, because the functions do not really get compiled. Not
incrementing here allows us to see how many functions are in fact really
compiled as a side effect of loading code and parsing sources. (E.g. for
evaluating compile-time constants.)

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/object.cc#newcod...
runtime/vm/object.cc:11370: // Inner functions get added to the closures array.
As part of compilation
I don't think iterating over inner functions is needed. We parse inner functions
while parsing their enclosing function. Thus, this step is just parsing them
*again*.

[hausner, 2016-09-09 17:30:51]

Another comment

https://codereview.chromium.org/2327693002/diff/1/runtime/bin/main.cc
File runtime/bin/main.cc (right):

https://codereview.chromium.org/2327693002/diff/1/runtime/bin/main.cc#newcode...
runtime/bin/main.cc:1481: Dart_ExitScope();
This will actually break tests that execute the code after compiling everything.
I think it's useful to be able to run a program and ask that all code be
compiled.

[siva, 2016-09-22 23:55:36]

Description was changed from

==========
Temporary changes to make a parse all benchmark in order to measure the
scanner/parser performance.
==========

to

==========
Add --parse-all option in order to benchmark and measure the scanner/parser
performance.
==========

[siva, 2016-09-23 00:04:04]

Updated changes PTAL.

https://codereview.chromium.org/2327693002/diff/1/runtime/bin/main.cc
File runtime/bin/main.cc (right):

https://codereview.chromium.org/2327693002/diff/1/runtime/bin/main.cc#newcode811
runtime/bin/main.cc:811: #if 0
On 2016/09/09 16:31:07, hausner wrote:
> Maybe remove this entirely in a separate change?

Removed the #if 0 ... #endif as I added a --parse-all option and let the
compile-all option remain the way it was.

https://codereview.chromium.org/2327693002/diff/1/runtime/bin/main.cc#newcode...
runtime/bin/main.cc:1481: Dart_ExitScope();
On 2016/09/09 17:30:51, hausner wrote:
> This will actually break tests that execute the code after compiling
everything.
> I think it's useful to be able to run a program and ask that all code be
> compiled.

Removed this as compile_all is unchanged.

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/compiler.cc#newc...
runtime/vm/compiler.cc:1403: INC_STAT(thread, num_functions_compiled, 1);
On 2016/09/09 16:31:07, hausner wrote:
> I know this is just a temporary thing, but it might make sense not to increase
> these counters here, because the functions do not really get compiled. Not
> incrementing here allows us to see how many functions are in fact really
> compiled as a side effect of loading code and parsing sources. (E.g. for
> evaluating compile-time constants.)

Done.

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/object.cc#newcod...
runtime/vm/object.cc:11370: // Inner functions get added to the closures array.
As part of compilation
On 2016/09/09 16:31:07, hausner wrote:
> I don't think iterating over inner functions is needed. We parse inner
functions
> while parsing their enclosing function. Thus, this step is just parsing them
> *again*.

But I guess it is representative of what we would do if we had to produce an IR.

[hausner, 2016-09-23 21:57:20]

LGTM w/comment.

The AST printer change seems orthogonal to parse_all. If you think it's work
keeping, it should go into a separate CL.

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/2327693002/diff/1/runtime/vm/object.cc#newcod...
runtime/vm/object.cc:11370: // Inner functions get added to the closures array.
As part of compilation
On 2016/09/23 00:04:04, siva wrote:
> On 2016/09/09 16:31:07, hausner wrote:
> > I don't think iterating over inner functions is needed. We parse inner
> functions
> > while parsing their enclosing function. Thus, this step is just parsing them
> > *again*.
> 
> But I guess it is representative of what we would do if we had to produce an
IR.

Sure. On the other hand, in an AOT compiler we would not reparse, but keep the
IR and produce code right away.

https://codereview.chromium.org/2327693002/diff/60001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/2327693002/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1412: AstPrinter ast_printer(false);
What's the purpose of invoking the ast printer visitor when nothing is printed?

[siva, 2016-09-23 22:05:37]

https://codereview.chromium.org/2327693002/diff/60001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/2327693002/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1412: AstPrinter ast_printer(false);
On 2016/09/23 21:57:20, hausner wrote:
> What's the purpose of invoking the ast printer visitor when nothing is
printed?

I wanted to keep it to measure if traversing the AST nodes had any effect on the
overall time.

[siva, 2016-09-23 22:20:42]

Description was changed from

==========
Add --parse-all option in order to benchmark and measure the scanner/parser
performance.
==========

to

==========
Add --parse-all option in order to benchmark and measure the scanner/parser
performance.

R=hausner@google.com

Committed:
https://github.com/dart-lang/sdk/commit/12f86594ecc32d7fa0c513fcc960d9adaee13428
==========

[siva, 2016-09-23 22:20:43]

Committed patchset #4 (id:60001) manually as
12f86594ecc32d7fa0c513fcc960d9adaee13428 (presubmit successful).