Isolate.exit uses Isolate::KillIfExists which is O(num_isolates) operation

[mraleph]

I was a bit curious why this code from Reddit is so slow. Turns out that Isolate.exit calls Isolate::KillIfExists which visits all the isolates and does the following:

  void VisitIsolate(Isolate* isolate) {
    MonitorLocker ml(Isolate::isolate_creation_monitor_);
    ASSERT(isolate != nullptr);
    if (ShouldKill(isolate)) {
      if (isolate->AcceptsMessagesLocked()) {
        isolate->KillLocked(msg_id_);
      }
    }
  }

  bool ShouldKill(Isolate* isolate) {
    // If a target_ is specified, then only kill the target_.
    // Otherwise, don't kill the service isolate or vm isolate.
    return (((target_ != nullptr) && (isolate == target_)) ||
            ((target_ == nullptr) && !IsSystemIsolate(isolate)));
  }

This seems wrong - it is an extremely expensive operation when number of isolates is large: it takes lock when visiting each isolate, so there is potential for high contention when many isolates are calling Isolate.exit at the same time.

I don't think Isolate::KillIfExists should be used when we already own the isolate like with Isolate.exit. KillIfExists is for killing other isolates in a race free manner.