Add configurable limit to allocations in heap profiler.

base/trace_event/heap_profiler_heap_dump_writer.cc

 // Copyright 2015 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/heap_profiler_heap_dump_writer.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <iterator>
 #include <tuple>
 #include <utility>
 #include <vector>
 
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/heap_profiler_stack_frame_deduplicator.h"
 #include "base/trace_event/heap_profiler_type_name_deduplicator.h"
+#include "base/trace_event/memory_dump_session_state.h"
+#include "base/trace_event/trace_config.h"
 #include "base/trace_event/trace_event_argument.h"
+#include "base/trace_event/trace_log.h"
 
 // Most of what the |HeapDumpWriter| does is aggregating detailed information
 // about the heap and deciding what to dump. The Input to this process is a list
 // of |AllocationContext|s and size pairs.
 //
 // The pairs are grouped into |Bucket|s. A bucket is a group of (context, size)
 // pairs where the properties of the contexts share a prefix. (Type name is
 // considered a list of length one here.) First all pairs are put into one
 // bucket that represents the entire heap. Then this bucket is recursively
 // broken down into smaller buckets. Each bucket keeps track of whether further
@@ skipping 31 matching lines @@
 
   // When true, the type name for all elements in this bucket must be equal.
   bool is_broken_down_by_type_name;
 };
 
 // Comparison operator to order buckets by their size.
 bool operator<(const Bucket& lhs, const Bucket& rhs) {
   return lhs.size < rhs.size;
 }
 
-// Groups the allocations in the bucket by |breakBy|. The buckets in the
+// Groups the allocations in the bucket by |break_by|. The buckets in the

[Primiano Tucci (use gerrit), 2016/04/22 14:18:28]

nice thanks for fixing also nits in existing code :)

[Maria, 2016/04/25 18:37:23]

Acknowledged.

 // returned list will have |backtrace_cursor| advanced or
 // |is_broken_down_by_type_name| set depending on the property to group by.
