Merge changes I11859ee8,Id17d1c87
* changes:
ObjPtr-ify the jvmti tagging system.
Fix 905 flake on low mem.
diff --git a/openjdkjvmti/jvmti_weak_table-inl.h b/openjdkjvmti/jvmti_weak_table-inl.h
index d9b8a84..5b28e45 100644
--- a/openjdkjvmti/jvmti_weak_table-inl.h
+++ b/openjdkjvmti/jvmti_weak_table-inl.h
@@ -77,7 +77,7 @@
}
template <typename T>
-bool JvmtiWeakTable<T>::GetTagSlowPath(art::Thread* self, art::mirror::Object* obj, T* result) {
+bool JvmtiWeakTable<T>::GetTagSlowPath(art::Thread* self, art::ObjPtr<art::mirror::Object> obj, T* result) {
// Under concurrent GC, there is a window between moving objects and sweeping of system
// weaks in which mutators are active. We may receive a to-space object pointer in obj,
// but still have from-space pointers in the table. Explicitly update the table once.
@@ -87,7 +87,7 @@
}
template <typename T>
-bool JvmtiWeakTable<T>::Remove(art::mirror::Object* obj, /* out */ T* tag) {
+bool JvmtiWeakTable<T>::Remove(art::ObjPtr<art::mirror::Object> obj, /* out */ T* tag) {
art::Thread* self = art::Thread::Current();
art::MutexLock mu(self, allow_disallow_lock_);
Wait(self);
@@ -95,7 +95,7 @@
return RemoveLocked(self, obj, tag);
}
template <typename T>
-bool JvmtiWeakTable<T>::RemoveLocked(art::mirror::Object* obj, T* tag) {
+bool JvmtiWeakTable<T>::RemoveLocked(art::ObjPtr<art::mirror::Object> obj, T* tag) {
art::Thread* self = art::Thread::Current();
allow_disallow_lock_.AssertHeld(self);
Wait(self);
@@ -104,7 +104,7 @@
}
template <typename T>
-bool JvmtiWeakTable<T>::RemoveLocked(art::Thread* self, art::mirror::Object* obj, T* tag) {
+bool JvmtiWeakTable<T>::RemoveLocked(art::Thread* self, art::ObjPtr<art::mirror::Object> obj, T* tag) {
auto it = tagged_objects_.find(art::GcRoot<art::mirror::Object>(obj));
if (it != tagged_objects_.end()) {
if (tag != nullptr) {
@@ -132,7 +132,7 @@
}
template <typename T>
-bool JvmtiWeakTable<T>::Set(art::mirror::Object* obj, T new_tag) {
+bool JvmtiWeakTable<T>::Set(art::ObjPtr<art::mirror::Object> obj, T new_tag) {
art::Thread* self = art::Thread::Current();
art::MutexLock mu(self, allow_disallow_lock_);
Wait(self);
@@ -140,7 +140,7 @@
return SetLocked(self, obj, new_tag);
}
template <typename T>
-bool JvmtiWeakTable<T>::SetLocked(art::mirror::Object* obj, T new_tag) {
+bool JvmtiWeakTable<T>::SetLocked(art::ObjPtr<art::mirror::Object> obj, T new_tag) {
art::Thread* self = art::Thread::Current();
allow_disallow_lock_.AssertHeld(self);
Wait(self);
@@ -149,7 +149,7 @@
}
template <typename T>
-bool JvmtiWeakTable<T>::SetLocked(art::Thread* self, art::mirror::Object* obj, T new_tag) {
+bool JvmtiWeakTable<T>::SetLocked(art::Thread* self, art::ObjPtr<art::mirror::Object> obj, T new_tag) {
auto it = tagged_objects_.find(art::GcRoot<art::mirror::Object>(obj));
if (it != tagged_objects_.end()) {
it->second = new_tag;
@@ -362,7 +362,7 @@
}
if (select) {
- art::mirror::Object* obj = pair.first.template Read<art::kWithReadBarrier>();
+ art::ObjPtr<art::mirror::Object> obj = pair.first.template Read<art::kWithReadBarrier>();
if (obj != nullptr) {
count++;
if (object_result_ptr != nullptr) {
@@ -386,14 +386,14 @@
}
template <typename T>
-art::mirror::Object* JvmtiWeakTable<T>::Find(T tag) {
+art::ObjPtr<art::mirror::Object> JvmtiWeakTable<T>::Find(T tag) {
art::Thread* self = art::Thread::Current();
art::MutexLock mu(self, allow_disallow_lock_);
Wait(self);
for (auto& pair : tagged_objects_) {
if (tag == pair.second) {
- art::mirror::Object* obj = pair.first.template Read<art::kWithReadBarrier>();
+ art::ObjPtr<art::mirror::Object> obj = pair.first.template Read<art::kWithReadBarrier>();
if (obj != nullptr) {
return obj;
}
diff --git a/openjdkjvmti/jvmti_weak_table.h b/openjdkjvmti/jvmti_weak_table.h
index cba8ef0..ea0d023 100644
--- a/openjdkjvmti/jvmti_weak_table.h
+++ b/openjdkjvmti/jvmti_weak_table.h
@@ -60,25 +60,25 @@
// Remove the mapping for the given object, returning whether such a mapping existed (and the old
// value).
- ALWAYS_INLINE bool Remove(art::mirror::Object* obj, /* out */ T* tag)
+ ALWAYS_INLINE bool Remove(art::ObjPtr<art::mirror::Object> obj, /* out */ T* tag)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(!allow_disallow_lock_);
- ALWAYS_INLINE bool RemoveLocked(art::mirror::Object* obj, /* out */ T* tag)
+ ALWAYS_INLINE bool RemoveLocked(art::ObjPtr<art::mirror::Object> obj, /* out */ T* tag)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_);
// Set the mapping for the given object. Returns true if this overwrites an already existing
// mapping.
- ALWAYS_INLINE virtual bool Set(art::mirror::Object* obj, T tag)
+ ALWAYS_INLINE virtual bool Set(art::ObjPtr<art::mirror::Object> obj, T tag)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(!allow_disallow_lock_);
- ALWAYS_INLINE virtual bool SetLocked(art::mirror::Object* obj, T tag)
+ ALWAYS_INLINE virtual bool SetLocked(art::ObjPtr<art::mirror::Object> obj, T tag)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_);
// Return the value associated with the given object. Returns true if the mapping exists, false
// otherwise.
- bool GetTag(art::mirror::Object* obj, /* out */ T* result)
+ bool GetTag(art::ObjPtr<art::mirror::Object> obj, /* out */ T* result)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(!allow_disallow_lock_) {
art::Thread* self = art::Thread::Current();
@@ -87,7 +87,7 @@
return GetTagLocked(self, obj, result);
}
- bool GetTagLocked(art::mirror::Object* obj, /* out */ T* result)
+ bool GetTagLocked(art::ObjPtr<art::mirror::Object> obj, /* out */ T* result)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_) {
art::Thread* self = art::Thread::Current();
@@ -118,7 +118,7 @@
ALWAYS_INLINE void Unlock() RELEASE(allow_disallow_lock_);
ALWAYS_INLINE void AssertLocked() ASSERT_CAPABILITY(allow_disallow_lock_);
- ALWAYS_INLINE art::mirror::Object* Find(T tag)
+ ALWAYS_INLINE art::ObjPtr<art::mirror::Object> Find(T tag)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(!allow_disallow_lock_);
@@ -132,16 +132,16 @@
private:
ALWAYS_INLINE
- bool SetLocked(art::Thread* self, art::mirror::Object* obj, T tag)
+ bool SetLocked(art::Thread* self, art::ObjPtr<art::mirror::Object> obj, T tag)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_);
ALWAYS_INLINE
- bool RemoveLocked(art::Thread* self, art::mirror::Object* obj, /* out */ T* tag)
+ bool RemoveLocked(art::Thread* self, art::ObjPtr<art::mirror::Object> obj, /* out */ T* tag)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_);
- bool GetTagLocked(art::Thread* self, art::mirror::Object* obj, /* out */ T* result)
+ bool GetTagLocked(art::Thread* self, art::ObjPtr<art::mirror::Object> obj, /* out */ T* result)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_) {
auto it = tagged_objects_.find(art::GcRoot<art::mirror::Object>(obj));
@@ -165,7 +165,7 @@
// Slow-path for GetTag. We didn't find the object, but we might be storing from-pointers and
// are asked to retrieve with a to-pointer.
ALWAYS_INLINE
- bool GetTagSlowPath(art::Thread* self, art::mirror::Object* obj, /* out */ T* result)
+ bool GetTagSlowPath(art::Thread* self, art::ObjPtr<art::mirror::Object> obj, /* out */ T* result)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_);
diff --git a/openjdkjvmti/object_tagging.cc b/openjdkjvmti/object_tagging.cc
index 0a51bf2..d52933a 100644
--- a/openjdkjvmti/object_tagging.cc
+++ b/openjdkjvmti/object_tagging.cc
@@ -71,7 +71,7 @@
jvmti_env_, art::Thread::Current(), tag);
}
-bool ObjectTagTable::Set(art::mirror::Object* obj, jlong new_tag) {
+bool ObjectTagTable::Set(art::ObjPtr<art::mirror::Object> obj, jlong new_tag) {
if (new_tag == 0) {
jlong tmp;
return Remove(obj, &tmp);
@@ -79,7 +79,7 @@
return JvmtiWeakTable<jlong>::Set(obj, new_tag);
}
-bool ObjectTagTable::SetLocked(art::mirror::Object* obj, jlong new_tag) {
+bool ObjectTagTable::SetLocked(art::ObjPtr<art::mirror::Object> obj, jlong new_tag) {
if (new_tag == 0) {
jlong tmp;
return RemoveLocked(obj, &tmp);
diff --git a/openjdkjvmti/object_tagging.h b/openjdkjvmti/object_tagging.h
index ca05a05..bd72ce3 100644
--- a/openjdkjvmti/object_tagging.h
+++ b/openjdkjvmti/object_tagging.h
@@ -61,21 +61,21 @@
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(!allow_disallow_lock_);
- bool Set(art::mirror::Object* obj, jlong tag) override
+ bool Set(art::ObjPtr<art::mirror::Object> obj, jlong tag) override
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(!allow_disallow_lock_);
- bool SetLocked(art::mirror::Object* obj, jlong tag) override
+ bool SetLocked(art::ObjPtr<art::mirror::Object> obj, jlong tag) override
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_);
- jlong GetTagOrZero(art::mirror::Object* obj)
+ jlong GetTagOrZero(art::ObjPtr<art::mirror::Object> obj)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(!allow_disallow_lock_) {
jlong tmp = 0;
GetTag(obj, &tmp);
return tmp;
}
- jlong GetTagOrZeroLocked(art::mirror::Object* obj)
+ jlong GetTagOrZeroLocked(art::ObjPtr<art::mirror::Object> obj)
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(allow_disallow_lock_) {
jlong tmp = 0;
diff --git a/test/905-object-free/expected.txt b/test/905-object-free/expected.txt
index c226df7..dfcd7b6 100644
--- a/test/905-object-free/expected.txt
+++ b/test/905-object-free/expected.txt
@@ -10,4 +10,4 @@
---
[]
---
-Free counts 100000 100000
+Free counts 200000 200000
diff --git a/test/905-object-free/src/art/Test905.java b/test/905-object-free/src/art/Test905.java
index dddd1aa..367da99 100644
--- a/test/905-object-free/src/art/Test905.java
+++ b/test/905-object-free/src/art/Test905.java
@@ -20,6 +20,7 @@
import java.lang.ref.ReferenceQueue;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.function.BiConsumer;
public class Test905 {
// Taken from jdwp tests.
@@ -110,26 +111,59 @@
System.out.println("---");
}
- private static void stressAllocate(int i) {
+ private static void stressAllocate(int i, BiConsumer<Integer, Object> saver) {
Object obj = new Object();
Main.setTag(obj, i);
setTag2(obj, i + 1);
+ saver.accept(i, obj);
}
private static void stress() {
getCollectedTags(0);
getCollectedTags(1);
- // Allocate objects.
- for (int i = 1; i <= 100000; ++i) {
- stressAllocate(i);
+ final int num_obj = 400000;
+ final Object[] saved = new Object[num_obj/2];
+ // Allocate objects, Save every other one. We want to be sure that it's only the deleted objects
+ // that get their tags cleared and non-deleted objects correctly keep track of their tags.
+ for (int i = 1; i <= num_obj; ++i) {
+ stressAllocate(i, (idx, obj) -> {
+ if ((idx.intValue() - 1) % 2 == 0) {
+ saved[(idx.intValue() - 1)/2] = obj;
+ }
+ });
}
gcAndWait();
long[] freedTags1 = getCollectedTags(0);
long[] freedTags2 = getCollectedTags(1);
+ // Sort the freedtags
+ Arrays.sort(freedTags1);
+ Arrays.sort(freedTags2);
+ // Make sure we freed all the ones we expect to and both envs agree on this.
System.out.println("Free counts " + freedTags1.length + " " + freedTags2.length);
for (int i = 0; i < freedTags1.length; ++i) {
if (freedTags1[i] + 1 != freedTags2[i]) {
- System.out.println("Mismatched tags " + freedTags1[i] + " " + freedTags2[i]);
+ System.out.println("Mismatched tags " + (freedTags1[i] + 1) + " " + freedTags2[i]);
+ }
+ }
+ // Make sure the saved-tags aren't present.
+ for (int i = 0; i < saved.length; i++) {
+ // index = (tag - 1)/2 --> (index * 2) + 1 = tag
+ long expectedTag1 = (i * 2) + 1;
+ if (Main.getTag(saved[i]) != expectedTag1) {
+ System.out.println("Saved object has unexpected tag in env 1. Expected "
+ + expectedTag1 + " got " + Main.getTag(saved[i]));
+ }
+ if (getTag2(saved[i]) != 1 + expectedTag1) {
+ System.out.println("Saved object has unexpected tag in env 2. Expected "
+ + (expectedTag1 + 1) + " got " + getTag2(saved[i]));
+ }
+ if (Arrays.binarySearch(freedTags1, expectedTag1) >= 0) {
+ System.out.println("Saved object was marked as deleted in env 1. Object was "
+ + expectedTag1);
+ }
+ if (Arrays.binarySearch(freedTags2, expectedTag1 + 1) >= 0) {
+ System.out.println("Saved object was marked as deleted in env 2. Object was "
+ + (expectedTag1 + 1));
}
}
}
@@ -161,4 +195,5 @@
private static native void enableFreeTracking(boolean enable);
private static native long[] getCollectedTags(int index);
private static native void setTag2(Object o, long tag);
+ private static native long getTag2(Object o);
}
diff --git a/test/905-object-free/tracking_free.cc b/test/905-object-free/tracking_free.cc
index bf86c9a..d85d9d3 100644
--- a/test/905-object-free/tracking_free.cc
+++ b/test/905-object-free/tracking_free.cc
@@ -18,6 +18,7 @@
#include <cstdio>
#include <iostream>
+#include <mutex>
#include <vector>
#include "android-base/logging.h"
@@ -33,17 +34,23 @@
namespace art {
namespace Test905ObjectFree {
+// The ObjectFree functions aren't required to be called on any particular thread so use these
+// mutexs to control access to the collected_tags lists.
+std::mutex ct1_mutex;
static std::vector<jlong> collected_tags1;
+std::mutex ct2_mutex;
static std::vector<jlong> collected_tags2;
jvmtiEnv* jvmti_env2;
static void JNICALL ObjectFree1(jvmtiEnv* ti_env, jlong tag) {
+ std::lock_guard<std::mutex> mu(ct1_mutex);
CHECK_EQ(ti_env, jvmti_env);
collected_tags1.push_back(tag);
}
static void JNICALL ObjectFree2(jvmtiEnv* ti_env, jlong tag) {
+ std::lock_guard<std::mutex> mu(ct2_mutex);
CHECK_EQ(ti_env, jvmti_env2);
collected_tags2.push_back(tag);
}
@@ -84,6 +91,7 @@
extern "C" JNIEXPORT jlongArray JNICALL Java_art_Test905_getCollectedTags(
JNIEnv* env, jclass klass ATTRIBUTE_UNUSED, jint index) {
+ std::lock_guard<std::mutex> mu((index == 0) ? ct1_mutex : ct2_mutex);
std::vector<jlong>& tags = (index == 0) ? collected_tags1 : collected_tags2;
jlongArray ret = env->NewLongArray(tags.size());
if (ret == nullptr) {
@@ -96,6 +104,14 @@
return ret;
}
+extern "C" JNIEXPORT jlong JNICALL Java_art_Test905_getTag2(
+ JNIEnv* env, jclass klass ATTRIBUTE_UNUSED, jobject obj) {
+ jlong tag;
+ jvmtiError ret = jvmti_env2->GetTag(obj, &tag);
+ JvmtiErrorToException(env, jvmti_env, ret);
+ return tag;
+}
+
extern "C" JNIEXPORT void JNICALL Java_art_Test905_setTag2(
JNIEnv* env, jclass klass ATTRIBUTE_UNUSED, jobject obj, jlong tag) {
jvmtiError ret = jvmti_env2->SetTag(obj, tag);