VM: Link native calls lazily.

VM: Link native calls lazily.

The first invocation of a native functions goes into LinkNativeCall which
determines the final entry point and patches the object pool entry.

When running precompiled code, this makes deserializing the object
pool entries for native functions easy, they all initially point to
a single entry (LinkNativeCall).

BUG=
R=rmacnak@google.com

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

[Florian Schneider, 2015-08-19 13:56:32]

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

[rmacnak, 2015-08-20 23:48:07]

lgtm

I did see one flake locally, but I don't see anything in your change that should
cause it.
I think Redirection::Get has a race if two threads reach the end of the list at
the same time and both create a new Redirection, whichever one gets appended
second will not match the later check against Redirection::Get in your assert.

runtime/vm/native_entry.cc:220: error: expected: current_function ==
reinterpret_cast<NativeFunction>( Simulator::RedirectExternalReference(
reinterpret_cast<uword>(LinkNativeCall), Simulator::kBootstrapNativeCall,
func.NumParameters()))


Command[vm]: DART_CONFIGURATION=DebugSIMARM out/DebugSIMARM/dart
--link-natives-lazily --ignore-unrecognized-flags
--package-root=out/DebugSIMARM/packages/
/usr/local/google/home/rmacnak/dart2/sdk/tests/language/duplicate_import_prefix_test.dart

https://chromiumcodereview.appspot.com/1294113004/diff/20001/runtime/vm/nativ...
File runtime/vm/native_entry.cc (right):

https://chromiumcodereview.appspot.com/1294113004/diff/20001/runtime/vm/nativ...
runtime/vm/native_entry.cc:185: bool is_native_auto_setup_scope = false;
only used behind USING_SIMULATOR

https://chromiumcodereview.appspot.com/1294113004/diff/20001/runtime/vm/nativ...
runtime/vm/native_entry.cc:186: intptr_t num_parameters = -1;
"

https://chromiumcodereview.appspot.com/1294113004/diff/20001/runtime/vm/parse...
File runtime/vm/parser.cc (right):

https://chromiumcodereview.appspot.com/1294113004/diff/20001/runtime/vm/parse...
runtime/vm/parser.cc:48: DEFINE_FLAG(bool, link_natives_lazily, true, "Link
native calls lazily");
enable in the precompile flag handler

[Florian Schneider, 2015-08-21 07:58:18]

https://codereview.chromium.org/1294113004/diff/20001/runtime/vm/native_entry.cc
File runtime/vm/native_entry.cc (right):

https://codereview.chromium.org/1294113004/diff/20001/runtime/vm/native_entry...
runtime/vm/native_entry.cc:185: bool is_native_auto_setup_scope = false;
On 2015/08/20 23:48:06, rmacnak wrote:
> only used behind USING_SIMULATOR

Done.

https://codereview.chromium.org/1294113004/diff/20001/runtime/vm/native_entry...
runtime/vm/native_entry.cc:186: intptr_t num_parameters = -1;
On 2015/08/20 23:48:06, rmacnak wrote:
> "

Done.

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

https://codereview.chromium.org/1294113004/diff/20001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:48: DEFINE_FLAG(bool, link_natives_lazily, true, "Link
native calls lazily");
On 2015/08/20 23:48:06, rmacnak wrote:
> enable in the precompile flag handler

Done.

We should look into how to test the precompile configuration continuously.

[Florian Schneider, 2015-08-21 07:59:03]

On 2015/08/20 23:48:07, rmacnak wrote:
> lgtm
> 
> I did see one flake locally, but I don't see anything in your change that
should
> cause it.
> I think Redirection::Get has a race if two threads reach the end of the list
at
> the same time and both create a new Redirection, whichever one gets appended
> second will not match the later check against Redirection::Get in your assert.
> 
> runtime/vm/native_entry.cc:220: error: expected: current_function ==
> reinterpret_cast<NativeFunction>( Simulator::RedirectExternalReference(
> reinterpret_cast<uword>(LinkNativeCall), Simulator::kBootstrapNativeCall,
> func.NumParameters()))
> 
> 
> Command[vm]: DART_CONFIGURATION=DebugSIMARM out/DebugSIMARM/dart
> --link-natives-lazily --ignore-unrecognized-flags
> --package-root=out/DebugSIMARM/packages/
>
/usr/local/google/home/rmacnak/dart2/sdk/tests/language/duplicate_import_prefix_test.dart
> 

Interesting. I'll check if I can reproduce this.

[Florian Schneider, 2015-08-21 09:37:56]

Committed patchset #3 (id:40001) manually as
c05f1f944851560c9acbdc0ca9d3ee92b408f038.