Use list of isolates in profiler

runtime/vm/isolate.cc

Index: runtime/vm/isolate.cc
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index 10a157831940c77e053a422ed2e7a342e9aba3cd..0194308bf0ddcce88b2ebda253ccbd9f51ecbfe1 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -314,6 +314,7 @@ Isolate::Isolate()
       object_histogram_(NULL),
       object_id_ring_(NULL),
       profiler_data_(NULL),
+      thread_state_(NULL),
       REUSABLE_HANDLE_LIST(REUSABLE_HANDLE_INITIALIZERS)
       reusable_handles_() {
   if (FLAG_print_object_histogram && (Dart::vm_isolate() != NULL)) {
@@ -342,12 +343,20 @@ Isolate::~Isolate() {
   delete spawn_state_;
 }
 
+
 void Isolate::SetCurrent(Isolate* current) {
   Isolate* old_current = Current();
-  if (old_current != current) {
+  if (old_current != NULL) {
+    old_current->set_thread_state(NULL);
     Profiler::EndExecution(old_current);
-    Thread::SetThreadLocal(isolate_key, reinterpret_cast<uword>(current));
+  }
+  Thread::SetThreadLocal(isolate_key, reinterpret_cast<uword>(current));
+  if (current != NULL) {

[siva, 2014/02/05 19:09:15]

We also need
ASSERT(current->thread_state() == NULL);

[Cutch, 2014/02/05 23:00:31]

Done.

+#if defined(DEBUG)
+    CheckNoIsolateHasThreadData(ThreadInterrupter::GetCurrentThreadState());
+#endif
     Profiler::BeginExecution(current);
+    current->set_thread_state(ThreadInterrupter::GetCurrentThreadState());

[siva, 2014/02/05 19:09:15]

Why not have a local variable
InterruptibleThreadState* current_thread_state =
ThreadInterrupter::GetCurrentThreadState();

and use that local in the two spots.

[Cutch, 2014/02/05 23:00:31]

Done.

   }
 }
 
@@ -364,6 +373,8 @@ void Isolate::InitOnce() {
   isolate_key = Thread::CreateThreadLocal();
   ASSERT(isolate_key != Thread::kUnsetThreadLocalKey);
   create_callback_ = NULL;
+  isolates_monitor_ = new Monitor();
+  ASSERT(isolates_monitor_ != NULL);
 }
 
 
@@ -374,6 +385,9 @@ Isolate* Isolate::Init(const char* name_prefix) {
   // Setup for profiling.
   Profiler::InitProfilingForIsolate(result);
 
+  // Add to isolate list.
+  IsolateCreated(result);
+
   // TODO(5411455): For now just set the recently created isolate as
   // the current isolate.
   SetCurrent(result);
@@ -748,6 +762,7 @@ void Isolate::Shutdown() {
   // TODO(5411455): For now just make sure there are no current isolates
   // as we are shutting down the isolate.
   SetCurrent(NULL);
+  IsolateShutdown(this);
   Profiler::ShutdownProfilingForIsolate(this);
 }
 
@@ -765,6 +780,11 @@ Dart_EntropySource Isolate::entropy_source_callback_ = NULL;
 Dart_IsolateInterruptCallback Isolate::vmstats_callback_ = NULL;
 Dart_ServiceIsolateCreateCalback Isolate::service_create_callback_ = NULL;
 
+Monitor* Isolate::isolates_monitor_ = NULL;
+Isolate** Isolate::isolates_ = NULL;
+intptr_t Isolate::isolates_capacity_ = 0;
+intptr_t Isolate::isolates_size_ = 0;
+
 void Isolate::VisitObjectPointers(ObjectPointerVisitor* visitor,
                                   bool visit_prologue_weak_handles,
                                   bool validate_frames) {
@@ -865,6 +885,66 @@ void Isolate::PrintToJSONStream(JSONStream* stream) {
 }
 
 
+void Isolate::IsolateCreated(Isolate* isolate) {
+  MonitorLocker ml(isolates_monitor_);
+  // Not currently registered in the isolates list.
+  ASSERT(FindIsolateIndex(isolate) < 0);
+  if (isolates_size_ == isolates_capacity_) {
+    // Full, grow isolates array.
+    ResizeIsolates(isolates_capacity_ == 0 ? 16 : isolates_capacity_ * 2);
+  }
+  // Add isolate to table.
+  isolates_[isolates_size_] = isolate;
+  isolates_size_++;
+}
+
+
+void Isolate::IsolateShutdown(Isolate* isolate) {
+  MonitorLocker ml(isolates_monitor_);
+  intptr_t index = FindIsolateIndex(isolate);
+  ASSERT(index >= 0);
+  intptr_t last = isolates_size_ - 1;
+  if (index != last) {
+    isolates_[index] = isolates_[last];
+  }
+  isolates_[last] = NULL;
+  isolates_size_--;
+}
+
+
+void Isolate::ResizeIsolates(intptr_t new_capacity) {
+  // Must be called with isolates_monitor_ locked.
+  ASSERT(new_capacity > isolates_capacity_);
+  isolates_ = reinterpret_cast<Isolate**>(
+      realloc(isolates_, sizeof(*isolates_) * new_capacity));
+  for (intptr_t i = isolates_capacity_; i < new_capacity; i++) {
+    isolates_[i] = NULL;
+  }
+  isolates_capacity_ = new_capacity;
+}

[siva, 2014/02/05 19:09:15]

See comment on using a linked list for isolates list.

+
+
+intptr_t Isolate::FindIsolateIndex(Isolate* isolate) {
+  // Must be called with isolates_monitor_ locked.
+  for (intptr_t i = 0; i < isolates_size_; i++) {
+    if (isolates_[i] == isolate) {
+      return i;
+    }
+  }
+  return -1;
+}

[siva, 2014/02/05 19:09:15]

If a singly linked list is used for isolates this function could also
be under
#if defined(DEBUG)
....
#endif
and could be named CheckForDuplicateIsolates

+
+
+void Isolate::CheckNoIsolateHasThreadData(InterruptableThreadState* state) {

[siva, 2014/02/05 19:09:15]

I would call this CheckForDuplicateThreadState and this function can be under
#if defined(DEBUG)
...
#endif
I suppose.

[Cutch, 2014/02/05 23:00:31]

Done.

+  ASSERT(state != NULL);
+  MonitorLocker ml(isolates_monitor_);
+  for (intptr_t i = 0; i < isolates_size_; i++) {
+    Isolate* isolate = isolates_[i];
+    ASSERT(isolate->thread_state() != state);
+  }
+}
+
+
 template<class T>
 T* Isolate::AllocateReusableHandle() {
   T* handle = reinterpret_cast<T*>(reusable_handles_.AllocateScopedHandle());