Service isolate rework

* Create vm-service isolate at Dart_Initialize time.
* Remove Service create callback.
* Simplify creation of service isolate.
* Creation is done on thread pool.
* Use vm-service isolate for loading in standalone embedder.
* Remove import of dart:io from builtin library.

performance changes:

no service isolate:

$ ./tools/test.py -m release
Test configuration: none_vm_release_ia32
[01:27 | 100% | +12363 | -    0]

$ ./tools/test.py -m release
Test configuration: none_vm_release_ia32
[01:27 | 100% | +12363 | -    0]

always start service isolate (as a thread pool task):

$ ./tools/test.py -m release
Test configuration: none_vm_release_ia32
[01:45 | 100% | +12363 | -    0]

$ ./tools/test.py -m release
Test configuration: none_vm_release_ia32
[01:44 | 100% | +12363 | -    0]

service does I/O (calls to Dart_LoadScript block until service is running):

$ ./tools/test.py -m release
Test configuration: none_vm_release_ia32
[01:55 | 100% | +12363 | -    0]

$ ./tools/test.py -m release
Test configuration: none_vm_release_ia32
[01:55 | 100% | +12363 | -    0]

R=asiva@google.com

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

[Cutch, 2014-09-19 21:25:59]

https://codereview.chromium.org/584023004/diff/1/runtime/bin/vmservice/server...
File runtime/bin/vmservice/server.dart (right):

https://codereview.chromium.org/584023004/diff/1/runtime/bin/vmservice/server...
runtime/bin/vmservice/server.dart:167: print('started');
This never happens when I run:

./out/DebugIA32/dart --package-root=out/DebugIA32/packages
tests/standalone/vmservice/unknown_command_test.dart

It's hung waiting for the bind to occur. If I send a SIGQUIT everything starts
moving again.

[Cutch, 2014-09-24 18:13:06]

johnmccutchan@google.com changed reviewers:
+ asiva@google.com, iposva@google.com

[Cutch, 2015-01-15 17:47:50]

This is ready.

PTAL.

[siva, 2015-01-22 22:39:31]

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin.dar...
runtime/bin/builtin.dart:43: _getCurrentDirectoryPath() native
"Directory_Current";
see comment below about these native methods.

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin.dar...
runtime/bin/builtin.dart:333: String _operatingSystemName() native
"Platform_OperatingSystem";
Platform_OperatingSystem is a native in the IO library and things
work here because the native lookup for builtin library falls back on the
io_library lookup. Once the IO folks realize that is not necessary to do that
the lookup for Platform_OperatingSystem will fail.

