[profiling] Fix BasicBlockProfiler implementation

Data object for tick collection are now created in a thread save way
and the block ids for counters use the same numbers as the printed
schedule.

Change-Id: I7a35088cb2fcd94a81067b16ad12b01d380fefa5
Reviewed-on: https://chromium-review.googlesource.com/1152923
Commit-Queue: Stephan Herhut <herhut@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55124}

src/basic-block-profiler.cc

@@ -4,6 +4,8 @@
 
 #include "src/basic-block-profiler.h"
 
+#include <algorithm>
+#include <numeric>
 #include <sstream>
 
 #include "src/base/lazy-instance.h"
@@ -21,8 +23,9 @@ BasicBlockProfiler* BasicBlockProfiler::Get() {
 }
 
 BasicBlockProfiler::Data::Data(size_t n_blocks)
-    : n_blocks_(n_blocks), block_ids_(n_blocks_), counts_(n_blocks_, 0) {}
-
+    : n_blocks_(n_blocks),
+      block_rpo_numbers_(n_blocks_),
+      counts_(n_blocks_, 0) {}
 
 BasicBlockProfiler::Data::~Data() {}
 
@@ -31,31 +34,31 @@ static void InsertIntoString(std::ostringstream* os, std::string* string) {
   string->insert(0, os->str());
 }
 
+static void InsertIntoString(const char* data, std::string* string) {
+  string->insert(0, data);
+}
 
 void BasicBlockProfiler::Data::SetCode(std::ostringstream* os) {
   InsertIntoString(os, &code_);
 }
 
