Implement sampling profiler (chromium side change)

base/debug/trace_event_impl.cc

Index: base/debug/trace_event_impl.cc
diff --git a/base/debug/trace_event_impl.cc b/base/debug/trace_event_impl.cc
index e774f621e0431b512e06ba20b4baaf305189df8a..591830b12d27afa4d2a05bea7a4857fca9682091 100644
--- a/base/debug/trace_event_impl.cc
+++ b/base/debug/trace_event_impl.cc
@@ -1094,7 +1094,7 @@ void TraceLog::ThreadLocalEventBuffer::FlushWhileLocked() {
 
   trace_log_->lock_.AssertAcquired();
   if (trace_log_->CheckGeneration(generation_)) {
-    // Return the chunk to the buffer only if the generation matches,
+    // Return the chunk to the buffer only if the generation matches.
     trace_log_->logged_events_->ReturnChunk(chunk_index_, chunk_.Pass());
   }
   // Otherwise this method may be called from the destructor, or TraceLog will
@@ -1107,7 +1107,7 @@ TraceLog* TraceLog::GetInstance() {
 }
 
 TraceLog::TraceLog()
-    : enabled_(false),
+    : mode_(DISABLED),
       num_traces_recorded_(0),
       event_callback_(0),
       dispatching_to_observer_list_(false),
@@ -1153,7 +1153,7 @@ TraceLog::TraceLog()
 
     LOG(ERROR) << "Start " << switches::kTraceToConsole
                << " with CategoryFilter '" << filter << "'.";
-    SetEnabled(CategoryFilter(filter), ECHO_TO_CONSOLE);
+    SetEnabled(CategoryFilter(filter), RECORDING_MODE, ECHO_TO_CONSOLE);
   }
 #endif
 
