[ Profiler ] Add safepoint checks in profiler code to improve sample

streaming performance

TEST=CI

Change-Id: Iae536901a41f4b34207e022ec1e6321cfc2d74b1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/213380
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>

runtime/vm/debugger.cc

@@ -327,7 +327,7 @@ bool Debugger::NeedsDebugEvents() {
 
 static void InvokeEventHandler(ServiceEvent* event) {
   ASSERT(!event->IsPause());  // For pause events, call Pause instead.
-  Service::HandleEvent(event);
+  Service::HandleEvent(event, /*enter_safepoint*/ false);
 }
 
 ErrorPtr Debugger::PauseInterrupted() {

runtime/vm/heap/heap.cc

@@ -1020,7 +1020,7 @@ void Heap::RecordAfterGC(GCType type) {
           if (!Isolate::IsSystemIsolate(isolate)) {
             ServiceEvent event(isolate, ServiceEvent::kGC);
             event.set_gc_stats(&stats_);
-            Service::HandleEvent(&event);
+            Service::HandleEvent(&event, /*enter_safepoint*/ false);
           }
         },
         /*at_safepoint=*/true);

runtime/vm/heap/safepoint.h

@@ -461,6 +461,37 @@ class TransitionToVM : public TransitionSafepointState {
   DISALLOW_COPY_AND_ASSIGN(TransitionToVM);
 };
 
+// TransitionToNative is used to transition the safepoint state of a
+// thread from "running VM code" to "running native code"
+// and ensures that the state is reverted back to the initial state
+// when exiting the scope/frame.
+class TransitionToNative : public TransitionSafepointState {
+ public:
+  explicit TransitionToNative(Thread* T)
+      : TransitionSafepointState(T), execution_state_(T->execution_state()) {
+    ASSERT(T == Thread::Current());
+    ASSERT((execution_state_ == Thread::kThreadInVM) ||
+           (execution_state_ == Thread::kThreadInNative));
+    if (execution_state_ == Thread::kThreadInVM) {
+      T->set_execution_state(Thread::kThreadInNative);
+      T->EnterSafepoint();
+    }
+    ASSERT(T->execution_state() == Thread::kThreadInNative);
+  }
+
+  ~TransitionToNative() {
+    ASSERT(thread()->execution_state() == Thread::kThreadInNative);
+    if (execution_state_ == Thread::kThreadInVM) {
+      thread()->ExitSafepoint();
+      thread()->set_execution_state(Thread::kThreadInVM);
+    }
+  }
+
+ private:
+  uint32_t execution_state_;
+  DISALLOW_COPY_AND_ASSIGN(TransitionToNative);
+};
+
 }  // namespace dart
 
 #endif  // RUNTIME_VM_HEAP_SAFEPOINT_H_

runtime/vm/isolate.cc

@@ -2116,7 +2116,7 @@ void Isolate::MakeRunnableLocked() {
 #ifndef PRODUCT
   if (!Isolate::IsSystemIsolate(this) && Service::isolate_stream.enabled()) {
     ServiceEvent runnableEvent(this, ServiceEvent::kIsolateRunnable);
-    Service::HandleEvent(&runnableEvent);
+    Service::HandleEvent(&runnableEvent, /* enter_safepoint */ false);
   }
   GetRunnableLatencyMetric()->set_value(UptimeMicros());
 #endif  // !PRODUCT
@@ -2396,21 +2396,25 @@ void Isolate::ProcessFreeSampleBlocks(Thread* thread) {
     head = next;
   }
   head = reversed_head;
+
+  if (Service::profiler_stream.enabled() && !IsSystemIsolate(this)) {
+    SampleBlockListProcessor buffer(head);
+    StackZone zone(thread);
+    HandleScope handle_scope(thread);
+    Profile profile;
+    profile.Build(thread, nullptr, head);
+    ServiceEvent event(this, ServiceEvent::kCpuSamples);
+    event.set_cpu_profile(&profile);
+    Service::HandleEvent(&event);
+  }
+
   while (head != nullptr) {
-    if (Service::profiler_stream.enabled() && !IsSystemIsolate(this)) {
-      StackZone zone(thread);
-      HandleScope handle_scope(thread);
-      Profile profile;
-      profile.Build(thread, nullptr, head);
-      ServiceEvent event(this, ServiceEvent::kCpuSamples);
-      event.set_cpu_profile(&profile);
-      Service::HandleEvent(&event);
-    }
     SampleBlock* next = head->next_free_;
     head->next_free_ = nullptr;
     head->evictable_ = true;
     Profiler::sample_block_buffer()->FreeBlock(head);
     head = next;
+    thread->CheckForSafepoint();
   }
 }
 #endif  // !defined(PRODUCT)

runtime/vm/profiler.cc

@@ -24,6 +24,7 @@
 #include "vm/signal_handler.h"
 #include "vm/simulator.h"
 #include "vm/stack_frame.h"
+#include "vm/timeline.h"
 #include "vm/version.h"
 
 namespace dart {
@@ -243,6 +244,23 @@ void SampleBlockBuffer::ProcessCompletedBlocks() {
                      Dart_Timeline_Event_Duration, 0, nullptr, nullptr);
 }
 