-
-void BasicBlockProfiler::Data::SetFunctionName(std::ostringstream* os) {
-  InsertIntoString(os, &function_name_);
+void BasicBlockProfiler::Data::SetFunctionName(std::unique_ptr<char[]> name) {
+  InsertIntoString(name.get(), &function_name_);
 }
 
-
 void BasicBlockProfiler::Data::SetSchedule(std::ostringstream* os) {
   InsertIntoString(os, &schedule_);
 }
 
-
-void BasicBlockProfiler::Data::SetBlockId(size_t offset, size_t block_id) {
+void BasicBlockProfiler::Data::SetBlockRpoNumber(size_t offset,
+                                                 int32_t block_rpo) {
   DCHECK(offset < n_blocks_);
-  block_ids_[offset] = block_id;
+  block_rpo_numbers_[offset] = block_rpo;
 }
 
-
-uint32_t* BasicBlockProfiler::Data::GetCounterAddress(size_t offset) {
+intptr_t BasicBlockProfiler::Data::GetCounterAddress(size_t offset) {
   DCHECK(offset < n_blocks_);
-  return &counts_[offset];
+  return reinterpret_cast<intptr_t>(&(counts_[offset]));
 }
 
 
@@ -70,6 +73,7 @@ BasicBlockProfiler::BasicBlockProfiler() {}
 
 
 BasicBlockProfiler::Data* BasicBlockProfiler::NewData(size_t n_blocks) {
+  base::LockGuard<base::Mutex> lock(&data_list_mutex_);
   Data* data = new Data(n_blocks);
   data_list_.push_back(data);
   return data;
@@ -102,17 +106,33 @@ std::ostream& operator<<(std::ostream& os, const BasicBlockProfiler& p) {
 
 
 std::ostream& operator<<(std::ostream& os, const BasicBlockProfiler::Data& d) {
+  int block_count_sum = std::accumulate(d.counts_.begin(), d.counts_.end(), 0);
+  if (block_count_sum == 0) return os;
   const char* name = "unknown function";
   if (!d.function_name_.empty()) {
     name = d.function_name_.c_str();
   }
   if (!d.schedule_.empty()) {
-    os << "schedule for " << name << std::endl;
+    os << "schedule for " << name << " (B0 entered " << d.counts_[0]
+       << " times)" << std::endl;
     os << d.schedule_.c_str() << std::endl;
   }
   os << "block counts for " << name << ":" << std::endl;
+  std::vector<std::pair<int32_t, uint32_t>> pairs;
+  pairs.reserve(d.n_blocks_);
   for (size_t i = 0; i < d.n_blocks_; ++i) {
-    os << "block " << d.block_ids_[i] << " : " << d.counts_[i] << std::endl;
+    pairs.push_back(std::make_pair(d.block_rpo_numbers_[i], d.counts_[i]));
+  }
+  std::sort(pairs.begin(), pairs.end(),
+            [=](std::pair<int32_t, uint32_t> left,
+                std::pair<int32_t, uint32_t> right) {
+              if (right.second == left.second)
+                return left.first < right.first;
+              return right.second < left.second;
+            });
+  for (auto it : pairs) {
+    if (it.second == 0) break;
+    os << "block B" << it.first << " : " << it.second << std::endl;
   }
   os << std::endl;
   if (!d.code_.empty()) {

src/basic-block-profiler.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "src/base/macros.h"
+#include "src/base/platform/mutex.h"
 #include "src/globals.h"
 
 namespace v8 {
@@ -24,10 +25,10 @@ class BasicBlockProfiler {
     const uint32_t* counts() const { return &counts_[0]; }
 
     void SetCode(std::ostringstream* os);
-    void SetFunctionName(std::ostringstream* os);
+    void SetFunctionName(std::unique_ptr<char[]> name);
     void SetSchedule(std::ostringstream* os);
-    void SetBlockId(size_t offset, size_t block_id);
-    uint32_t* GetCounterAddress(size_t offset);
+    void SetBlockRpoNumber(size_t offset, int32_t block_rpo);
+    intptr_t GetCounterAddress(size_t offset);
 
    private:
     friend class BasicBlockProfiler;
@@ -40,7 +41,7 @@ class BasicBlockProfiler {
     void ResetCounts();
 
     const size_t n_blocks_;
-    std::vector<size_t> block_ids_;
+    std::vector<int32_t> block_rpo_numbers_;
     std::vector<uint32_t> counts_;
     std::string function_name_;
     std::string schedule_;
@@ -64,6 +65,7 @@ class BasicBlockProfiler {
       std::ostream& os, const BasicBlockProfiler& s);
 
   DataList data_list_;
+  base::Mutex data_list_mutex_;
 
   DISALLOW_COPY_AND_ASSIGN(BasicBlockProfiler);
 };

src/compiler/basic-block-instrumentor.cc

@@ -40,26 +40,23 @@ static NodeVector::iterator FindInsertionPoint(BasicBlock* block) {
 
 // TODO(dcarney): need to mark code as non-serializable.
 static const Operator* PointerConstant(CommonOperatorBuilder* common,
-                                       void* ptr) {
-  return kPointerSize == 8
-             ? common->Int64Constant(reinterpret_cast<intptr_t>(ptr))
-             : common->Int32Constant(
-                   static_cast<int32_t>(reinterpret_cast<intptr_t>(ptr)));
+                                       intptr_t ptr) {
+  return kPointerSize == 8 ? common->Int64Constant(ptr)
+                           : common->Int32Constant(static_cast<int32_t>(ptr));
 }
 
 BasicBlockProfiler::Data* BasicBlockInstrumentor::Instrument(
     OptimizedCompilationInfo* info, Graph* graph, Schedule* schedule,
     Isolate* isolate) {
+  // Basic block profiling disables concurrent compilation, so handle deref is
+  // fine.
+  AllowHandleDereference allow_handle_dereference;
   // Skip the exit block in profiles, since the register allocator can't handle
   // it and entry into it means falling off the end of the function anyway.
   size_t n_blocks = static_cast<size_t>(schedule->RpoBlockCount()) - 1;
   BasicBlockProfiler::Data* data = BasicBlockProfiler::Get()->NewData(n_blocks);
   // Set the function name.
-  if (info->has_shared_info()) {
-    std::ostringstream os;
-    info->shared_info()->Name()->PrintUC16(os);
-    data->SetFunctionName(&os);
-  }
+  data->SetFunctionName(info->GetDebugName());
   // Capture the schedule string before instrumentation.
   {
     std::ostringstream os;
@@ -76,7 +73,7 @@ BasicBlockProfiler::Data* BasicBlockInstrumentor::Instrument(
   for (BasicBlockVector::iterator it = blocks->begin(); block_number < n_blocks;
        ++it, ++block_number) {
     BasicBlock* block = (*it);
-    data->SetBlockId(block_number, block->id().ToSize());
+    data->SetBlockRpoNumber(block_number, block->rpo_number());
     // TODO(dcarney): wire effect and control deps for load and store.
     // Construct increment operation.
     Node* base = graph->NewNode(

src/isolate.cc

@@ -3010,7 +3010,7 @@ bool Isolate::Init(StartupDeserializer* des) {
     set_event_logger(Logger::DefaultEventLoggerSentinel);
   }
 
-  if (FLAG_trace_turbo || FLAG_trace_turbo_graph) {
+  if (FLAG_trace_turbo || FLAG_trace_turbo_graph || FLAG_turbo_profiling) {
     PrintF("Concurrent recompilation has been disabled for tracing.\n");
   } else if (OptimizingCompileDispatcher::Enabled()) {
     optimizing_compile_dispatcher_ = new OptimizingCompileDispatcher(this);