Merge "Fix compaction bugs related to IdentityHashCode"
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index c63e2d7..169aa9c 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -105,18 +105,17 @@
jobject Dbg::TypeCache::Add(mirror::Class* t) {
ScopedObjectAccessUnchecked soa(Thread::Current());
- int32_t hash_code = t->IdentityHashCode();
+ JNIEnv* const env = soa.Env();
+ ScopedLocalRef<jobject> local_ref(soa.Env(), soa.AddLocalReference<jobject>(t));
+ const int32_t hash_code = soa.Decode<mirror::Class*>(local_ref.get())->IdentityHashCode();
auto range = objects_.equal_range(hash_code);
for (auto it = range.first; it != range.second; ++it) {
- if (soa.Decode<mirror::Class*>(it->second) == t) {
+ if (soa.Decode<mirror::Class*>(it->second) == soa.Decode<mirror::Class*>(local_ref.get())) {
// Found a matching weak global, return it.
return it->second;
}
}
- JNIEnv* env = soa.Env();
- const jobject local_ref = soa.AddLocalReference<jobject>(t);
- const jobject weak_global = env->NewWeakGlobalRef(local_ref);
- env->DeleteLocalRef(local_ref);
+ const jobject weak_global = env->NewWeakGlobalRef(local_ref.get());
objects_.insert(std::make_pair(hash_code, weak_global));
return weak_global;
}
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 42bfe21..90115c3 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -671,12 +671,14 @@
return false;
}
// Not found. Add it.
+ static_assert(!kMovingMethods, "Not safe if methods can move");
int32_t hash_code = method->IdentityHashCode();
deoptimized_methods_.insert(std::make_pair(hash_code, GcRoot<mirror::ArtMethod>(method)));
return true;
}
bool Instrumentation::FindDeoptimizedMethod(mirror::ArtMethod* method) {
+ static_assert(!kMovingMethods, "Not safe if methods can move");
int32_t hash_code = method->IdentityHashCode();
auto range = deoptimized_methods_.equal_range(hash_code);
for (auto it = range.first; it != range.second; ++it) {
@@ -700,6 +702,7 @@
}
bool Instrumentation::RemoveDeoptimizedMethod(mirror::ArtMethod* method) {
+ static_assert(!kMovingMethods, "Not safe if methods can move");
int32_t hash_code = method->IdentityHashCode();
auto range = deoptimized_methods_.equal_range(hash_code);
for (auto it = range.first; it != range.second; ++it) {
diff --git a/runtime/jobject_comparator.cc b/runtime/jobject_comparator.cc
index 77f93ff..1f424b3 100644
--- a/runtime/jobject_comparator.cc
+++ b/runtime/jobject_comparator.cc
@@ -25,33 +25,32 @@
bool JobjectComparator::operator()(jobject jobj1, jobject jobj2) const {
// Ensure null references and cleared jweaks appear at the end.
- if (jobj1 == NULL) {
+ if (jobj1 == nullptr) {
return true;
- } else if (jobj2 == NULL) {
+ } else if (jobj2 == nullptr) {
return false;
}
ScopedObjectAccess soa(Thread::Current());
- mirror::Object* obj1 = soa.Decode<mirror::Object*>(jobj1);
- mirror::Object* obj2 = soa.Decode<mirror::Object*>(jobj2);
- if (obj1 == NULL) {
+ StackHandleScope<2> hs(soa.Self());
+ Handle<mirror::Object> obj1(hs.NewHandle(soa.Decode<mirror::Object*>(jobj1)));
+ Handle<mirror::Object> obj2(hs.NewHandle(soa.Decode<mirror::Object*>(jobj2)));
+ if (obj1.Get() == nullptr) {
return true;
- } else if (obj2 == NULL) {
+ } else if (obj2.Get() == nullptr) {
return false;
}
// Sort by class...
if (obj1->GetClass() != obj2->GetClass()) {
return obj1->GetClass()->IdentityHashCode() < obj2->GetClass()->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 count1 = obj1->SizeOf();
+ const size_t count2 = obj2->SizeOf();
+ if (count1 != count2) {
+ return count1 < count2;
+ }
+ // ...and finally by identity hash code.
+ return obj1->IdentityHashCode() < obj2->IdentityHashCode();
}
} // namespace art
diff --git a/runtime/reference_table.cc b/runtime/reference_table.cc
index e454b20..357d454 100644
--- a/runtime/reference_table.cc
+++ b/runtime/reference_table.cc
@@ -71,33 +71,6 @@
return obj->AsArray()->GetLength();
}
-struct ObjectComparator {
- 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>();
- DCHECK(obj1 != nullptr);
- DCHECK(obj2 != nullptr);
- Runtime* runtime = Runtime::Current();
- DCHECK(!runtime->IsClearedJniWeakGlobal(obj1));
- DCHECK(!runtime->IsClearedJniWeakGlobal(obj2));
- // Sort by class...
- if (obj1->GetClass() != obj2->GetClass()) {
- return obj1->GetClass()->IdentityHashCode() < obj2->GetClass()->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();
- }
-};
-
// Log an object with some additional info.
//
// Pass in the number of elements in the array (or 0 if this is not an
@@ -143,6 +116,38 @@
}
void ReferenceTable::Dump(std::ostream& os, Table& entries) {
+ // Compare GC roots, first by class, then size, then address.
+ struct GcRootComparator {
+ 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());
+ // These GC roots are already forwarded in ReferenceTable::Dump. We sort by class since there
+ // are no suspend points which can happen during the sorting process. This works since
+ // we are guaranteed that the addresses of obj1, obj2, obj1->GetClass, obj2->GetClass wont
+ // change during the sorting process. The classes are forwarded by ref->GetClass().
+ mirror::Object* obj1 = root1.Read<kWithoutReadBarrier>();
+ mirror::Object* obj2 = root2.Read<kWithoutReadBarrier>();
+ DCHECK(obj1 != nullptr);
+ DCHECK(obj2 != nullptr);
+ Runtime* runtime = Runtime::Current();
+ DCHECK(!runtime->IsClearedJniWeakGlobal(obj1));
+ DCHECK(!runtime->IsClearedJniWeakGlobal(obj2));
+ // Sort by class...
+ if (obj1->GetClass() != obj2->GetClass()) {
+ return obj1->GetClass() < obj2->GetClass();
+ }
+ // ...then by size...
+ const size_t size1 = obj1->SizeOf();
+ const size_t size2 = obj2->SizeOf();
+ if (size1 != size2) {
+ return size1 < size2;
+ }
+ // ...and finally by address.
+ return obj1 < obj2;
+ }
+ };
+
if (entries.empty()) {
os << " (empty)\n";
return;
@@ -201,7 +206,7 @@
if (sorted_entries.empty()) {
return;
}
- std::sort(sorted_entries.begin(), sorted_entries.end(), ObjectComparator());
+ std::sort(sorted_entries.begin(), sorted_entries.end(), GcRootComparator());
// Dump a summary of the whole table.
os << " Summary:\n";