+ProcessedSampleBuffer* SampleBlockListProcessor::BuildProcessedSampleBuffer(
+    SampleFilter* filter,
+    ProcessedSampleBuffer* buffer) {
+  ASSERT(filter != NULL);
+  Thread* thread = Thread::Current();
+  Zone* zone = thread->zone();
+
+  if (buffer == nullptr) {
+    buffer = new (zone) ProcessedSampleBuffer();
+  }
+  while (head_ != nullptr) {
+    head_->BuildProcessedSampleBuffer(filter, buffer);
+    head_ = head_->next_free_;
+  }
+  return buffer;
+}
+
 ProcessedSampleBuffer* SampleBlockBuffer::BuildProcessedSampleBuffer(
     SampleFilter* filter,
     ProcessedSampleBuffer* buffer) {
@@ -1552,13 +1570,17 @@ void CodeLookupTable::Build(Thread* thread) {
   // Clear.
   code_objects_.Clear();
 
+  thread->CheckForSafepoint();
   // Add all found Code objects.
   {
+    TimelineBeginEndScope tl(Timeline::GetIsolateStream(),
+                             "CodeLookupTable::Build HeapIterationScope");
     HeapIterationScope iteration(thread);
     CodeLookupTableBuilder cltb(this);
     iteration.IterateVMIsolateObjects(&cltb);
     iteration.IterateOldObjects(&cltb);
   }
+  thread->CheckForSafepoint();
 
   // Sort by entry.
   code_objects_.Sort(CodeDescriptor::Compare);
@@ -1632,6 +1654,7 @@ ProcessedSampleBuffer* SampleBuffer::BuildProcessedSampleBuffer(
 
   const intptr_t length = capacity();
   for (intptr_t i = 0; i < length; i++) {
+    thread->CheckForSafepoint();
     Sample* sample = At(i);
     if (sample->ignore_sample()) {
       // Bad sample.

runtime/vm/profiler.h

@@ -767,6 +767,7 @@ class SampleBlock : public SampleBuffer {
   SampleBlock* next_free_ = nullptr;
 
  private:
+  friend class SampleBlockListProcessor;
   friend class SampleBlockBuffer;
   friend class Isolate;
 
@@ -873,6 +874,20 @@ class SampleBlockBuffer : public ProcessedSampleBufferBuilder {
   DISALLOW_COPY_AND_ASSIGN(SampleBlockBuffer);
 };
 
+class SampleBlockListProcessor : public ProcessedSampleBufferBuilder {
+ public:
+  explicit SampleBlockListProcessor(SampleBlock* head) : head_(head) {}
+
+  virtual ProcessedSampleBuffer* BuildProcessedSampleBuffer(
+      SampleFilter* filter,
+      ProcessedSampleBuffer* buffer = nullptr);
+
+ private:
+  SampleBlock* head_;
+
+  DISALLOW_COPY_AND_ASSIGN(SampleBlockListProcessor);
+};
+
 class AllocationSampleBuffer : public SampleBuffer {
  public:
   explicit AllocationSampleBuffer(intptr_t capacity = 60000);

runtime/vm/profiler_service.cc

@@ -1013,6 +1013,7 @@ class ProfileBuilder : public ValueObject {
       RegisterLiveProfileCode(new ProfileCode(
           ProfileCode::kDartCode, code.PayloadStart(),
           code.PayloadStart() + code.Size(), code.compile_timestamp(), code));
+      thread_->CheckForSafepoint();
     }
 
     // Iterate over samples.
@@ -1047,6 +1048,7 @@ class ProfileBuilder : public ValueObject {
       }
 
       TickExitFrame(sample->vm_tag(), sample_index, sample);
+      thread_->CheckForSafepoint();
     }
     SanitizeMinMaxTimes();
   }
@@ -1095,18 +1097,21 @@ class ProfileBuilder : public ValueObject {
       ProfileCode* code = live_table->At(i);
       ASSERT(code != NULL);
       code->SetFunctionAndName(function_table);
+      thread_->CheckForSafepoint();
     }
 
     for (intptr_t i = 0; i < dead_table->length(); i++) {
       ProfileCode* code = dead_table->At(i);
       ASSERT(code != NULL);
       code->SetFunctionAndName(function_table);
+      thread_->CheckForSafepoint();
     }
 
     for (intptr_t i = 0; i < tag_table->length(); i++) {
       ProfileCode* code = tag_table->At(i);
       ASSERT(code != NULL);
       code->SetFunctionAndName(function_table);
+      thread_->CheckForSafepoint();
     }
   }
 
@@ -1725,6 +1730,7 @@ void Profile::PrintProfileJSON(JSONStream* stream, bool include_code_samples) {
 
 void Profile::PrintProfileJSON(JSONObject* obj, bool include_code_samples) {
   ScopeTimer sw("Profile::PrintProfileJSON", FLAG_trace_profiler);
+  Thread* thread = Thread::Current();
   obj->AddProperty("type", "CpuSamples");
   PrintHeaderJSON(obj);
   if (include_code_samples) {
@@ -1733,16 +1739,19 @@ void Profile::PrintProfileJSON(JSONObject* obj, bool include_code_samples) {
       ProfileCode* code = live_code_->At(i);
       ASSERT(code != NULL);
       code->PrintToJSONArray(&codes);
+      thread->CheckForSafepoint();
     }
     for (intptr_t i = 0; i < dead_code_->length(); i++) {
       ProfileCode* code = dead_code_->At(i);
       ASSERT(code != NULL);
       code->PrintToJSONArray(&codes);
+      thread->CheckForSafepoint();
     }
     for (intptr_t i = 0; i < tag_code_->length(); i++) {
       ProfileCode* code = tag_code_->At(i);
       ASSERT(code != NULL);
       code->PrintToJSONArray(&codes);
+      thread->CheckForSafepoint();
     }
   }
 
@@ -1752,9 +1761,11 @@ void Profile::PrintProfileJSON(JSONObject* obj, bool include_code_samples) {
       ProfileFunction* function = functions_->At(i);
       ASSERT(function != NULL);
       function->PrintToJSONArray(&functions);
+      thread->CheckForSafepoint();
     }
   }
   PrintSamplesJSON(obj, include_code_samples);
+  thread->CheckForSafepoint();
 }
 
 void ProfilerService::PrintJSONImpl(Thread* thread,

runtime/vm/service.cc

@@ -1106,7 +1106,7 @@ static void ReportPauseOnConsole(ServiceEvent* event) {
   }
 }
 
-void Service::HandleEvent(ServiceEvent* event) {
+void Service::HandleEvent(ServiceEvent* event, bool enter_safepoint) {
   if (event->stream_info() != NULL && !event->stream_info()->enabled()) {
     if (FLAG_warn_on_pause_with_no_debugger && event->IsPause()) {
       // If we are about to pause a running program which has no
@@ -1133,7 +1133,8 @@ void Service::HandleEvent(ServiceEvent* event) {
     params.AddProperty("streamId", stream_id);
     params.AddProperty("event", event);
   }
-  PostEvent(event->isolate(), stream_id, event->KindAsCString(), &js);
+  PostEvent(event->isolate(), stream_id, event->KindAsCString(), &js,
+            enter_safepoint);
 
   // Post event to the native Service Stream handlers if set.
   if (event->stream_info() != nullptr &&
@@ -1147,13 +1148,28 @@ void Service::HandleEvent(ServiceEvent* event) {
 void Service::PostEvent(Isolate* isolate,
                         const char* stream_id,
                         const char* kind,
-                        JSONStream* event) {
-  ASSERT(stream_id != NULL);
-  ASSERT(kind != NULL);
-  ASSERT(event != NULL);
+                        JSONStream* event,
+                        bool enter_safepoint) {
+  if (enter_safepoint) {
+    // Enter a safepoint so we don't block the mutator while processing
+    // large events.
+    TransitionToNative transition(Thread::Current());
+    PostEventImpl(isolate, stream_id, kind, event);
+    return;
+  }
+  PostEventImpl(isolate, stream_id, kind, event);
+}
+
+void Service::PostEventImpl(Isolate* isolate,
+                            const char* stream_id,
+                            const char* kind,
+                            JSONStream* event) {
+  ASSERT(stream_id != nullptr);
+  ASSERT(kind != nullptr);
+  ASSERT(event != nullptr);
 
   if (FLAG_trace_service) {
-    if (isolate != NULL) {
+    if (isolate != nullptr) {
       OS::PrintErr(
           "vm-service: Pushing ServiceEvent(isolate='%s', "
           "isolateId='" ISOLATE_SERVICE_ID_FORMAT_STRING

runtime/vm/service.h

@@ -98,7 +98,7 @@ class Service : public AllStatic {
   // Handles a message which is directed to a particular isolate.
   static ErrorPtr HandleIsolateMessage(Isolate* isolate, const Array& message);
 
-  static void HandleEvent(ServiceEvent* event);
+  static void HandleEvent(ServiceEvent* event, bool enter_safepoint = true);
 
   static void RegisterIsolateEmbedderCallback(
       const char* name,
@@ -231,7 +231,13 @@ class Service : public AllStatic {
   static void PostEvent(Isolate* isolate,
                         const char* stream_id,
                         const char* kind,
-                        JSONStream* event);
+                        JSONStream* event,
+                        bool enter_safepoint);
+
+  static void PostEventImpl(Isolate* isolate,
+                            const char* stream_id,
+                            const char* kind,
+                            JSONStream* event);
 
   static ErrorPtr MaybePause(Isolate* isolate, const Error& error);
 

runtime/vm/timeline.cc

@@ -1514,7 +1514,7 @@ void TimelineEventBlock::Finish() {
   if (Service::timeline_stream.enabled()) {
     ServiceEvent service_event(ServiceEvent::kTimelineEvents);
     service_event.set_timeline_event_block(this);
-    Service::HandleEvent(&service_event);
+    Service::HandleEvent(&service_event, /* enter_safepoint */ false);
   }
 #endif
 }