Clean up ReferenceTable::Dump

Make sure that we never have nulls and cleared weak globals in the
sorted table. Cleaned up comparator.

Bug: 18597401
Change-Id: I5f437dfa29c813e17cdde411175abc927283716d

1 files changed, 41 insertions, 61 deletions

diff --git a/runtime/reference_table.cc b/runtime/reference_table.cc
index 01c507086..17eb7edac 100644
--- a/runtime/reference_table.cc
+++ b/runtime/reference_table.cc
@@ -72,39 +72,24 @@ static size_t GetElementCount(mirror::Object* obj) SHARED_LOCKS_REQUIRED(Locks::
 }
 
 struct ObjectComparator {
-  bool operator()(GcRoot<mirror::Object> root1, GcRoot<mirror::Object> root2)
+  bool operator()(GcRoot<mirror::Object> root1, GcRoot<mirror::Object> root2) const
     // TODO: enable analysis when analysis can work with the STL.
       NO_THREAD_SAFETY_ANALYSIS {
     Locks::mutator_lock_->AssertSharedHeld(Thread::Current());
     mirror::Object* obj1 = root1.Read<kWithoutReadBarrier>();
     mirror::Object* obj2 = root2.Read<kWithoutReadBarrier>();
-    // Ensure null references and cleared jweaks appear at the end.
-    if (obj1 == NULL) {
-      return true;
-    } else if (obj2 == NULL) {
-      return false;
-    }
-    Runtime* runtime = Runtime::Current();
-    if (runtime->IsClearedJniWeakGlobal(obj1)) {
-      return true;
-    } else if (runtime->IsClearedJniWeakGlobal(obj2)) {
-      return false;
-    }
-
     // Sort by class...
     if (obj1->GetClass() != obj2->GetClass()) {
       return obj1->GetClass()->IdentityHashCode() < obj2->IdentityHashCode();
-    } else {
-      // ...then by size...
-      size_t count1 = obj1->SizeOf();
-      size_t count2 = obj2->SizeOf();
-      if (count1 != count2) {
-        return count1 < count2;
-      } else {
-        // ...and finally by identity hash code.
-        return obj1->IdentityHashCode() < obj2->IdentityHashCode();
-      }
     }
+    // ...then by size...
+    const size_t size1 = obj1->SizeOf();
+    const size_t size2 = obj2->SizeOf();
+    if (size1 != size2) {
+      return size1 < size2;
+    }
+    // ...and finally by identity hash code.
+    return obj1->IdentityHashCode() < obj2->IdentityHashCode();
   }
 };
 
@@ -166,16 +151,17 @@ void ReferenceTable::Dump(std::ostream& os, Table& entries) {
     first = 0;
   }
   os << "  Last " << (count - first) << " entries (of " << count << "):\n";
