[vm] Move origin_id from isolate to isolate group.

Isolates in one group share same origin_id anyway, so it makes sense to store it on the group too.


Remove isolate's _originNumber from service api - isolate group should be used instead.

Based on feedback from https://dart-review.git.corp.google.com/c/sdk/+/418503/21/runtime/lib/isolate.cc#107

TEST=ci
Change-Id: Iab4b6393a042c9302e911a276a6afc6dab63e70d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/424140
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>

pkg/vm_service/test/get_isolate_rpc_test.dart

@@ -17,7 +17,6 @@ final tests = <IsolateTest>[
     expect(result.isolateFlags, isNotNull);
     expect(result.isolateFlags!.length, isPositive);
     expect(result.isSystemIsolate, isFalse);
-    expect(result.json!['_originNumber'], result.number);
     expect(result.startTime, isPositive);
     expect(result.livePorts, isPositive);
     expect(result.pauseOnExit, isFalse);

pkg/vm_service/test/http_invocations/http_get_isolate_rpc_common.dart

@@ -39,7 +39,6 @@ final httpGetIsolateRpcTests = <IsolateTest>[
       )! as Isolate;
       Expect.isTrue(result.id!.startsWith('isolates/'));
       Expect.isNotNull(result.number);
-      Expect.equals(result.json!['_originNumber'], result.number);
       Expect.isTrue(result.startTime! > 0);
       Expect.isTrue(result.livePorts! > 0);
       Expect.isFalse(result.pauseOnExit);

runtime/lib/isolate.cc

@@ -102,12 +102,9 @@ DEFINE_NATIVE_ENTRY(SendPort_get_hashcode, 0, 1) {
   return Smi::New(hash);
 }
 
-static bool InSameGroup(Isolate* sender, const SendPort& receiver) {
-  // Cannot determine whether sender is in same group (yet).
-  if (sender->origin_id() == ILLEGAL_PORT) return false;
-
+static bool InSameGroup(IsolateGroup* sender, const SendPort& receiver) {
   // Only allow arbitrary messages between isolates of the same IG.
-  return sender->origin_id() == receiver.origin_id();
+  return sender->id() == receiver.origin_id();
 }
 
 DEFINE_NATIVE_ENTRY(SendPort_sendInternal_, 0, 2) {
@@ -115,7 +112,8 @@ DEFINE_NATIVE_ENTRY(SendPort_sendInternal_, 0, 2) {
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, obj, arguments->NativeArgAt(1));
 
   const Dart_Port destination_port_id = port.Id();
-  const bool same_group = InSameGroup(isolate, port);
+  IsolateGroup* group = thread->isolate_group();
+  const bool same_group = InSameGroup(group, port);
 #if defined(DEBUG)
   if (same_group) {
     ASSERT(PortMap::IsReceiverInThisIsolateGroupOrClosed(destination_port_id,
@@ -505,11 +503,11 @@ DEFINE_NATIVE_ENTRY(Isolate_exit_, 0, 2) {
   if (!port.IsNull()) {
     GET_NATIVE_ARGUMENT(Instance, obj, arguments->NativeArgAt(1));
 
-    const bool same_group = InSameGroup(isolate, port);
+    IsolateGroup* group = thread->isolate_group();
+    const bool same_group = InSameGroup(group, port);
 #if defined(DEBUG)
     if (same_group) {
-      ASSERT(PortMap::IsReceiverInThisIsolateGroupOrClosed(port.Id(),
-                                                           isolate->group()));
+      ASSERT(PortMap::IsReceiverInThisIsolateGroupOrClosed(port.Id(), group));
     }
 #endif
     if (!same_group) {
@@ -557,7 +555,6 @@ DEFINE_NATIVE_ENTRY(Isolate_exit_, 0, 2) {
 class IsolateSpawnState {
  public:
   IsolateSpawnState(Dart_Port parent_port,
-                    Dart_Port origin_id,
                     const char* script_url,
                     PersistentHandle* closure_tuple_handle,
                     SerializedObjectBuffer* message_buffer,
@@ -582,7 +579,6 @@ class IsolateSpawnState {
   ~IsolateSpawnState();
 
   Dart_Port parent_port() const { return parent_port_; }
-  Dart_Port origin_id() const { return origin_id_; }
   Dart_Port on_exit_port() const { return on_exit_port_; }
   Dart_Port on_error_port() const { return on_error_port_; }
   const char* script_url() const { return script_url_; }
@@ -606,7 +602,6 @@ class IsolateSpawnState {
 
  private:
   Dart_Port parent_port_;
-  Dart_Port origin_id_ = ILLEGAL_PORT;
   Dart_Port on_exit_port_;
   Dart_Port on_error_port_;
   const char* script_url_;
@@ -630,7 +625,6 @@ static const char* NewConstChar(const char* chars) {
 }
 
 IsolateSpawnState::IsolateSpawnState(Dart_Port parent_port,
-                                     Dart_Port origin_id,
                                      const char* script_url,
                                      PersistentHandle* closure_tuple_handle,
                                      SerializedObjectBuffer* message_buffer,
@@ -642,7 +636,6 @@ IsolateSpawnState::IsolateSpawnState(Dart_Port parent_port,
                                      const char* debug_name,
                                      IsolateGroup* isolate_group)
     : parent_port_(parent_port),
-      origin_id_(origin_id),
       on_exit_port_(on_exit_port),
       on_error_port_(on_error_port),
       script_url_(script_url),
@@ -876,11 +869,6 @@ class SpawnIsolateTask : public ThreadPool::Task {
       return;
     }
 
-    if (state_->origin_id() != ILLEGAL_PORT) {
-      // origin_id is set to parent isolate main port id when spawning via
-      // spawnFunction.
-      child->set_origin_id(state_->origin_id());
-    }
     bool errors_are_fatal = state_->errors_are_fatal();
     Dart_Port on_error_port = state_->on_error_port();
     Dart_Port on_exit_port = state_->on_exit_port();
@@ -1119,10 +1107,9 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 0, 10) {
   }
 
   std::unique_ptr<IsolateSpawnState> state(new IsolateSpawnState(
-      port.Id(), isolate->origin_id(), String2UTF8(script_uri),
-      closure_tuple_handle, &message_buffer, utf8_package_config,
-      paused.value(), fatal_errors, on_exit_port, on_error_port,
-      utf8_debug_name, isolate->group()));
+      port.Id(), String2UTF8(script_uri), closure_tuple_handle, &message_buffer,
+      utf8_package_config, paused.value(), fatal_errors, on_exit_port,
+      on_error_port, utf8_debug_name, isolate->group()));
 
   isolate->group()->thread_pool()->Run<SpawnIsolateTask>(isolate,
                                                          std::move(state));

runtime/observatory/lib/src/elements/debugger.dart

@@ -1108,7 +1108,6 @@ class IsolateListCommand extends DebuggerCommand {
       String current = (isolate == debugger.isolate ? '*' : '');
       debugger.console
           .print("${isolate.number.toString().padLeft(maxIdLen, ' ')} "
-              "${isolate.originNumber.toString().padLeft(maxIdLen, ' ')} "
               "${isolate.name!.padRight(maxNameLen, ' ')} "
               "${_isolateRunState(isolate).padRight(maxRunStateLen, ' ')} "
               "${current}");

runtime/observatory/lib/src/service/object.dart

@@ -1404,7 +1404,6 @@ class Isolate extends ServiceObjectOwner implements M.Isolate {
   VM get vm => owner as VM;
   Isolate get isolate => this;
   int? number;
-  int? originNumber;
   DateTime? startTime;
   Duration? get upTime {
     if (startTime == null) {
@@ -1681,7 +1680,6 @@ class Isolate extends ServiceObjectOwner implements M.Isolate {
     loading = false;
     runnable = map['runnable'] == true;
     _upgradeCollection(map, isolate);
-    originNumber = int.tryParse(map['_originNumber']);
     if (map['rootLib'] != null) {
       rootLibrary = map['rootLib'];
     }

runtime/observatory/tests/service/get_isolate_rpc_test.dart

@@ -20,7 +20,6 @@ var tests = <VMTest>[
     expect(result['isolateFlags'].length, isPositive);
     expect(result['isSystemIsolate'], isFalse);
     expect(result['isolateGroupId'], startsWith('isolateGroups/'));
-    expect(result['_originNumber'], equals(result['number']));
     expect(result['startTime'], isPositive);
     expect(result['livePorts'], isPositive);
     expect(result['pauseOnExit'], isFalse);

runtime/observatory/tests/service/http_get_isolate_rpc_common.dart

@@ -64,7 +64,6 @@ Future<Null> testeeBefore() async {
     Expect.equals(result['type'], 'Isolate');
     Expect.isTrue(result['id'].startsWith('isolates/'));
     Expect.type<String>(result['number']);
-    Expect.equals(result['_originNumber'], result['number']);
     Expect.isTrue(result['startTime'] > 0);
     Expect.isTrue(result['livePorts'] > 0);
     Expect.isFalse(result['pauseOnExit']);

runtime/vm/dart_api_impl.cc

@@ -1403,7 +1403,6 @@ Dart_CreateIsolateInGroup(Dart_Isolate group_member,
   Isolate* isolate;
   isolate = CreateWithinExistingIsolateGroup(member->group(), name, error);
   if (isolate != nullptr) {
-    isolate->set_origin_id(member->origin_id());
     isolate->set_init_callback_data(child_isolate_data);
     isolate->set_on_shutdown_callback(shutdown_callback);
     isolate->set_on_cleanup_callback(cleanup_callback);

runtime/vm/isolate.cc

@@ -110,7 +110,7 @@ class VerifyOriginId : public IsolateVisitor {
  public:
   explicit VerifyOriginId(Dart_Port id) : id_(id) {}
 
-  void VisitIsolate(Isolate* isolate) { ASSERT(isolate->origin_id() != id_); }
+  void VisitIsolate(Isolate* isolate) { ASSERT(isolate->group()->id() != id_); }
 
  private:
   Dart_Port id_;
@@ -733,7 +733,7 @@ void IsolateGroup::ForEach(std::function<void(IsolateGroup*)> action) {
 }
 
 void IsolateGroup::RunWithIsolateGroup(
-    uint64_t id,
+    Dart_Port id,
     std::function<void(IsolateGroup*)> action,
     std::function<void()> not_found) {
   ReadRwLocker wl(Thread::Current(), isolate_groups_rwlock_);
@@ -1944,7 +1944,6 @@ Isolate* Isolate::InitIsolate(const char* name_prefix,
   VerifyOriginId id_verifier(result->main_port());
   Isolate::VisitIsolates(&id_verifier);
 #endif
-  result->set_origin_id(result->main_port());
 
   // First we ensure we enter the isolate. This will ensure we're participating
   // in any safepointing requests from this point on. Other threads requesting a
@@ -2062,17 +2061,6 @@ int64_t Isolate::UptimeMicros() const {
   return OS::GetCurrentMonotonicMicros() - start_time_micros_;
 }
 
-Dart_Port Isolate::origin_id() {
-  MutexLocker ml(&origin_id_mutex_);
-  return origin_id_;
-}
-
-void Isolate::set_origin_id(Dart_Port id) {
-  MutexLocker ml(&origin_id_mutex_);
-  ASSERT((id == main_port_ && origin_id_ == 0) || (origin_id_ == main_port_));
-  origin_id_ = id;
-}
-
 void Isolate::set_finalizers(const GrowableObjectArray& value) {
   finalizers_ = value.ptr();
 }
@@ -3217,8 +3205,6 @@ void Isolate::PrintJSON(JSONStream* stream, bool ref) {
   if (ref) {
     return;
   }
-  jsobj.AddPropertyF("_originNumber", "%" Pd64 "",
-                     static_cast<int64_t>(origin_id()));
   int64_t uptime_millis = UptimeMicros() / kMicrosecondsPerMillisecond;
   int64_t start_time = OS::GetCurrentTimeMillis() - uptime_millis;
   jsobj.AddPropertyTimeMillis("startTime", start_time);

runtime/vm/isolate.h

@@ -679,13 +679,13 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   Become* become() const { return become_; }
   void set_become(Become* become) { become_ = become; }
 
-  uint64_t id() const { return id_; }
+  Dart_Port id() const { return id_; }
 
   static void Init();
   static void Cleanup();
 
   static void ForEach(std::function<void(IsolateGroup*)> action);
-  static void RunWithIsolateGroup(uint64_t id,
+  static void RunWithIsolateGroup(Dart_Port id,
                                   std::function<void(IsolateGroup*)> action,
                                   std::function<void()> not_found);
 
@@ -886,7 +886,7 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   static IntrusiveDList<IsolateGroup>* isolate_groups_;
   static Random* isolate_group_random_;
 
-  uint64_t id_ = 0;
+  Dart_Port id_ = 0;
 
   std::unique_ptr<StoreBuffer> store_buffer_;
   std::unique_ptr<Heap> heap_;
@@ -1065,8 +1065,6 @@ class Isolate : public IntrusiveDListEntry<Isolate> {
     ASSERT(main_port_ == 0);  // Only set main port once.
     main_port_ = port;
   }
-  Dart_Port origin_id();
-  void set_origin_id(Dart_Port id);
   void set_pause_capability(uint64_t value) { pause_capability_ = value; }
   uint64_t pause_capability() const { return pause_capability_; }
   void set_terminate_capability(uint64_t value) {
@@ -1665,9 +1663,6 @@ class Isolate : public IntrusiveDListEntry<Isolate> {
   Dart_IsolateCleanupCallback on_cleanup_callback_ = nullptr;
   char* name_ = nullptr;
   Dart_Port main_port_ = 0;
-  // Isolates created by Isolate.spawn have the same origin id.
-  Dart_Port origin_id_ = 0;
-  Mutex origin_id_mutex_;
   uint64_t pause_capability_ = 0;
   uint64_t terminate_capability_ = 0;
   void* init_callback_data_ = nullptr;

runtime/vm/object.cc

@@ -26408,7 +26408,7 @@ ReceivePortPtr ReceivePort::New(Dart_Port id,
   Thread* thread = Thread::Current();
   Zone* zone = thread->zone();
   const SendPort& send_port =
-      SendPort::Handle(zone, SendPort::New(id, thread->isolate()->origin_id()));
+      SendPort::Handle(zone, SendPort::New(id, thread->isolate_group()->id()));
 #if !defined(PRODUCT)
   const StackTrace& allocation_location_ =
       HasStack() ? GetCurrentStackTrace(0) : StackTrace::Handle();

runtime/vm/port.cc

@@ -203,7 +203,7 @@ Dart_Port PortMap::GetOriginId(Dart_Port id) {
     // Message handler is a native port instead of an isolate.
     return ILLEGAL_PORT;
   }
-  return isolate->origin_id();
+  return isolate->group()->id();
 }
 
 bool PortMap::IsOwnedByCurrentThread(Dart_Port id) {

runtime/vm/service.cc

@@ -1642,7 +1642,7 @@ static void ActOnIsolateGroup(JSONStream* js,
     PrintInvalidParamError(js, "isolateGroupId");
     return;
   }
-  uint64_t isolate_group_id = UInt64Parameter::Parse(
+  Dart_Port isolate_group_id = Int64Parameter::Parse(
       String::Handle(String::SubString(s, prefix.Length())).ToCString());
   IsolateGroup::RunWithIsolateGroup(
       isolate_group_id,

runtime/vm/service_isolate.cc

@@ -151,7 +151,7 @@ bool ServiceIsolate::IsRunning() {
 
 bool ServiceIsolate::IsServiceIsolateDescendant(Isolate* isolate) {
   MonitorLocker ml(monitor_);
-  return isolate->origin_id() == origin_;
+  return isolate->group()->id() == origin_;
 }
 
 Dart_Port ServiceIsolate::Port() {
@@ -297,7 +297,7 @@ void ServiceIsolate::SetServiceIsolate(Isolate* isolate) {
   isolate_ = isolate;
   if (isolate_ != nullptr) {
     ASSERT(isolate->is_service_isolate());
-    origin_ = isolate_->origin_id();
+    origin_ = isolate_->group()->id();
   }
 }