[tracing] Fix, simplify and speed up accounting of TraceEvent memory overhead

base/trace_event/trace_event_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/trace_event/trace_event_impl.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/base_switches.h"
@@ skipping 425 matching lines @@
 }
 
 TraceBufferChunk::~TraceBufferChunk() {
 }
 
 void TraceBufferChunk::Reset(uint32 new_seq) {
   for (size_t i = 0; i < next_free_; ++i)
     chunk_[i].Reset();
   next_free_ = 0;
   seq_ = new_seq;
-  cached_overhead_estimate_when_full_.reset();
+  cached_overhead_estimate_.reset();
 }
 
 TraceEvent* TraceBufferChunk::AddTraceEvent(size_t* event_index) {
   DCHECK(!IsFull());
   *event_index = next_free_++;
   return &chunk_[*event_index];
 }
 
 scoped_ptr<TraceBufferChunk> TraceBufferChunk::Clone() const {
   scoped_ptr<TraceBufferChunk> cloned_chunk(new TraceBufferChunk(seq_));
   cloned_chunk->next_free_ = next_free_;
   for (size_t i = 0; i < next_free_; ++i)
     cloned_chunk->chunk_[i].CopyFrom(chunk_[i]);
   return cloned_chunk.Pass();
 }
 
 void TraceBufferChunk::EstimateTraceMemoryOverhead(
     TraceEventMemoryOverhead* overhead) {
-  if (cached_overhead_estimate_when_full_) {
-    DCHECK(IsFull());
-    overhead->Update(*cached_overhead_estimate_when_full_);
+  if (!cached_overhead_estimate_) {
+    cached_overhead_estimate_.reset(new TraceEventMemoryOverhead);
+
+    // When estimating the size of TraceBufferChunk, exclude the array of trace
+    // events, as they are computed individually below.
+    cached_overhead_estimate_->Add("TraceBufferChunk",
+                                   sizeof(*this) - sizeof(chunk_));
+  }
+
+  const size_t num_cached_estimated_events =
+      cached_overhead_estimate_->GetCount("TraceEvent");
+  DCHECK_LE(num_cached_estimated_events, size());
+
+  if (IsFull() && num_cached_estimated_events == size()) {
+    overhead->Update(*cached_overhead_estimate_);
     return;
   }
 
-  // Cache the memory overhead estimate only if the chunk is full.
-  TraceEventMemoryOverhead* estimate = overhead;
+  for (size_t i = num_cached_estimated_events; i < size(); ++i)
+    chunk_[i].EstimateTraceMemoryOverhead(cached_overhead_estimate_.get());
+
   if (IsFull()) {
-    cached_overhead_estimate_when_full_.reset(new TraceEventMemoryOverhead);
-    estimate = cached_overhead_estimate_when_full_.get();
+    cached_overhead_estimate_->AddSelf();
+  } else {
+    // The unused TraceEvents in |chunks_| are not cached. They will keep
+    // changing as new TraceEvents are added to this chunk, so they are
+    // computed on the flight.

[dsinclair, 2015/07/22 20:37:31]

nit: s/flight/fly

[Primiano Tucci (use gerrit), 2015/07/22 21:55:23]

Oh.My mind is already in vacation mode, I guess I was thinking about how joyful
I'll be when catching a flight tomorrow :P

Done.

+    const size_t num_unused_trace_events = capacity() - size();
+    if (num_unused_trace_events) {
+      overhead->Add("TraceEvent (unused)",
+                    num_unused_trace_events * sizeof(TraceEvent));
+    }
   }
 
-  estimate->Add("TraceBufferChunk", sizeof(*this));
-  for (size_t i = 0; i < next_free_; ++i)
-    chunk_[i].EstimateTraceMemoryOverhead(estimate);
-
-  if (IsFull()) {
-    estimate->AddSelf();
-    overhead->Update(*estimate);
-  }
+  overhead->Update(*cached_overhead_estimate_);
 }
 
 // A helper class that allows the lock to be acquired in the middle of the scope
 // and unlocks at the end of scope if locked.
 class TraceLog::OptionalAutoLock {
  public:
   explicit OptionalAutoLock(Lock* lock) : lock_(lock), locked_(false) {}
 
   ~OptionalAutoLock() {
     if (locked_)
@@ skipping 176 matching lines @@
   }
 }
 
 void TraceEvent::Reset() {
   // Only reset fields that won't be initialized in Initialize(), or that may
   // hold references to other objects.
   duration_ = TimeDelta::FromInternalValue(-1);
   parameter_copy_storage_ = NULL;
   for (int i = 0; i < kTraceMaxNumArgs; ++i)
     convertable_values_[i] = NULL;
-  cached_memory_overhead_estimate_.reset();
 }
 
 void TraceEvent::UpdateDuration(const TraceTicks& now,
                                 const ThreadTicks& thread_now) {
   DCHECK_EQ(duration_.ToInternalValue(), -1);
   duration_ = now - timestamp_;
   thread_duration_ = thread_now - thread_timestamp_;
 }
 
 void TraceEvent::EstimateTraceMemoryOverhead(
     TraceEventMemoryOverhead* overhead) {
-  if (!cached_memory_overhead_estimate_) {
-    cached_memory_overhead_estimate_.reset(new TraceEventMemoryOverhead);
-    cached_memory_overhead_estimate_->Add("TraceEvent", sizeof(*this));
-    // TODO(primiano): parameter_copy_storage_ is refcounted and, in theory,
-    // could be shared by several events and we might overcount. In practice
-    // this is unlikely but it's worth checking.
-    if (parameter_copy_storage_) {
-      cached_memory_overhead_estimate_->AddRefCountedString(
-          *parameter_copy_storage_.get());
-    }
-    for (size_t i = 0; i < kTraceMaxNumArgs; ++i) {
-      if (arg_types_[i] == TRACE_VALUE_TYPE_CONVERTABLE) {
-        convertable_values_[i]->EstimateTraceMemoryOverhead(
-            cached_memory_overhead_estimate_.get());
-      }
-    }
-    cached_memory_overhead_estimate_->AddSelf();
+  overhead->Add("TraceEvent", sizeof(*this));
+
+  // TODO(primiano): parameter_copy_storage_ is refcounted and, in theory,
+  // could be shared by several events and we might overcount. In practice
+  // this is unlikely but it's worth checking.
+  if (parameter_copy_storage_)
+    overhead->AddRefCountedString(*parameter_copy_storage_.get());
+
+  for (size_t i = 0; i < kTraceMaxNumArgs; ++i) {
+    if (arg_types_[i] == TRACE_VALUE_TYPE_CONVERTABLE)
+      convertable_values_[i]->EstimateTraceMemoryOverhead(overhead);
   }
-  overhead->Update(*cached_memory_overhead_estimate_);
 }
 
 // static
 void TraceEvent::AppendValueAsJSON(unsigned char type,
                                    TraceEvent::TraceValue value,
                                    std::string* out) {
   switch (type) {
     case TRACE_VALUE_TYPE_BOOL:
       *out += value.as_bool ? "true" : "false";
       break;
@@ skipping 1823 matching lines @@
 }
 
 ScopedTraceBinaryEfficient::~ScopedTraceBinaryEfficient() {
   if (*category_group_enabled_) {
     TRACE_EVENT_API_UPDATE_TRACE_EVENT_DURATION(category_group_enabled_,
                                                 name_, event_handle_);
   }
 }
 
 }  // namespace trace_event_internal