commit 739383c80684eeb41d380ca5d18e1e9a1fe9fd7f
author Lokesh Gidra <lokeshgidra@google.com> Fri Mar 08 14:28:43 2019 -0800
committer Treehugger Robot <treehugger-gerrit@google.com> Sat Mar 09 19:25:36 2019 +0000
tree 8341c7721df73df0b55b767cc3d9899ecf074cc1
parent 9da91d2ee78a32e4bfa5f402f23835652b87023e

Fix overflow in freed_bytes_histogram

Also ensures that histograms have appropriate Value type.

Test: art/test/testrunner/testrunner.py --host
Bug: b/127959698
Change-Id: Iefa5516170eef558c66fecac4b1a72b3623ff20f

runtime/gc/collector/garbage_collector.cc

diff --git a/runtime/gc/collector/garbage_collector.cc b/runtime/gc/collector/garbage_collector.cc
index e4ae10a..4aa58b6 100644
--- a/runtime/gc/collector/garbage_collector.cc
+++ b/runtime/gc/collector/garbage_collector.cc
@@ -67,12 +67,8 @@
     : heap_(heap),
       name_(name),
       pause_histogram_((name_ + " paused").c_str(), kPauseBucketSize, kPauseBucketCount),
-      rss_histogram_((name_ + " peak-rss").c_str(),
-                     /*initial_bucket_width=*/ 10,
-                     /*max_buckets=*/ 20),
-      freed_bytes_histogram_((name_ + " freed-bytes").c_str(),
-                             /*initial_bucket_width=*/ 10,
-                             /*max_buckets=*/ 20),
+      rss_histogram_((name_ + " peak-rss").c_str(), kMemBucketSize, kMemBucketCount),
+      freed_bytes_histogram_((name_ + " freed-bytes").c_str(), kMemBucketSize, kMemBucketCount),
       cumulative_timings_(name),
       pause_histogram_lock_("pause histogram lock", kDefaultMutexLevel, true),
       is_transaction_active_(false) {
@@ -165,10 +161,11 @@
   // Update cumulative statistics with how many bytes the GC iteration freed.
   total_freed_objects_ += current_iteration->GetFreedObjects() +
       current_iteration->GetFreedLargeObjects();
-  total_freed_bytes_ += current_iteration->GetFreedBytes() +
+  int64_t freed_bytes = current_iteration->GetFreedBytes() +
       current_iteration->GetFreedLargeObjectBytes();
-  freed_bytes_histogram_.AddValue((current_iteration->GetFreedBytes() +
-                                   current_iteration->GetFreedLargeObjectBytes()) / KB);
+  total_freed_bytes_ += freed_bytes;
+  // Rounding negative freed bytes to 0 as we are not interested in such corner cases.
+  freed_bytes_histogram_.AddValue(std::max<int64_t>(freed_bytes / KB, 0));
   uint64_t end_time = NanoTime();
   uint64_t thread_cpu_end_time = ThreadCpuNanoTime();
   total_thread_cpu_time_ns_ += thread_cpu_end_time - thread_cpu_start_time;

runtime/gc/collector/garbage_collector.h

diff --git a/runtime/gc/collector/garbage_collector.h b/runtime/gc/collector/garbage_collector.h
index a4f9467..7505658 100644
--- a/runtime/gc/collector/garbage_collector.h
+++ b/runtime/gc/collector/garbage_collector.h
@@ -147,6 +147,8 @@
 
   static constexpr size_t kPauseBucketSize = 500;
   static constexpr size_t kPauseBucketCount = 32;
+  static constexpr size_t kMemBucketSize = 10;
+  static constexpr size_t kMemBucketCount = 16;
 
   Heap* const heap_;
   std::string name_;