@@ -1192,8 +1192,12 @@ const char* TraceLog::GetCategoryGroupName(
 void TraceLog::UpdateCategoryGroupEnabledFlag(int category_index) {
   unsigned char enabled_flag = 0;
   const char* category_group = g_category_groups[category_index];
-  if (enabled_ && category_filter_.IsCategoryGroupEnabled(category_group))
+  if (mode_ == RECORDING_MODE &&
+      category_filter_.IsCategoryGroupEnabled(category_group))
     enabled_flag |= ENABLED_FOR_RECORDING;
+  if (mode_ == MONITORING_MODE &&
+      category_filter_.IsCategoryGroupEnabled(category_group))
+    enabled_flag |= ENABLED_FOR_MONITORING;
   if (event_callback_ &&
       event_callback_category_filter_.IsCategoryGroupEnabled(category_group))
     enabled_flag |= ENABLED_FOR_EVENT_CALLBACK;
@@ -1256,6 +1260,7 @@ void TraceLog::GetKnownCategoryGroups(
 }
 
 void TraceLog::SetEnabled(const CategoryFilter& category_filter,
+                          Mode mode,
                           Options options) {
   std::vector<EnabledStateObserver*> observer_list;
   {
@@ -1266,7 +1271,7 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
 
     Options old_options = trace_options();
 
-    if (enabled_) {
+    if (IsEnabled()) {
       if (options != old_options) {
         DLOG(ERROR) << "Attemting to re-enable tracing with a different "
                     << "set of options.";
@@ -1283,7 +1288,7 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
       return;
     }
 
-    enabled_ = true;
+    mode_ = mode;
 
     if (options != old_options) {
       subtle::NoBarrier_Store(&trace_options_, options);
@@ -1295,7 +1300,7 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
     category_filter_ = CategoryFilter(category_filter);
     UpdateCategoryGroupEnabledFlags();
 
-    if ((options & ENABLE_SAMPLING) || (options & MONITOR_SAMPLING)) {
+    if (options & ENABLE_SAMPLING) {
       sampling_thread_.reset(new TraceSamplingThread);
       sampling_thread_->RegisterSampleBucket(
           &g_trace_state[0],
@@ -1341,7 +1346,7 @@ void TraceLog::SetDisabled() {
 void TraceLog::SetDisabledWhileLocked() {
   lock_.AssertAcquired();
 
-  if (!enabled_)
+  if (!IsEnabled())
     return;
 
   if (dispatching_to_observer_list_) {
@@ -1350,7 +1355,7 @@ void TraceLog::SetDisabledWhileLocked() {
     return;
   }
 
-  enabled_ = false;
+  mode_ = DISABLED;
 
   if (sampling_thread_.get()) {
     // Stop the sampling thread.
@@ -1384,7 +1389,7 @@ void TraceLog::SetDisabledWhileLocked() {
 
 int TraceLog::GetNumTracesRecorded() {
   AutoLock lock(lock_);
-  if (!enabled_)
+  if (!IsEnabled())
     return -1;
   return num_traces_recorded_;
 }
@@ -1425,7 +1430,7 @@ TraceBuffer* TraceLog::CreateTraceBuffer() {
   Options options = trace_options();
   if (options & RECORD_CONTINUOUSLY)
     return new TraceBufferRingBuffer(kTraceEventRingBufferChunks);
-  else if (options & MONITOR_SAMPLING)
+  else if ((options & ENABLE_SAMPLING) && mode_ == MONITORING_MODE)
     return new TraceBufferRingBuffer(kMonitorTraceEventBufferChunks);
   else if (options & ECHO_TO_CONSOLE)
     return new TraceBufferRingBuffer(kEchoToConsoleTraceEventBufferChunks);
@@ -1437,6 +1442,7 @@ TraceEvent* TraceLog::AddEventToThreadSharedChunkWhileLocked(
   lock_.AssertAcquired();
 
   if (thread_shared_chunk_ && thread_shared_chunk_->IsFull()) {
+    // Return the chunk to the buffer only if the generation matches.
     logged_events_->ReturnChunk(thread_shared_chunk_index_,
                                 thread_shared_chunk_.Pass());
   }
@@ -1693,6 +1699,7 @@ TraceEventHandle TraceLog::AddTraceEventWithThreadIdAndTimestamp(
     const scoped_refptr<ConvertableToTraceFormat>* convertable_values,
     unsigned char flags) {
   TraceEventHandle handle = { 0, 0, 0 };
+
   if (!*category_group_enabled)
     return handle;
 
@@ -1769,7 +1776,11 @@ TraceEventHandle TraceLog::AddTraceEventWithThreadIdAndTimestamp(
   }
 
   std::string console_message;
-  if ((*category_group_enabled & ENABLED_FOR_RECORDING)) {
+  if ((mode_ == RECORDING_MODE &&
+       *category_group_enabled & ENABLED_FOR_RECORDING) ||
+      (mode_ == MONITORING_MODE &&
+       *category_group_enabled & ENABLED_FOR_MONITORING) &&
+       phase == TRACE_EVENT_PHASE_SAMPLE) {

[haraken, 2013/12/18 21:48:10]

Concern 1: There is a threading race in mode_. mode_ is set by the FILE thread
and accessed by the UI thread. We don't want to use a lock to access mode_,
because AddTraceEvent() is always called by TRACE_EVENT macros and thus
performance-sensitive.

Concern 2: This implementation will slow down TRACE_EVENT macros. Imagine that
the monitoring mode is enabled for all categories. In this case, all TRACE_EVENT
macros (for the recording mode) start calling AddTraceEvent() and then reach
this check. Since this check returns false, AddTraceEvent() returns without
doing nothing. However, it would be heavy to execute AddTraceEvent() for all
TRACE_EVENT macros. (We want to enable the sampling profiler always on in the
future.)

[Xianzhu, 2013/12/18 22:26:54]

I think we can omit checking mode_ here because we should trust
*category_group_enabled which has been updated according to mode_ in
UpdateCategoryEnabledFlags().
This doesn't look good. Currently the extra cost is less than 3~4 machine
instructions for each TRACE_EVENT macro when it doesn't actually generate
traces.

I think we should also add the check in the macros:
- in sampling macros, change "if (*category_group_enabled)" to "if
(*category_group_enabled & ENABLED_FOR_MONITORING)"
- in other macros, change to "if (*category_group_enabled &
(ENABLED_FOR_RECORDING | ENABLED_FOR_EVENT_CALLBACK))"
so that for recording macros, the extra cost is still near zero when recording
and event_callback are not enabled.

[haraken, 2013/12/18 22:40:21]

Would you elaborate on how to do that?

(1) TRACE_EVENT macros are just checking *category_group_enabled. They are not
checking *category_group_enabled & ENABLED_FOR_XXX. We want to keep the current
check for performance.

(2) If we want to enable some categories in the monitoring mode, we need to
enable *category_group_enabled for the categories.

(3) Then TRACE_EVENT macros start falling back to AddTraceEvent().

Are you suggesting using two category_group_enabled, one for the recording mode
and the other for the monitoring mode?
Actually I tried this at first but found that it won't make sense to distinguish
ENABLED_FOR_RECORDING from ENABLED_FOR_MONITORING. Because the recording mode
and the monitoring mode are mutually exclusive, we can just use
category_group_enabled for a category filter of the mode that is currently
enabled.

     OptionalAutoLock lock(lock_);
 
     TraceEvent* trace_event = NULL;