Merge "Revert "Revert "Avoid adding region space bitmap to heap bitmap"""
diff --git a/runtime/gc/collector_type.h b/runtime/gc/collector_type.h
index 7014357..eef4fba 100644
--- a/runtime/gc/collector_type.h
+++ b/runtime/gc/collector_type.h
@@ -55,6 +55,8 @@
kCollectorTypeClassLinker,
// JIT Code cache fake collector.
kCollectorTypeJitCodeCache,
+ // Hprof fake collector.
+ kCollectorTypeHprof,
// Fake collector for installing/removing a system-weak holder.
kCollectorTypeAddRemoveSystemWeakHolder,
};
diff --git a/runtime/gc/gc_cause.cc b/runtime/gc/gc_cause.cc
index 7ff845d..9e34346 100644
--- a/runtime/gc/gc_cause.cc
+++ b/runtime/gc/gc_cause.cc
@@ -39,6 +39,7 @@
case kGcCauseClassLinker: return "ClassLinker";
case kGcCauseJitCodeCache: return "JitCodeCache";
case kGcCauseAddRemoveSystemWeakHolder: return "SystemWeakHolder";
+ case kGcCauseHprof: return "Hprof";
}
LOG(FATAL) << "Unreachable";
UNREACHABLE();
diff --git a/runtime/gc/gc_cause.h b/runtime/gc/gc_cause.h
index f54f0e4..9b285b1 100644
--- a/runtime/gc/gc_cause.h
+++ b/runtime/gc/gc_cause.h
@@ -53,6 +53,8 @@
kGcCauseJitCodeCache,
// Not a real GC cause, used to add or remove system-weak holders.
kGcCauseAddRemoveSystemWeakHolder,
+ // Not a real GC cause, used to hprof running in the middle of GC.
+ kGcCauseHprof,
};
const char* PrettyCause(GcCause cause);
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);
diff --git a/runtime/hprof/hprof.cc b/runtime/hprof/hprof.cc
index 133502e..e59c4bb 100644
--- a/runtime/hprof/hprof.cc
+++ b/runtime/hprof/hprof.cc
@@ -50,6 +50,7 @@
#include "gc_root.h"
#include "gc/accounting/heap_bitmap.h"
#include "gc/allocation_record.h"
+#include "gc/scoped_gc_critical_section.h"
#include "gc/heap.h"
#include "gc/space/space.h"
#include "globals.h"
@@ -463,6 +464,7 @@
}
bool okay;
+ visited_objects_.clear();
if (direct_to_ddms_) {
if (kDirectStream) {
okay = DumpToDdmsDirect(overall_size, max_length, CHUNK_TYPE("HPDS"));
@@ -911,6 +913,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 +1098,7 @@
if (obj->IsClass() && obj->AsClass()->IsRetired()) {
return;
}
+ DCHECK(visited_objects_.insert(obj).second) << "Already visited " << obj;
++total_objects_;
@@ -1444,22 +1450,15 @@
// Otherwise, "filename" is used to create an output file.
void DumpHeap(const char* filename, int fd, bool direct_to_ddms) {
CHECK(filename != nullptr);
-
Thread* self = Thread::Current();
- gc::Heap* heap = Runtime::Current()->GetHeap();
- if (heap->IsGcConcurrentAndMoving()) {
- // Need to take a heap dump while GC isn't running. See the
- // comment in Heap::VisitObjects().
- heap->IncrementDisableMovingGC(self);
- }
- {
- ScopedSuspendAll ssa(__FUNCTION__, true /* long suspend */);
- Hprof hprof(filename, fd, direct_to_ddms);
- hprof.Dump();
- }
- if (heap->IsGcConcurrentAndMoving()) {
- heap->DecrementDisableMovingGC(self);
- }
+ // Need to take a heap dump while GC isn't running. See the comment in Heap::VisitObjects().
+ // Also we need the critical section to avoid visiting the same object twice. See b/34967844
+ gc::ScopedGCCriticalSection gcs(self,
+ gc::kGcCauseHprof,
+ gc::kCollectorTypeHprof);
+ ScopedSuspendAll ssa(__FUNCTION__, true /* long suspend */);
+ Hprof hprof(filename, fd, direct_to_ddms);
+ hprof.Dump();
}
} // namespace hprof