Register existing isolates when service isolate is started

Register existing isolates when service isolate is started

BUG=
R=asiva@google.com

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

[siva, 2014-04-21 21:27:53]

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

https://codereview.chromium.org/242493007/diff/1/runtime/vm/service.cc#newcod...
runtime/vm/service.cc:302: ASSERT(!function.IsNull());
Can the lookup of this top level function _registerIsolate be done once in the
constructor and then the function used in VisitIsolate()?

This can turn out to be expensive if the number of isolates is large.

https://codereview.chromium.org/242493007/diff/1/runtime/vm/service/vmservice...
File runtime/vm/service/vmservice.dart (right):

https://codereview.chromium.org/242493007/diff/1/runtime/vm/service/vmservice...
runtime/vm/service/vmservice.dart:100: service.messageHandler(message);
Why does it have to be done this way, why can't it be implemented as:
void _registerIsolate(int port_id, SendPort sp, String name) {
  var service = new VMService();
  service.runningIsolates.isolateStartup(port_id, sp, name);
}

[Cutch, 2014-04-21 22:00:40]

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

https://codereview.chromium.org/242493007/diff/1/runtime/vm/service.cc#newcod...
runtime/vm/service.cc:302: ASSERT(!function.IsNull());
On 2014/04/21 21:27:53, siva wrote:
> Can the lookup of this top level function _registerIsolate be done once in the
> constructor and then the function used in VisitIsolate()?
> 
> This can turn out to be expensive if the number of isolates is large.

Done.

https://codereview.chromium.org/242493007/diff/1/runtime/vm/service/vmservice...
File runtime/vm/service/vmservice.dart (right):

https://codereview.chromium.org/242493007/diff/1/runtime/vm/service/vmservice...
runtime/vm/service/vmservice.dart:100: service.messageHandler(message);
On 2014/04/21 21:27:53, siva wrote:
> Why does it have to be done this way, why can't it be implemented as:
> void _registerIsolate(int port_id, SendPort sp, String name) {
>   var service = new VMService();
>   service.runningIsolates.isolateStartup(port_id, sp, name);
> }

Done.

[siva, 2014-04-21 22:11:41]

LGTM with comments.

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

https://codereview.chromium.org/242493007/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:302: }
ASSERT(Isolate::Current() == service_isolate_);
here too.

https://codereview.chromium.org/242493007/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:319: ASSERT(!r.IsError());
This might give you an unused variable error in release mode ('r' unused).

[Cutch, 2014-04-21 22:16:17]

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

https://codereview.chromium.org/242493007/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:302: }
On 2014/04/21 22:11:41, siva wrote:
> ASSERT(Isolate::Current() == service_isolate_);
> here too.

Done.

https://codereview.chromium.org/242493007/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:319: ASSERT(!r.IsError());
On 2014/04/21 22:11:41, siva wrote:
> This might give you an unused variable error in release mode ('r' unused).

Done.

[Cutch, 2014-04-21 22:16:43]

Committed patchset #3 manually as r35227 (presubmit successful).