Maybe you should just have a native method in builtin that returns the extension
name given the name.

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin_nol...
File runtime/bin/builtin_nolib.cc (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin_nol...
runtime/bin/builtin_nolib.cc:91: ASSERT(!Dart_IsError(result));
This code seems to be identical to the one in builtin.cc why not factor this out
into a common file.

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/vmservice/l...
File runtime/bin/vmservice/loader.dart (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/vmservice/l...
runtime/bin/vmservice/loader.dart:1: // Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
2015

https://codereview.chromium.org/584023004/diff/280001/runtime/lib/isolate.cc
File runtime/lib/isolate.cc (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/lib/isolate.cc#...
runtime/lib/isolate.cc:111: bool destination_is_service =
Service::IsServicePort(destination_port_id);
I thought the service protocol is based on JSON.
I am not sure if it is a good idea to allow random objects here, if typed data
objects are the only ones you are interested in then it is already allowed
(collection library and typeddata library).

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:1526: HANDLESCOPE(isolate);
If you are worried about excessive handle creation during print why not move
this HANDLESCOPE inside Print.

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/service.cc#n...
runtime/vm/service.cc:677: Service::FinishedInitializing();
on error cases a thread that is waiting on Service::WaitForLoadPort would
continue to wait because load_port would be ILLEGAL_PORT at this point?

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/service.cc#n...
runtime/vm/service.cc:740: Service::SetLoadPort(rp.Id());
This is somewhat confusing, this function does a notifyAll here
and immediately on return from 'RunMain' FinishedInitializing is called
which also does a notifyAll, why is it necessary to do two notifyAll calls.

[Cutch, 2015-01-26 18:59:31]

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin.dar...
runtime/bin/builtin.dart:43: _getCurrentDirectoryPath() native
"Directory_Current";
On 2015/01/22 22:39:31, siva wrote:
> see comment below about these native methods.

Directory_Current is part of the builtin natives.

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin.dar...
runtime/bin/builtin.dart:333: String _operatingSystemName() native
"Platform_OperatingSystem";
On 2015/01/22 22:39:31, siva wrote:
> Platform_OperatingSystem is a native in the IO library and things
> work here because the native lookup for builtin library falls back on the
> io_library lookup. Once the IO folks realize that is not necessary to do that
> the lookup for Platform_OperatingSystem will fail.
> 
> Maybe you should just have a native method in builtin that returns the
extension
> name given the name.

I've added a new native for builtin that returns the shared library extension
for the OS.

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin_nol...
File runtime/bin/builtin_nolib.cc (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/builtin_nol...
runtime/bin/builtin_nolib.cc:91: ASSERT(!Dart_IsError(result));
On 2015/01/22 22:39:31, siva wrote:
> This code seems to be identical to the one in builtin.cc why not factor this
out
> into a common file. 

Done.

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/vmservice/l...
File runtime/bin/vmservice/loader.dart (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/bin/vmservice/l...
runtime/bin/vmservice/loader.dart:1: // Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
On 2015/01/22 22:39:31, siva wrote:
> 2015

Done.

https://codereview.chromium.org/584023004/diff/280001/runtime/lib/isolate.cc
File runtime/lib/isolate.cc (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/lib/isolate.cc#...
runtime/lib/isolate.cc:111: bool destination_is_service =
Service::IsServicePort(destination_port_id);
On 2015/01/22 22:39:31, siva wrote:
> I thought the service protocol is based on JSON.
> I am not sure if it is a good idea to allow random objects here, if typed data
> objects are the only ones you are interested in then it is already allowed
> (collection library and typeddata library).

Done.

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:1526: HANDLESCOPE(isolate);
On 2015/01/22 22:39:31, siva wrote:
> If you are worried about excessive handle creation during print why not move
> this HANDLESCOPE inside Print.

The issue is that there is no handle scope at all. This code used to only work
in release mode.

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/service.cc#n...
runtime/vm/service.cc:677: Service::FinishedInitializing();
On 2015/01/22 22:39:31, siva wrote:
> on error cases a thread that is waiting on Service::WaitForLoadPort would
> continue to wait because load_port would be ILLEGAL_PORT at this point?

No, the loop in WaitForLoadPort will break if initializing_ == false.

https://codereview.chromium.org/584023004/diff/280001/runtime/vm/service.cc#n...
runtime/vm/service.cc:740: Service::SetLoadPort(rp.Id());
On 2015/01/22 22:39:31, siva wrote:
> This is somewhat confusing, this function does a notifyAll here
> and immediately on return from 'RunMain' FinishedInitializing is called
> which also does a notifyAll, why is it necessary to do two notifyAll calls.

Removed notifyAll from SetLoadPort.

[siva, 2015-01-27 19:12:46]

LGTM with one comment.

https://codereview.chromium.org/584023004/diff/360001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/584023004/diff/360001/runtime/bin/dartutils.c...
runtime/bin/dartutils.cc:483: Builtin::SetLoadPort(load_port);
Maybe return an error if load_port remains ILLEGAL_PORT for whatever reason.

https://codereview.chromium.org/584023004/diff/360001/runtime/bin/dartutils.c...
runtime/bin/dartutils.cc:606: }
it was not clear to me why these Builtin_Native functions are listed here and
not in builtin_native.cc (maybe cleanup in a different cl).

https://codereview.chromium.org/584023004/diff/360001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/584023004/diff/360001/runtime/include/dart_ap...
runtime/include/dart_api.h:2747: * \return Returns the port for load requests.
You need to mention that it might return ILLEGAL_PORT if an error occurs while
initializing the service isolate.

[Cutch, 2015-01-27 19:31:00]

https://codereview.chromium.org/584023004/diff/360001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/584023004/diff/360001/runtime/bin/dartutils.c...
runtime/bin/dartutils.cc:483: Builtin::SetLoadPort(load_port);
On 2015/01/27 19:12:46, siva wrote:
> Maybe return an error if load_port remains ILLEGAL_PORT for whatever reason.

Done.

https://codereview.chromium.org/584023004/diff/360001/runtime/bin/dartutils.c...
runtime/bin/dartutils.cc:606: }
On 2015/01/27 19:12:46, siva wrote:
> it was not clear to me why these Builtin_Native functions are listed here and
> not in builtin_native.cc (maybe cleanup in a different cl).

Acknowledged.

https://codereview.chromium.org/584023004/diff/360001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/584023004/diff/360001/runtime/include/dart_ap...
runtime/include/dart_api.h:2747: * \return Returns the port for load requests.
On 2015/01/27 19:12:46, siva wrote:
> You need to mention that it might return ILLEGAL_PORT if an error occurs while
> initializing the service isolate.

Done.

[Cutch, 2015-01-27 19:46:28]

Committed patchset #21 (id:400001) manually as 43202 (presubmit successful).