+  Runtime* runtime = Runtime::Current();
   for (int idx = count - 1; idx >= first; --idx) {
-    mirror::Object* ref = entries[idx].Read();
-    if (ref == NULL) {
+    mirror::Object* ref = entries[idx].Read<kWithoutReadBarrier>();
+    if (ref == nullptr) {
       continue;
     }
-    if (Runtime::Current()->IsClearedJniWeakGlobal(ref)) {
+    if (runtime->IsClearedJniWeakGlobal(ref)) {
       os << StringPrintf("    %5d: cleared jweak\n", idx);
       continue;
     }
-    if (ref->GetClass() == NULL) {
+    if (ref->GetClass() == nullptr) {
       // should only be possible right after a plain dvmMalloc().
       size_t size = ref->SizeOf();
       os << StringPrintf("    %5d: %p (raw) (%zd bytes)\n", idx, ref, size);
@@ -189,7 +175,7 @@ void ReferenceTable::Dump(std::ostream& os, Table& entries) {
     if (element_count != 0) {
       StringAppendF(&extras, " (%zd elements)", element_count);
     } else if (ref->GetClass()->IsStringClass()) {
-      mirror::String* s = const_cast<mirror::Object*>(ref)->AsString();
+      mirror::String* s = ref->AsString();
       std::string utf8(s->ToModifiedUtf8());
       if (s->GetLength() <= 16) {
         StringAppendF(&extras, " \"%s\"", utf8.c_str());
@@ -200,51 +186,45 @@ void ReferenceTable::Dump(std::ostream& os, Table& entries) {
     os << StringPrintf("    %5d: ", idx) << ref << " " << className << extras << "\n";
   }
 
-  // Make a copy of the table and sort it.
+  // Make a copy of the table and sort it, only adding non null and not cleared elements.
   Table sorted_entries;
-  for (size_t i = 0; i < entries.size(); ++i) {
-    mirror::Object* entry = entries[i].Read();
-    sorted_entries.push_back(GcRoot<mirror::Object>(entry));
-  }
-  std::sort(sorted_entries.begin(), sorted_entries.end(), ObjectComparator());
-
-  // Remove any uninteresting stuff from the list. The sort moved them all to the end.
-  while (!sorted_entries.empty() && sorted_entries.back().IsNull()) {
-    sorted_entries.pop_back();
-  }
-  while (!sorted_entries.empty() &&
-         Runtime::Current()->IsClearedJniWeakGlobal(
-             sorted_entries.back().Read<kWithoutReadBarrier>())) {
-    sorted_entries.pop_back();
+  for (GcRoot<mirror::Object>& root : entries) {
+    if (!root.IsNull() && !runtime->IsClearedJniWeakGlobal(root.Read<kWithoutReadBarrier>())) {
+      sorted_entries.push_back(root);
+    }
   }
   if (sorted_entries.empty()) {
     return;
   }
+  std::sort(sorted_entries.begin(), sorted_entries.end(), ObjectComparator());
 
   // Dump a summary of the whole table.
   os << "  Summary:\n";
   size_t equiv = 0;
   size_t identical = 0;
-  for (size_t idx = 1; idx < count; idx++) {
-    mirror::Object* prev = sorted_entries[idx-1].Read<kWithoutReadBarrier>();
-    mirror::Object* current = sorted_entries[idx].Read<kWithoutReadBarrier>();
-    size_t element_count = GetElementCount(prev);
-    if (current == prev) {
-      // Same reference, added more than once.
-      identical++;
-    } else if (current->GetClass() == prev->GetClass() && GetElementCount(current) == element_count) {
-      // Same class / element count, different object.
-      equiv++;
-    } else {
-      // Different class.
-      DumpSummaryLine(os, prev, element_count, identical, equiv);
-      equiv = identical = 0;
+  mirror::Object* prev = nullptr;
+  for (GcRoot<mirror::Object>& root : sorted_entries) {
+    mirror::Object* current = root.Read<kWithoutReadBarrier>();
+    if (prev != nullptr) {
+      const size_t element_count = GetElementCount(prev);
+      if (current == prev) {
+        // Same reference, added more than once.
+        ++identical;
+      } else if (current->GetClass() == prev->GetClass() &&
+          GetElementCount(current) == element_count) {
+        // Same class / element count, different object.
+        ++equiv;
+      } else {
+        // Different class.
+        DumpSummaryLine(os, prev, element_count, identical, equiv);
+        equiv = 0;
+        identical = 0;
+      }
     }
+    prev = current;
   }
   // Handle the last entry.
-  DumpSummaryLine(os, sorted_entries.back().Read<kWithoutReadBarrier>(),
-                  GetElementCount(sorted_entries.back().Read<kWithoutReadBarrier>()),
-                  identical, equiv);
+  DumpSummaryLine(os, prev, GetElementCount(prev), identical, equiv);
 }
 
 void ReferenceTable::VisitRoots(RootCallback* visitor, void* arg, uint32_t tid,