-std::vector<Bucket> GetSubbuckets(const Bucket& bucket, BreakDownMode breakBy) {
+std::vector<Bucket> GetSubbuckets(const Bucket& bucket,
+                                  BreakDownMode break_by) {
   base::hash_map<const char*, Bucket> breakdown;
 
-  if (breakBy == BreakDownMode::kByBacktrace) {
+  if (break_by == BreakDownMode::kByBacktrace) {
     for (const auto& context_and_metrics : bucket.metrics_by_context) {
       const Backtrace& backtrace = context_and_metrics.first->backtrace;
       const char* const* begin = std::begin(backtrace.frames);
       const char* const* end = std::end(backtrace.frames);
       const char* const* cursor = begin + bucket.backtrace_cursor;
 
       // The backtrace in the context is padded with null pointers, but these
       // should not be considered for breakdown. Adjust end to point past the
       // last non-null frame.
       while (begin != end && *(end - 1) == nullptr)
         end--;
 
       DCHECK_LE(cursor, end);
 
       if (cursor != end) {
         Bucket& subbucket = breakdown[*cursor];
         subbucket.size += context_and_metrics.second.size;
         subbucket.count += context_and_metrics.second.count;
         subbucket.metrics_by_context.push_back(context_and_metrics);
         subbucket.backtrace_cursor = bucket.backtrace_cursor + 1;
         subbucket.is_broken_down_by_type_name =
             bucket.is_broken_down_by_type_name;
         DCHECK_GT(subbucket.size, 0u);
         DCHECK_GT(subbucket.count, 0u);
       }
     }
-  } else if (breakBy == BreakDownMode::kByTypeName) {
+  } else if (break_by == BreakDownMode::kByTypeName) {
     if (!bucket.is_broken_down_by_type_name) {
       for (const auto& context_and_metrics : bucket.metrics_by_context) {
         const AllocationContext* context = context_and_metrics.first;
         Bucket& subbucket = breakdown[context->type_name];
         subbucket.size += context_and_metrics.second.size;
         subbucket.count += context_and_metrics.second.count;
         subbucket.metrics_by_context.push_back(context_and_metrics);
         subbucket.backtrace_cursor = bucket.backtrace_cursor;
         subbucket.is_broken_down_by_type_name = true;
         DCHECK_GT(subbucket.size, 0u);
         DCHECK_GT(subbucket.count, 0u);
       }
     }
   }
 
   std::vector<Bucket> buckets;
   buckets.reserve(breakdown.size());
   for (auto key_bucket : breakdown)
     buckets.push_back(key_bucket.second);
 
   return buckets;
 }
 
-// Breaks down the bucket by |breakBy|. Returns only buckets that contribute
-// significantly to the total size. The long tail is omitted.
-std::vector<Bucket> BreakDownBy(const Bucket& bucket, BreakDownMode breakBy) {
-  std::vector<Bucket> buckets = GetSubbuckets(bucket, breakBy);
+// Breaks down the bucket by |break_by|. Returns only buckets that contribute
+// more than |minSizeBytes| to the total size. The long tail is omitted.

[Primiano Tucci (use gerrit), 2016/04/22 14:18:28]

s/minSizeBytes/min_size_bytes/

[Maria, 2016/04/25 18:37:23]

Done.

+std::vector<Bucket> BreakDownBy(const Bucket& bucket,
+                                BreakDownMode break_by,
+                                size_t min_size_bytes) {
+  std::vector<Bucket> buckets = GetSubbuckets(bucket, break_by);
 
   // Ensure that |buckets| is a max-heap (the data structure, not memory heap),
   // so its front contains the largest bucket. Buckets should be iterated
   // ordered by size, but sorting the vector is overkill because the long tail
   // of small buckets will be discarded. By using a max-heap, the optimal case
   // where all but the first bucket are discarded is O(n). The worst case where
   // no bucket is discarded is doing a heap sort, which is O(n log n).
   std::make_heap(buckets.begin(), buckets.end());
 
   // Keep including buckets until adding one would increase the number of
-  // bytes accounted for by less than 0.8% (1/125). This simple heuristic works
-  // quite well. The large buckets end up in [it, end()), [begin(), it) is the
-  // part that contains the max-heap of small buckets.
-  size_t accounted_for = 0;
+  // bytes accounted for by |min_size_bytes|. The large buckets end up in
+  // [it, end()), [begin(), it) is the part that contains the max-heap
+  // of small buckets.
   std::vector<Bucket>::iterator it;
   for (it = buckets.end(); it != buckets.begin(); --it) {
-    accounted_for += buckets.front().size;
-    if (buckets.front().size < (accounted_for / 125))
+    if (buckets.front().size < min_size_bytes)
       break;
 
     // Put the largest bucket in [begin, it) at |it - 1| and max-heapify
     // [begin, it - 1). This puts the next largest bucket at |buckets.front()|.
     std::pop_heap(buckets.begin(), it);
   }
 
   // At this point, |buckets| looks like this (numbers are bucket sizes):
   //
   // <-- max-heap of small buckets --->
@@ skipping 11 matching lines @@
 
 }  // namespace
 
 bool operator<(Entry lhs, Entry rhs) {
   // There is no need to compare |size|. If the backtrace and type name are
   // equal then the sizes must be equal as well.
   return std::tie(lhs.stack_frame_id, lhs.type_id) <
          std::tie(rhs.stack_frame_id, rhs.type_id);
 }
 
-HeapDumpWriter::HeapDumpWriter(StackFrameDeduplicator* stack_frame_deduplicator,
-                               TypeNameDeduplicator* type_name_deduplicator)
-    : stack_frame_deduplicator_(stack_frame_deduplicator),
-      type_name_deduplicator_(type_name_deduplicator) {}
+HeapDumpWriter::HeapDumpWriter(const MemoryDumpSessionState& session_state)

[Primiano Tucci (use gerrit), 2016/04/22 14:18:28]

Since this is an internal class, here I'd keep the explicit pointers being
passed as argument (sorry I realized only now that my comment was a bit
misplaced and should have referred to the public method ExtraceHeapDump below).
Instead the change below to ExtractHeapDump is fine

The rationale is: if you take the pointers here out of a const& reference, there
is the risk that the SessionState (that owns the deuplicators) can be destroyed
while this class is still alive.

In this specific case will work, because the ExtractHeapDump is the one that
makes sure that HeapDumpWriter does not outlive the SessionState being passed.
BUt in this piece of code is not explicit and you should not rely on it (instead
it's fine to do that in ExtractHeapDump, see my comment there).

[Maria, 2016/04/25 18:37:23]

Done.

+    : stack_frame_deduplicator_(session_state.stack_frame_deduplicator()),
+      type_name_deduplicator_(session_state.type_name_deduplicator()),
+      breakdown_threshold_bytes_(session_state.memory_dump_config().
+          heap_profiler_options.breakdown_threshold_bytes) {
+}
 
 HeapDumpWriter::~HeapDumpWriter() {}
 
 bool HeapDumpWriter::AddEntryForBucket(const Bucket& bucket) {
   // The contexts in the bucket are all different, but the [begin, cursor) range
   // is equal for all contexts in the bucket, and the type names are the same if
   // |is_broken_down_by_type_name| is set.
   DCHECK(!bucket.metrics_by_context.empty());
 
   const AllocationContext* context = bucket.metrics_by_context.front().first;
@@ skipping 12 matching lines @@
                       : -1;
 
   entry.size = bucket.size;
   entry.count = bucket.count;
 
   auto position_and_inserted = entries_.insert(entry);
   return position_and_inserted.second;
 }
 
 void HeapDumpWriter::BreakDown(const Bucket& bucket) {
-  auto by_backtrace = BreakDownBy(bucket, BreakDownMode::kByBacktrace);
-  auto by_type_name = BreakDownBy(bucket, BreakDownMode::kByTypeName);
+  auto by_backtrace = BreakDownBy(bucket,
+                                  BreakDownMode::kByBacktrace,
+                                  breakdown_threshold_bytes_);
+  auto by_type_name = BreakDownBy(bucket,
+                                  BreakDownMode::kByTypeName,
+                                  breakdown_threshold_bytes_);
 
   // Insert entries for the buckets. If a bucket was not present before, it has
   // not been broken down before, so recursively continue breaking down in that
   // case. There might be multiple routes to the same entry (first break down
   // by type name, then by backtrace, or first by backtrace and then by type),
   // so a set is used to avoid dumping and breaking down entries more than once.
 
   for (const Bucket& subbucket : by_backtrace)
     if (AddEntryForBucket(subbucket))
       BreakDown(subbucket);
@@ skipping 66 matching lines @@
   }
 
   traced_value->EndArray();  // "entries"
   return traced_value;
 }
 
 }  // namespace internal
 
 std::unique_ptr<TracedValue> ExportHeapDump(
     const hash_map<AllocationContext, AllocationMetrics>& metrics_by_context,
-    StackFrameDeduplicator* stack_frame_deduplicator,
-    TypeNameDeduplicator* type_name_deduplicator) {
-  internal::HeapDumpWriter writer(stack_frame_deduplicator,
-                                  type_name_deduplicator);
+    const MemoryDumpSessionState& session_state) {
+  internal::HeapDumpWriter writer(session_state);

[Primiano Tucci (use gerrit), 2016/04/22 14:18:28]

Here instead is perfectly reasonable.
This is the place where you should extract the * pointers and pass  them to the
HeapDumpWriter.
Everything is consistent here because the code guarantees that the
HeapDUmpWriter gets destroyed before you return, so it can't possibly outlive
the SessionState that you get passed.

[Maria, 2016/04/25 18:37:23]

Done.

   return Serialize(writer.Summarize(metrics_by_context));
 }
 
 }  // namespace trace_event
 }  // namespace base