Precompilation: Eagerly bind all the static calls after compiling and drop the static call table.

Precompilation: Eagerly bind all the static calls after compiling and drop the static call tables.

Shrinks dart2js precompiled.isolate by 2.2%.

R=fschneider@google.com

Committed: https://github.com/dart-lang/sdk/commit/7e821565d90e3a4e953f20e1b6f3231259e6e2e9

[rmacnak, 2015-10-02 22:59:17]

rmacnak@google.com changed reviewers:
+ fschneider@google.com, iposva@google.com

[rmacnak, 2015-10-02 22:59:17]

Assuming we won't try to precompile pretenuring allocation stubs and switch to
them.

[Ivan Posva, 2015-10-03 00:07:32]

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

https://codereview.chromium.org/1382243002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:716: retained_functions = GrowableObjectArray::New();
What purpose does this allocation serve?

https://codereview.chromium.org/1382243002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:755: CodePatcher::PatchStaticCallAt(pc_offset.Value()
+ code.EntryPoint(),
Can you please explain why only patching of the function based calls is needed
and not for the stubs above?

[Florian Schneider, 2015-10-05 13:59:01]

On 2015/10/02 22:59:17, rmacnak wrote:
> Assuming we won't try to precompile pretenuring allocation stubs and switch to
> them.

How difficult would it be to avoid the CallStaticFunction stub in precompiled
code from the start? This would also eliminate the unused entry pointing to it
from the object pool.

[rmacnak, 2015-10-05 17:53:39]

On 2015/10/05 13:59:01, Florian Schneider wrote:
> On 2015/10/02 22:59:17, rmacnak wrote:
> > Assuming we won't try to precompile pretenuring allocation stubs and switch
to
> > them.
> 
> How difficult would it be to avoid the CallStaticFunction stub in precompiled
> code from the start? This would also eliminate the unused entry pointing to it
> from the object pool.

After BindStaticCalls, no object pool will point to the CallStaticFunction stub.
Functions with multiple static call sites to the same target will still have
duplicate entries in the object pool, but the regular VM needs this to support
breakpoints so eliminating this would mean different ways of compiling in the
regular VM and precompilation.

[rmacnak, 2015-10-05 17:54:04]

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

https://codereview.chromium.org/1382243002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:716: retained_functions = GrowableObjectArray::New();
On 2015/10/03 00:07:31, Ivan Posva wrote:
> What purpose does this allocation serve?

From copied iteration code. Removed.

https://codereview.chromium.org/1382243002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:755: CodePatcher::PatchStaticCallAt(pc_offset.Value()
+ code.EntryPoint(),
On 2015/10/03 00:07:31, Ivan Posva wrote:
> Can you please explain why only patching of the function based calls is needed
> and not for the stubs above?

Allocation stubs are generated eagerly, so their uses already point to the right
Code. Static calls might refer to function that haven't been compiled yet, so
they point to a stub that will ensure compilation then patch up the reference to
the right Code.

[Ivan Posva, 2015-10-05 18:30:06]

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

https://codereview.chromium.org/1382243002/diff/1/runtime/vm/precompiler.cc#n...
runtime/vm/precompiler.cc:755: CodePatcher::PatchStaticCallAt(pc_offset.Value()
+ code.EntryPoint(),
On 2015/10/05 17:54:03, rmacnak wrote:
> On 2015/10/03 00:07:31, Ivan Posva wrote:
> > Can you please explain why only patching of the function based calls is
needed
> > and not for the stubs above?
> 
> Allocation stubs are generated eagerly, so their uses already point to the
right
> Code. Static calls might refer to function that haven't been compiled yet, so
> they point to a stub that will ensure compilation then patch up the reference
to
> the right Code.

Put differently: You are using the stub as a handle until compilation is
finished, correct? Can you please add a comment to that effect? Thanks!

[Florian Schneider, 2015-10-05 19:20:51]

On 2015/10/05 17:53:39, rmacnak wrote:
> On 2015/10/05 13:59:01, Florian Schneider wrote:
> > On 2015/10/02 22:59:17, rmacnak wrote:
> > > Assuming we won't try to precompile pretenuring allocation stubs and
switch
> to
> > > them.
> > 
> > How difficult would it be to avoid the CallStaticFunction stub in
precompiled
> > code from the start? This would also eliminate the unused entry pointing to
it
> > from the object pool.
> 
> After BindStaticCalls, no object pool will point to the CallStaticFunction
stub.
> Functions with multiple static call sites to the same target will still have
> duplicate entries in the object pool, but the regular VM needs this to support
> breakpoints so eliminating this would mean different ways of compiling in the
> regular VM and precompilation.

Right, since we're patching the object pool and not the instructions.

We already run with --no-debugger-support with precompilation, right? It would
be nice to avoid any duplicate
entries in this case, but we should find out how much duplicates there are first
(e.g. in dart2js precompiled code)

LGTM.

[rmacnak, 2015-10-05 22:52:00]

Committed patchset #2 (id:20001) manually as
7e821565d90e3a4e953f20e1b6f3231259e6e2e9 (presubmit successful).

[rmacnak, 2015-10-05 22:55:15]

On 2015/10/05 18:30:06, Ivan Posva wrote:
> https://codereview.chromium.org/1382243002/diff/1/runtime/vm/precompiler.cc
> File runtime/vm/precompiler.cc (right):
> 
>
https://codereview.chromium.org/1382243002/diff/1/runtime/vm/precompiler.cc#n...
> runtime/vm/precompiler.cc:755:
CodePatcher::PatchStaticCallAt(pc_offset.Value()
> + code.EntryPoint(),
> On 2015/10/05 17:54:03, rmacnak wrote:
> > On 2015/10/03 00:07:31, Ivan Posva wrote:
> > > Can you please explain why only patching of the function based calls is
> needed
> > > and not for the stubs above?
> > 
> > Allocation stubs are generated eagerly, so their uses already point to the
> right
> > Code. Static calls might refer to function that haven't been compiled yet,
so
> > they point to a stub that will ensure compilation then patch up the
reference
> to
> > the right Code.
> 
> Put differently: You are using the stub as a handle until compilation is
> finished, correct? Can you please add a comment to that effect? Thanks!

Yes