Tidy up and finish reference table dumping.
Change-Id: I9f0d214e27a75d373e3144b738f1e3da51bbc0ca
diff --git a/src/hprof/hprof.cc b/src/hprof/hprof.cc
index eacc73f..02f80c1 100644
--- a/src/hprof/hprof.cc
+++ b/src/hprof/hprof.cc
@@ -201,8 +201,7 @@
// only true when marking the root set or unreachable
// objects. Used to add rootset references to obj.
int Hprof::MarkRootObject(const Object *obj, jobject jniObj) {
- HprofRecord *rec = ¤t_record_;
- int err; // TODO: we may return this uninitialized
+ HprofRecord* rec = ¤t_record_;
HprofHeapTag heapTag = (HprofHeapTag)gc_scan_state_;
if (heapTag == 0) {
@@ -221,9 +220,7 @@
case HPROF_ROOT_STICKY_CLASS:
case HPROF_ROOT_MONITOR_USED:
case HPROF_ROOT_INTERNED_STRING:
- case HPROF_ROOT_FINALIZING:
case HPROF_ROOT_DEBUGGER:
- case HPROF_ROOT_REFERENCE_CLEANUP:
case HPROF_ROOT_VM_INTERNAL:
rec->AddU1(heapTag);
rec->AddId((HprofObjectId)obj);
@@ -268,13 +265,24 @@
rec->AddU4((uint32_t)-1); //xxx
break;
- default:
- err = 0;
+ case HPROF_CLASS_DUMP:
+ case HPROF_INSTANCE_DUMP:
+ case HPROF_OBJECT_ARRAY_DUMP:
+ case HPROF_PRIMITIVE_ARRAY_DUMP:
+ case HPROF_HEAP_DUMP_INFO:
+ case HPROF_PRIMITIVE_ARRAY_NODATA_DUMP:
+ // Ignored.
+ break;
+
+ case HPROF_ROOT_FINALIZING:
+ case HPROF_ROOT_REFERENCE_CLEANUP:
+ case HPROF_UNREACHABLE:
+ LOG(FATAL) << "obsolete tag " << static_cast<int>(heapTag);
break;
}
objects_in_segment_++;
- return err;
+ return 0;
}
int Hprof::StackTraceSerialNumber(const void* /*obj*/) {
diff --git a/src/indirect_reference_table.cc b/src/indirect_reference_table.cc
index 14d804b..69f171c 100644
--- a/src/indirect_reference_table.cc
+++ b/src/indirect_reference_table.cc
@@ -60,9 +60,7 @@
alloc_entries_ = max_entries_ = -1;
}
-/*
- * Make sure that the entry at "idx" is correctly paired with "iref".
- */
+// Make sure that the entry at "idx" is correctly paired with "iref".
bool IndirectReferenceTable::CheckEntry(const char* what, IndirectRef iref, int idx) const {
const Object* obj = table_[idx];
IndirectRef checkRef = ToIndirectRef(obj, idx);
@@ -89,12 +87,11 @@
DCHECK_GE(segment_state_.parts.numHoles, prevState.parts.numHoles);
if (topIndex == alloc_entries_) {
- /* reached end of allocated space; did we hit buffer max? */
+ // reached end of allocated space; did we hit buffer max?
if (topIndex == max_entries_) {
- LOG(ERROR) << "JNI ERROR (app bug): " << kind_ << " table overflow "
- << "(max=" << max_entries_ << ")";
- Dump();
- LOG(FATAL); // TODO: operator<< for IndirectReferenceTable
+ LOG(FATAL) << "JNI ERROR (app bug): " << kind_ << " table overflow "
+ << "(max=" << max_entries_ << ")\n"
+ << Dumpable<IndirectReferenceTable>(*this);
}
size_t newSize = alloc_entries_ * 2;
@@ -106,12 +103,11 @@
table_ = reinterpret_cast<const Object**>(realloc(table_, newSize * sizeof(const Object*)));
slot_data_ = reinterpret_cast<IndirectRefSlot*>(realloc(slot_data_, newSize * sizeof(IndirectRefSlot)));
if (table_ == NULL || slot_data_ == NULL) {
- LOG(ERROR) << "JNI ERROR (app bug): unable to expand "
+ LOG(FATAL) << "JNI ERROR (app bug): unable to expand "
<< kind_ << " table (from "
<< alloc_entries_ << " to " << newSize
- << ", max=" << max_entries_ << ")";
- Dump();
- LOG(FATAL); // TODO: operator<< for IndirectReferenceTable
+ << ", max=" << max_entries_ << ")\n"
+ << Dumpable<IndirectReferenceTable>(*this);
}
// Clear the newly-allocated slot_data_ elements.
@@ -120,16 +116,14 @@
alloc_entries_ = newSize;
}
- /*
- * We know there's enough room in the table. Now we just need to find
- * the right spot. If there's a hole, find it and fill it; otherwise,
- * add to the end of the list.
- */
+ // We know there's enough room in the table. Now we just need to find
+ // the right spot. If there's a hole, find it and fill it; otherwise,
+ // add to the end of the list.
IndirectRef result;
int numHoles = segment_state_.parts.numHoles - prevState.parts.numHoles;
if (numHoles > 0) {
DCHECK_GT(topIndex, 1U);
- /* find the first hole; likely to be near the end of the list */
+ // Find the first hole; likely to be near the end of the list.
const Object** pScan = &table_[topIndex - 1];
DCHECK(*pScan != NULL);
while (*--pScan != NULL) {
@@ -140,7 +134,7 @@
*pScan = obj;
segment_state_.parts.numHoles--;
} else {
- /* add to the end */
+ // Add to the end.
UpdateSlotAdd(obj, topIndex);
result = ToIndirectRef(obj, topIndex);
table_[topIndex++] = obj;
@@ -157,16 +151,13 @@
void IndirectReferenceTable::AssertEmpty() {
if (begin() != end()) {
- Dump();
- LOG(FATAL) << "Internal Error: non-empty local reference table";
+ LOG(FATAL) << "Internal Error: non-empty local reference table\n"
+ << Dumpable<IndirectReferenceTable>(*this);
}
}
-/*
- * Verify that the indirect table lookup is valid.
- *
- * Returns "false" if something looks bad.
- */
+// Verifies that the indirect table lookup is valid.
+// Returns "false" if something looks bad.
bool IndirectReferenceTable::GetChecked(IndirectRef iref) const {
if (iref == NULL) {
LOG(WARNING) << "Attempt to look up NULL " << kind_;
@@ -181,7 +172,6 @@
int topIndex = segment_state_.parts.topIndex;
int idx = ExtractIndex(iref);
if (idx >= topIndex) {
- /* bad -- stale reference? */
LOG(ERROR) << "JNI ERROR (app bug): accessed stale " << kind_ << " "
<< iref << " (index " << idx << " in a table of size " << topIndex << ")";
AbortMaybe();
@@ -214,19 +204,14 @@
return Find(direct_pointer, 0, segment_state_.parts.topIndex, table_) != -1;
}
-/*
- * Remove "obj" from "pRef". We extract the table offset bits from "iref"
- * and zap the corresponding entry, leaving a hole if it's not at the top.
- *
- * If the entry is not between the current top index and the bottom index
- * specified by the cookie, we don't remove anything. This is the behavior
- * required by JNI's DeleteLocalRef function.
- *
- * Note this is NOT called when a local frame is popped. This is only used
- * for explicit single removals.
- *
- * Returns "false" if nothing was removed.
- */
+// Removes an object. We extract the table offset bits from "iref"
+// and zap the corresponding entry, leaving a hole if it's not at the top.
+// If the entry is not between the current top index and the bottom index
+// specified by the cookie, we don't remove anything. This is the behavior
+// required by JNI's DeleteLocalRef function.
+// This method is not called when a local frame is popped; this is only used
+// for explicit single removals.
+// Returns "false" if nothing was removed.
bool IndirectReferenceTable::Remove(uint32_t cookie, IndirectRef iref) {
IRTSegmentState prevState;
prevState.all = cookie;
@@ -299,11 +284,9 @@
}
}
} else {
- /*
- * Not the top-most entry. This creates a hole. We NULL out the
- * entry to prevent somebody from deleting it twice and screwing up
- * the hole count.
- */
+ // Not the top-most entry. This creates a hole. We NULL out the
+ // entry to prevent somebody from deleting it twice and screwing up
+ // the hole count.
if (table_[idx] == NULL) {
LOG(INFO) << "--- WEIRD: removing null entry " << idx;
return false;
@@ -329,10 +312,10 @@
}
}
-void IndirectReferenceTable::Dump() const {
- LOG(WARNING) << kind_ << " table dump:";
+void IndirectReferenceTable::Dump(std::ostream& os) const {
+ os << kind_ << " table dump:\n";
std::vector<const Object*> entries(table_, table_ + Capacity());
- ReferenceTable::Dump(entries);
+ ReferenceTable::Dump(os, entries);
}
} // namespace art
diff --git a/src/indirect_reference_table.h b/src/indirect_reference_table.h
index b0a0d64..710e43f 100644
--- a/src/indirect_reference_table.h
+++ b/src/indirect_reference_table.h
@@ -287,7 +287,7 @@
void AssertEmpty();
- void Dump() const;
+ void Dump(std::ostream& os) const;
/*
* Return the #of entries in the entire table. This includes holes, and
diff --git a/src/indirect_reference_table_test.cc b/src/indirect_reference_table_test.cc
index cea4afd..387a2cd 100644
--- a/src/indirect_reference_table_test.cc
+++ b/src/indirect_reference_table_test.cc
@@ -52,7 +52,7 @@
IndirectRef iref2 = irt.Add(cookie, obj2);
EXPECT_TRUE(iref2 != NULL);
- irt.Dump();
+ irt.Dump(LOG(DEBUG));
EXPECT_EQ(obj0, irt.Get(iref0));
EXPECT_EQ(obj1, irt.Get(iref1));
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index 846a717..a20bcc9 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -90,21 +90,15 @@
uint32_t cookie = env->local_ref_cookie;
IndirectRef ref = locals.Add(cookie, obj);
- if (ref == NULL) {
- // TODO: just change Add's DCHECK to CHECK and lose this?
- locals.Dump();
- LOG(FATAL) << "Failed adding to JNI local reference table "
- << "(has " << locals.Capacity() << " entries)";
- }
#if 0 // TODO: fix this to understand PushLocalFrame, so we can turn it on.
if (env->check_jni) {
size_t entry_count = locals.Capacity();
if (entry_count > 16) {
LOG(WARNING) << "Warning: more than 16 JNI local references: "
- << entry_count << " (most recent was a " << PrettyTypeOf(obj) << ")";
- locals.Dump();
- // TODO: LOG(FATAL) instead.
+ << entry_count << " (most recent was a " << PrettyTypeOf(obj) << ")\n"
+ << Dumpable<IndirectReferenceTable>(locals);
+ // TODO: LOG(FATAL) in a later release?
}
}
#endif
@@ -2654,9 +2648,9 @@
functions = enabled ? GetCheckJniNativeInterface() : &gJniNativeInterface;
}
-void JNIEnvExt::DumpReferenceTables() {
- locals.Dump();
- monitors.Dump();
+void JNIEnvExt::DumpReferenceTables(std::ostream& os) {
+ locals.Dump(os);
+ monitors.Dump(os);
}
void JNIEnvExt::PushFrame(int /*capacity*/) {
@@ -2826,18 +2820,18 @@
}
}
-void JavaVMExt::DumpReferenceTables() {
+void JavaVMExt::DumpReferenceTables(std::ostream& os) {
{
MutexLock mu(globals_lock);
- globals.Dump();
+ globals.Dump(os);
}
{
MutexLock mu(weak_globals_lock);
- weak_globals.Dump();
+ weak_globals.Dump(os);
}
{
MutexLock mu(pins_lock);
- pin_table.Dump();
+ pin_table.Dump(os);
}
}
diff --git a/src/jni_internal.h b/src/jni_internal.h
index e12a7cd..0d97aa4 100644
--- a/src/jni_internal.h
+++ b/src/jni_internal.h
@@ -100,7 +100,7 @@
void DumpForSigQuit(std::ostream& os);
- void DumpReferenceTables();
+ void DumpReferenceTables(std::ostream& os);
void SetCheckJniEnabled(bool enabled);
@@ -145,7 +145,7 @@
JNIEnvExt(Thread* self, JavaVMExt* vm);
~JNIEnvExt();
- void DumpReferenceTables();
+ void DumpReferenceTables(std::ostream& os);
void SetCheckJniEnabled(bool enabled);
diff --git a/src/native/dalvik_system_VMDebug.cc b/src/native/dalvik_system_VMDebug.cc
index f125272..49ef593 100644
--- a/src/native/dalvik_system_VMDebug.cc
+++ b/src/native/dalvik_system_VMDebug.cc
@@ -201,8 +201,8 @@
LOG(INFO) << "--- reference table dump ---";
JNIEnvExt* e = reinterpret_cast<JNIEnvExt*>(env);
- e->DumpReferenceTables();
- e->vm->DumpReferenceTables();
+ e->DumpReferenceTables(LOG(INFO));
+ e->vm->DumpReferenceTables(LOG(INFO));
LOG(INFO) << "---";
}
diff --git a/src/reference_table.cc b/src/reference_table.cc
index dd32765..ee1760b 100644
--- a/src/reference_table.cc
+++ b/src/reference_table.cc
@@ -52,7 +52,7 @@
// If "obj" is an array, return the number of elements in the array.
// Otherwise, return zero.
-size_t GetElementCount(const Object* obj) {
+static size_t GetElementCount(const Object* obj) {
if (obj == NULL || obj == kClearedJniWeakGlobal || !obj->IsArrayInstance()) {
return 0;
}
@@ -97,13 +97,13 @@
// Pass in the number of elements in the array (or 0 if this is not an
// array object), and the number of additional objects that are identical
// or equivalent to the original.
-void LogSummaryLine(const Object* obj, size_t elems, int identical, int equiv) {
+static void DumpSummaryLine(std::ostream& os, const Object* obj, size_t element_count, int identical, int equiv) {
if (obj == NULL) {
- LOG(WARNING) << " NULL reference (count=" << equiv << ")";
+ os << " NULL reference (count=" << equiv << ")\n";
return;
}
if (obj == kClearedJniWeakGlobal) {
- LOG(WARNING) << " cleared jweak (count=" << equiv << ")";
+ os << " cleared jweak (count=" << equiv << ")\n";
return;
}
@@ -113,8 +113,8 @@
// Class' type parameter here would be misleading.
className = "java.lang.Class";
}
- if (elems != 0) {
- StringAppendF(&className, " (%zd elements)", elems);
+ if (element_count != 0) {
+ StringAppendF(&className, " (%zd elements)", element_count);
}
size_t total = identical + equiv + 1;
@@ -122,21 +122,21 @@
if (identical + equiv != 0) {
StringAppendF(&msg, " (%d unique instances)", equiv + 1);
}
- LOG(WARNING) << " " << msg;
+ os << " " << msg << "\n";
}
size_t ReferenceTable::Size() const {
return entries_.size();
}
-void ReferenceTable::Dump() const {
- LOG(WARNING) << name_ << " reference table dump:";
- Dump(entries_);
+void ReferenceTable::Dump(std::ostream& os) const {
+ os << name_ << " reference table dump:\n";
+ Dump(os, entries_);
}
-void ReferenceTable::Dump(const Table& entries) {
+void ReferenceTable::Dump(std::ostream& os, const Table& entries) {
if (entries.empty()) {
- LOG(WARNING) << " (empty)";
+ os << " (empty)\n";
return;
}
@@ -147,50 +147,39 @@
if (first < 0) {
first = 0;
}
- LOG(WARNING) << " Last " << (count - first) << " entries (of " << count << "):";
+ os << " Last " << (count - first) << " entries (of " << count << "):\n";
for (int idx = count - 1; idx >= first; --idx) {
const Object* ref = entries[idx];
if (ref == NULL) {
continue;
}
if (ref == kClearedJniWeakGlobal) {
- LOG(WARNING) << StringPrintf(" %5d: cleared jweak", idx);
+ os << StringPrintf(" %5d: cleared jweak\n", idx);
continue;
}
if (ref->GetClass() == NULL) {
// should only be possible right after a plain dvmMalloc().
size_t size = ref->SizeOf();
- LOG(WARNING) << StringPrintf(" %5d: %p (raw) (%zd bytes)", idx, ref, size);
+ os << StringPrintf(" %5d: %p (raw) (%zd bytes)\n", idx, ref, size);
continue;
}
std::string className(PrettyTypeOf(ref));
std::string extras;
- size_t elems = GetElementCount(ref);
- if (elems != 0) {
- StringAppendF(&extras, " (%zd elements)", elems);
- }
-#if 0
- // TODO: support dumping string data.
- else if (ref->GetClass() == gDvm.classJavaLangString) {
- const StringObject* str = reinterpret_cast<const StringObject*>(ref);
- extras += " \"";
- size_t count = 0;
- char* s = dvmCreateCstrFromString(str);
- char* p = s;
- for (; *p && count < 16; ++p, ++count) {
- extras += *p;
- }
- if (*p == 0) {
- extras += "\"";
+ size_t element_count = GetElementCount(ref);
+ if (element_count != 0) {
+ StringAppendF(&extras, " (%zd elements)", element_count);
+ } else if (ref->GetClass()->IsStringClass()) {
+ String* s = const_cast<Object*>(ref)->AsString();
+ std::string utf8(s->ToModifiedUtf8());
+ if (s->GetLength() <= 16) {
+ StringAppendF(&extras, " \"%s\"", utf8.c_str());
} else {
- StringAppendF(&extras, "... (%d chars)", str->length());
+ StringAppendF(&extras, " \"%.16s... (%d chars)", utf8.c_str(), s->GetLength());
}
- free(s);
}
-#endif
- LOG(WARNING) << StringPrintf(" %5d: ", idx) << ref << " " << className << extras;
+ os << StringPrintf(" %5d: ", idx) << ref << " " << className << extras << "\n";
}
// Make a copy of the table and sort it.
@@ -209,27 +198,27 @@
}
// Dump a summary of the whole table.
- LOG(WARNING) << " Summary:";
+ os << " Summary:\n";
size_t equiv = 0;
size_t identical = 0;
for (size_t idx = 1; idx < count; idx++) {
const Object* prev = sorted_entries[idx-1];
const Object* current = sorted_entries[idx];
- size_t elems = GetElementCount(prev);
+ size_t element_count = GetElementCount(prev);
if (current == prev) {
// Same reference, added more than once.
identical++;
- } else if (current->GetClass() == prev->GetClass() && GetElementCount(current) == elems) {
+ } else if (current->GetClass() == prev->GetClass() && GetElementCount(current) == element_count) {
// Same class / element count, different object.
equiv++;
} else {
// Different class.
- LogSummaryLine(prev, elems, identical, equiv);
+ DumpSummaryLine(os, prev, element_count, identical, equiv);
equiv = identical = 0;
}
}
// Handle the last entry.
- LogSummaryLine(sorted_entries.back(), GetElementCount(sorted_entries.back()), identical, equiv);
+ DumpSummaryLine(os, sorted_entries.back(), GetElementCount(sorted_entries.back()), identical, equiv);
}
void ReferenceTable::VisitRoots(Heap::RootVisitor* visitor, void* arg) {
diff --git a/src/reference_table.h b/src/reference_table.h
index 9142b7c..28af887 100644
--- a/src/reference_table.h
+++ b/src/reference_table.h
@@ -43,13 +43,13 @@
size_t Size() const;
- void Dump() const;
+ void Dump(std::ostream& os) const;
void VisitRoots(Heap::RootVisitor* visitor, void* arg);
private:
typedef std::vector<const Object*> Table;
- static void Dump(const Table& entries);
+ static void Dump(std::ostream& os, const Table& entries);
friend class IndirectReferenceTable; // For Dump.
std::string name_;
diff --git a/src/reference_table_test.cc b/src/reference_table_test.cc
index 4c1e67f..c7c1cc6 100644
--- a/src/reference_table_test.cc
+++ b/src/reference_table_test.cc
@@ -27,10 +27,10 @@
Object* o1 = String::AllocFromModifiedUtf8("hello");
Object* o2 = ShortArray::Alloc(0);
- // TODO: rewrite Dump to take a std::ostream& so we can test it better.
-
ReferenceTable rt("test", 0, 4);
- rt.Dump();
+ std::ostringstream oss1;
+ rt.Dump(oss1);
+ EXPECT_TRUE(oss1.str().find("(empty)") != std::string::npos) << oss1.str();
EXPECT_EQ(0U, rt.Size());
rt.Remove(NULL);
EXPECT_EQ(0U, rt.Size());
@@ -40,8 +40,16 @@
EXPECT_EQ(1U, rt.Size());
rt.Add(o2);
EXPECT_EQ(2U, rt.Size());
- rt.Dump();
+ rt.Add(o2);
+ EXPECT_EQ(3U, rt.Size());
+ std::ostringstream oss2;
+ rt.Dump(oss2);
+ EXPECT_TRUE(oss2.str().find("Last 3 entries (of 3):") != std::string::npos) << oss2.str();
+ EXPECT_TRUE(oss2.str().find("1 of java.lang.String") != std::string::npos) << oss2.str();
+ EXPECT_TRUE(oss2.str().find("2 of short[] (1 unique instances)") != std::string::npos) << oss2.str();
rt.Remove(o1);
+ EXPECT_EQ(2U, rt.Size());
+ rt.Remove(o2);
EXPECT_EQ(1U, rt.Size());
rt.Remove(o2);
EXPECT_EQ(0U, rt.Size());
diff --git a/src/timing_logger.h b/src/timing_logger.h
index 594ae98..d713edf 100644
--- a/src/timing_logger.h
+++ b/src/timing_logger.h
@@ -38,15 +38,19 @@
labels_.push_back(label);
}
- void Dump() {
- LOG(INFO) << name_ << ": begin";
- for (size_t i = 1; i < times_.size(); ++i) {
- LOG(INFO) << name_ << StringPrintf(": %8lld ms, ", NsToMs(times_[i] - times_[i-1])) << labels_[i];
- }
- LOG(INFO) << name_ << ": end, " << NsToMs(GetTotalNs()) << " ms";
+ void Dump() const {
+ Dump(LOG(INFO));
}
- uint64_t GetTotalNs() {
+ void Dump(std::ostream& os) const {
+ os << name_ << ": begin";
+ for (size_t i = 1; i < times_.size(); ++i) {
+ os << name_ << StringPrintf(": %8lld ms, ", NsToMs(times_[i] - times_[i-1])) << labels_[i];
+ }
+ os << name_ << ": end, " << NsToMs(GetTotalNs()) << " ms";
+ }
+
+ uint64_t GetTotalNs() const {
return times_.back() - times_.front();
}