Fix deferred library code disabling for inlined functions.

Fix deferred library code disabling for inlined functions.

R=johnmccutchan@google.com

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

[Cutch, 2014-06-06 21:55:48]

lgtm with some suggestions

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

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/compiler.cc#n...
runtime/vm/compiler.cc:563: } else if (FLAG_trace_patching) {
Should this be:

if (FLAG_trace_compiler || FLAG_trace_patching) with the same print?

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/flow_graph.cc
File runtime/vm/flow_graph.cc (right):

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/flow_graph.cc...
runtime/vm/flow_graph.cc:67: void FlowGraph::AddToDeferredPrefixes(
What about making this a non-static function so later on you would do:

caller_graph()->AddDeferredPrefixes(callee_graph()->deferred_prefixes());

?

[srdjan, 2014-06-06 22:06:16]

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

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/compiler.cc#n...
runtime/vm/compiler.cc:563: } else if (FLAG_trace_patching) {
On 2014/06/06 21:55:47, Cutch wrote:
> Should this be:
> 
> if (FLAG_trace_compiler || FLAG_trace_patching) with the same print?

Done.

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/flow_graph.cc
File runtime/vm/flow_graph.cc (right):

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/flow_graph.cc...
runtime/vm/flow_graph.cc:67: void FlowGraph::AddToDeferredPrefixes(
On 2014/06/06 21:55:48, Cutch wrote:
> What about making this a non-static function so later on you would do:
> 
> caller_graph()->AddDeferredPrefixes(callee_graph()->deferred_prefixes());
> 
> ?

Done.

[hausner, 2014-06-06 22:07:45]

DBC: The change in Parser.cc slows down the common case where we don't see any
deferred prefixes.

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/parser.cc
File runtime/vm/parser.cc (left):

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/parser.cc#old...
runtime/vm/parser.cc:141:
&GrowableObjectArray::ZoneHandle(GrowableObjectArray::New());
We pay this price only in the rare case where we find deferred prefixes.

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/parser.h
File runtime/vm/parser.h (right):

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/parser.h#newc...
runtime/vm/parser.h:50: deferred_prefixes_(new ZoneGrowableArray<const
LibraryPrefix*>()),
I didn't want to allocate anything for the 99.99% of the cases where we will not
encounter any deferred prefixes.

[srdjan, 2014-06-06 22:14:35]

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/parser.cc
File runtime/vm/parser.cc (left):

https://codereview.chromium.org/313403004/diff/40001/runtime/vm/parser.cc#old...
runtime/vm/parser.cc:141:
&GrowableObjectArray::ZoneHandle(GrowableObjectArray::New());
On 2014/06/06 22:07:44, hausner wrote:
> We pay this price only in the rare case where we find deferred prefixes.

The new solution simplifies the code and makes it more robust, IMO, as we do not
need to check for NULL here and when adding inlined deferred library prefixes.
The array is allocated once per parsing, the overhead seems small.

[srdjan, 2014-06-06 22:14:47]

Committed patchset #5 manually as r37107 (presubmit successful).

[Ivan Posva, 2014-06-07 00:31:14]

DBC -ip

https://codereview.chromium.org/313403004/diff/80001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/313403004/diff/80001/runtime/vm/parser.cc#new...
runtime/vm/parser.cc:144: deferred_prefixes_->Add(&LibraryPrefix::ZoneHandle(I,
prefix.raw()));
Why do you have to get a new ZoneHandle here? I don't think we have an inner
zone while we are inlining.

[srdjan, 2014-06-12 20:25:47]

https://codereview.chromium.org/313403004/diff/80001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/313403004/diff/80001/runtime/vm/parser.cc#new...
runtime/vm/parser.cc:144: deferred_prefixes_->Add(&LibraryPrefix::ZoneHandle(I,
prefix.raw()));
On 2014/06/07 00:31:14, Ivan Posva wrote:
> Why do you have to get a new ZoneHandle here? I don't think we have an inner
> zone while we are inlining.

Generally: if a handle's lifetime escapes the scope (in this case function) we
make it a ZoneHandle.