Lazily generate local var descriptors.

Lazily generate local var descriptors.

R=hausner@google.com

Committed: https://code.google.com/p/dart/source/detail?r=45581

[srdjan, 2015-05-06 16:05:09]

srdjan@google.com changed reviewers:
+ fschneider@google.com, hausner@google.com

[hausner, 2015-05-06 16:39:55]

I'd like to see numbers that show how much data we saved. Also, I'm worried
about the mushrooming of command line flags. Changing the flags later is a
breaking change (since tools may use them in scripts).

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

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1132: void Compiler::ComputeLocalVarDescriptors(const
Code& code) {
This code is now duplicated (also present in the
FlowGraphCompiler::FinalizeVarDescriptors). Could this be factored out?

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1140: var_descs = LocalVarDescriptors::New(1);
Should the code after UNREACHABLE() remain?

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1153: parsed_function->function());
Could you not use function here instead of parsed_function->function()?

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_compiler.cc (right):

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/flow_graph_...
runtime/vm/flow_graph_compiler.cc:33: "Inlining interval diagnostics");;
Two is better than one?

[srdjan, 2015-05-06 20:00:47]

Thanks for the review.

The numbers:
Saves 714kb on dart2js when compiling dart2js. In every program run, the
observatory will show the amount of memory allocated for LocalVarDescriptors.
That amount is almost completely eliminated.

The flag is meant to be transitional, in case that there are problems with this
change. Added comment 'TRANSITIONAL' to the  flag. I will remove it (and the
necessary code) in 2 days. The alternative is to remove the flag and the code
now. I am fine with both approaches.

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

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1132: void Compiler::ComputeLocalVarDescriptors(const
Code& code) {
On 2015/05/06 16:39:54, hausner wrote:
> This code is now duplicated (also present in the
> FlowGraphCompiler::FinalizeVarDescriptors). Could this be factored out?

Removed it from here; FlowGraphCompiler::FinalizeVarDescriptors should go away
once the transition to lazy local var descriptors has completed.

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1140: var_descs = LocalVarDescriptors::New(1);
On 2015/05/06 16:39:54, hausner wrote:
> Should the code after UNREACHABLE() remain?

Removed, will add it back once functionality implemented.

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1153: parsed_function->function());
On 2015/05/06 16:39:54, hausner wrote:
> Could you not use function here instead of parsed_function->function()?

Done.

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_compiler.cc (right):

https://codereview.chromium.org/1128803002/diff/100001/runtime/vm/flow_graph_...
runtime/vm/flow_graph_compiler.cc:33: "Inlining interval diagnostics");;
On 2015/05/06 16:39:55, hausner wrote:
> Two is better than one?

:-) removed.

[hausner, 2015-05-06 21:05:48]

LGTM

[srdjan, 2015-05-07 04:17:04]

Committed patchset #8 (id:140001) manually as r45581 (presubmit successful).