Added isolate + thread high watermark tracking to Observatory

runtime/vm/isolate.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/isolate.h"
 
 #include "include/dart_api.h"
 #include "include/dart_native_api.h"
 #include "platform/assert.h"
 #include "platform/text_buffer.h"
@@ skipping 758 matching lines @@
     : store_buffer_(new StoreBuffer()),
       heap_(NULL),
       user_tag_(0),
       current_tag_(UserTag::null()),
       default_tag_(UserTag::null()),
       object_store_(NULL),
       class_table_(),
       single_step_(false),
       thread_registry_(new ThreadRegistry()),
       safepoint_handler_(new SafepointHandler(this)),
+      isolate_memory_high_watermark_(0),
       message_notify_callback_(NULL),
       name_(NULL),
       debugger_name_(NULL),
       start_time_micros_(OS::GetCurrentMonotonicMicros()),
       main_port_(0),
       origin_id_(0),
       pause_capability_(0),
       terminate_capability_(0),
       errors_fatal_(true),
       init_callback_data_(NULL),
@@ skipping 1307 matching lines @@
     if (!handlers.IsNull()) {
       JSONArray extensions(&jsobj, "extensionRPCs");
       String& handler_name = String::Handle();
       for (intptr_t i = 0; i < handlers.Length(); i += kRegisteredEntrySize) {
         handler_name ^= handlers.At(i + kRegisteredNameIndex);
         extensions.AddValue(handler_name.ToCString());
       }
     }
   }
 
+  jsobj.AddProperty("isolateMemoryHighWatermark",

[Cutch, 2017/01/03 20:58:54]

Please make this private.

[bkonyi, 2017/01/03 22:00:38]

Done.

+                    isolate_memory_high_watermark_);
   jsobj.AddProperty("threads", thread_registry_);

[Cutch, 2017/01/03 20:58:54]

this too.

[bkonyi, 2017/01/03 22:00:38]

Done.

 }
 #endif
 
 
 void Isolate::set_tag_table(const GrowableObjectArray& value) {
   tag_table_ = value.raw();
 }
 
 
 void Isolate::set_current_tag(const UserTag& tag) {
@@ skipping 529 matching lines @@
     // If a safepoint operation is in progress wait for it
     // to finish before scheduling this thread in.
     while (!bypass_safepoint && safepoint_handler()->SafepointInProgress()) {
       ml.Wait();
     }
 
     // Now get a free Thread structure.
     thread = thread_registry()->GetFreeThreadLocked(this, is_mutator);
     ASSERT(thread != NULL);
 
+    thread->ResetHighWatermark();

[Cutch, 2017/01/03 20:58:54]

why would we be resetting the high watermark when it's scheduled? Doesn't this
lose information?

[bkonyi, 2017/01/03 22:00:38]

I was thinking once a thread was no longer being used and is part of the free
thread list, we would want to clear the watermark information before the thread
is reused later for a new task. If that's not correct, should I ever be clearing
the watermarks?

[Cutch, 2017/01/03 22:28:00]

No this was me remembering the way the threads *used* to be structured. I went
and read the code again and I believe your code here is correct.

+
     // Set up other values and set the TLS value.
     thread->isolate_ = this;
     ASSERT(heap() != NULL);
     thread->heap_ = heap();
     thread->set_os_thread(os_thread);
     ASSERT(thread->execution_state() == Thread::kThreadInNative);
     thread->set_execution_state(Thread::kThreadInVM);
     thread->set_safepoint_state(0);
     thread->set_vm_tag(VMTag::kVMTagId);
     ASSERT(thread->no_safepoint_scope_depth() == 0);
@@ skipping 26 matching lines @@
       thread->clear_sticky_error();
     }
   } else {
     ASSERT(thread->api_top_scope_ == NULL);
     ASSERT(thread->zone_ == NULL);
   }
   if (!bypass_safepoint) {
     // Ensure that the thread reports itself as being at a safepoint.
     thread->EnterSafepoint();
   }
+  UpdateIsolateHighWatermark();
   OSThread* os_thread = thread->os_thread();
   ASSERT(os_thread != NULL);
   os_thread->DisableThreadInterrupts();
   os_thread->set_thread(NULL);
   OSThread::SetCurrent(os_thread);
   if (is_mutator) {
     mutator_thread_ = NULL;
   }
   thread->isolate_ = NULL;
   thread->heap_ = NULL;
   thread->set_os_thread(NULL);
   thread->set_execution_state(Thread::kThreadInNative);
   thread->set_safepoint_state(Thread::SetAtSafepoint(true, 0));
   thread->clear_pending_functions();
   ASSERT(thread->no_safepoint_scope_depth() == 0);
   // Return thread structure.
   thread_registry()->ReturnThreadLocked(is_mutator, thread);
 }
 
 
+void Isolate::UpdateIsolateHighWatermark() {
+  intptr_t thread_watermarks_total =

[Cutch, 2017/01/03 20:58:54]

const intptr_t ...

Also, try and use more descriptive names that are consistent with each other.

For example:

UpdateIsolateHighWatermark() and isolate_memory_high_watermark_

The field `isolate_memory_high_watermark_` is already inside the isolate so drop
the isolate_: `memory_high_watermark_`.

Similarly with the function name. It's already defined on the Isolate class so
maybe call it UpdateMemoryHighWatermark()

[bkonyi, 2017/01/03 22:00:38]

Done.

+      thread_registry()->ThreadHighWatermarksTotalLocked();
+  if (thread_watermarks_total > isolate_memory_high_watermark_) {
+    isolate_memory_high_watermark_ = thread_watermarks_total;
+  }
+}
+
+
 static RawInstance* DeserializeObject(Thread* thread,
                                       uint8_t* obj_data,
                                       intptr_t obj_len) {
   if (obj_data == NULL) {
     return Instance::null();
   }
   MessageSnapshotReader reader(obj_data, obj_len, thread);
   Zone* zone = thread->zone();
   const Object& obj = Object::Handle(zone, reader.ReadObject());
   ASSERT(!obj.IsError());
@@ skipping 216 matching lines @@
 void IsolateSpawnState::DecrementSpawnCount() {
   ASSERT(spawn_count_monitor_ != NULL);
   ASSERT(spawn_count_ != NULL);
   MonitorLocker ml(spawn_count_monitor_);
   ASSERT(*spawn_count_ > 0);
   *spawn_count_ = *spawn_count_ - 1;
   ml.Notify();
 }
 
 }  // namespace dart