commit fdb7d5d882a5f18326746cd92e83473be48231eb
author Mathieu Chartier <mathieuc@google.com> Wed Feb 15 18:06:39 2017 -0800
committer Mathieu Chartier <mathieuc@google.com> Thu Feb 16 02:13:37 2017 +0000
tree 3755a653ddc638f5484ea628d6bac68f1f5cac46
parent 4caf20398382daf8e41649fb4840f8d2e945dc12

Avoid adding region space bitmap to heap bitmap

Adding the bitmap here really served no purpose and caused
VisitObjects to visit region space bitmap objects twice. This had
unwanted results like adding objects to hprof dumps twice.

Regression test is covered by DCHECK in hprof and test 130.

Test: N6P boot and test-art-host

Bug: 34967844
Change-Id: I53a6075d577430089bb42f84a3c80c1cc5302206

runtime/gc/heap.cc

diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 0a45fce..a78de37 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -987,7 +987,9 @@
     // Continuous spaces don't necessarily have bitmaps.
     accounting::ContinuousSpaceBitmap* live_bitmap = continuous_space->GetLiveBitmap();
     accounting::ContinuousSpaceBitmap* mark_bitmap = continuous_space->GetMarkBitmap();
-    if (live_bitmap != nullptr) {
+    // The region space bitmap is not added since VisitObjects visits the region space objects with
+    // special handling.
+    if (live_bitmap != nullptr && !space->IsRegionSpace()) {
       CHECK(mark_bitmap != nullptr);
       live_bitmap_->AddContinuousSpaceBitmap(live_bitmap);
       mark_bitmap_->AddContinuousSpaceBitmap(mark_bitmap);
@@ -1028,7 +1030,7 @@
     // Continuous spaces don't necessarily have bitmaps.
     accounting::ContinuousSpaceBitmap* live_bitmap = continuous_space->GetLiveBitmap();
     accounting::ContinuousSpaceBitmap* mark_bitmap = continuous_space->GetMarkBitmap();
-    if (live_bitmap != nullptr) {
+    if (live_bitmap != nullptr && !space->IsRegionSpace()) {
       DCHECK(mark_bitmap != nullptr);
       live_bitmap_->RemoveContinuousSpaceBitmap(live_bitmap);
       mark_bitmap_->RemoveContinuousSpaceBitmap(mark_bitmap);

runtime/hprof/hprof.cc

diff --git a/runtime/hprof/hprof.cc b/runtime/hprof/hprof.cc
index 133502e..f390645 100644
--- a/runtime/hprof/hprof.cc
+++ b/runtime/hprof/hprof.cc
@@ -463,6 +463,7 @@
     }
 
     bool okay;
+    visited_objects_.clear();
     if (direct_to_ddms_) {
       if (kDirectStream) {
         okay = DumpToDdmsDirect(overall_size, max_length, CHUNK_TYPE("HPDS"));
@@ -911,6 +912,9 @@
   // bits.
   std::unordered_set<uint64_t> simple_roots_;
 
+  // To make sure we don't dump the same object multiple times. b/34967844
+  std::unordered_set<mirror::Object*> visited_objects_;
+
   friend class GcRootVisitor;
   DISALLOW_COPY_AND_ASSIGN(Hprof);
 };
@@ -1093,6 +1097,7 @@
   if (obj->IsClass() && obj->AsClass()->IsRetired()) {
     return;
   }
+  DCHECK(visited_objects_.insert(obj).second) << "Already visited " << obj;
 
   ++total_objects_;