Improve performance of JNI field operations.
Change improves performance of JniField Fadden test by around 25%, we're still
2x slower than Dalvik.
Aggressively inline ScopedObjectAccess, Thread::SetState and field helpers.
If we're not contention logging don't call MilliTime (avoids a double register
spill).
Remove (broken?) thread checks in scoped object access, they are redundant with
ones being performed in check JNI.
Change-Id: I128eed1e4205d4d540d5c6f430ef9e3853745585
diff --git a/src/base/mutex-inl.h b/src/base/mutex-inl.h
index 122fad5..0da0e18 100644
--- a/src/base/mutex-inl.h
+++ b/src/base/mutex-inl.h
@@ -40,21 +40,29 @@
class ScopedContentionRecorder {
public:
+#if CONTENTION_LOGGING
ScopedContentionRecorder(BaseMutex* mutex, uint64_t blocked_tid, uint64_t owner_tid) :
mutex_(mutex), blocked_tid_(blocked_tid), owner_tid_(owner_tid),
start_milli_time_(MilliTime()) {
}
+#else
+ ScopedContentionRecorder(BaseMutex*, uint64_t, uint64_t) {}
+#endif
~ScopedContentionRecorder() {
+#if CONTENTION_LOGGING
uint64_t end_milli_time = MilliTime();
mutex_->RecordContention(blocked_tid_, owner_tid_, end_milli_time - start_milli_time_);
+#endif
}
private:
+#if CONTENTION_LOGGING
BaseMutex* const mutex_;
- uint64_t blocked_tid_;
- uint64_t owner_tid_;
+ const uint64_t blocked_tid_;
+ const uint64_t owner_tid_;
const uint64_t start_milli_time_;
+#endif
};
static inline uint64_t SafeGetTid(const Thread* self) {
@@ -124,7 +132,7 @@
bool done = false;
do {
int32_t cur_state = state_;
- if (cur_state >= 0) {
+ if (LIKELY(cur_state >= 0)) {
// Add as an extra reader.
done = android_atomic_acquire_cas(cur_state, cur_state + 1, &state_) == 0;
} else {
diff --git a/src/mirror/field-inl.h b/src/mirror/field-inl.h
index b73cf19..cda461b 100644
--- a/src/mirror/field-inl.h
+++ b/src/mirror/field-inl.h
@@ -19,6 +19,13 @@
#include "field.h"
+#include "base/logging.h"
+#include "gc/card_table-inl.h"
+#include "jvalue.h"
+#include "object-inl.h"
+#include "object_utils.h"
+#include "primitive.h"
+
namespace art {
namespace mirror {
@@ -48,6 +55,166 @@
return MemberOffset(GetField32(OFFSET_OF_OBJECT_MEMBER(Field, offset_), false));
}
+inline uint32_t Field::Get32(const Object* object) const {
+ DCHECK(object != NULL) << PrettyField(this);
+ DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
+ return object->GetField32(GetOffset(), IsVolatile());
+}
+
+inline void Field::Set32(Object* object, uint32_t new_value) const {
+ DCHECK(object != NULL) << PrettyField(this);
+ DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
+ object->SetField32(GetOffset(), new_value, IsVolatile());
+}
+
+inline uint64_t Field::Get64(const Object* object) const {
+ DCHECK(object != NULL) << PrettyField(this);
+ DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
+ return object->GetField64(GetOffset(), IsVolatile());
+}
+
+inline void Field::Set64(Object* object, uint64_t new_value) const {
+ DCHECK(object != NULL) << PrettyField(this);
+ DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
+ object->SetField64(GetOffset(), new_value, IsVolatile());
+}
+
+inline Object* Field::GetObj(const Object* object) const {
+ DCHECK(object != NULL) << PrettyField(this);
+ DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
+ return object->GetFieldObject<Object*>(GetOffset(), IsVolatile());
+}
+
+inline void Field::SetObj(Object* object, const Object* new_value) const {
+ DCHECK(object != NULL) << PrettyField(this);
+ DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
+ object->SetFieldObject(GetOffset(), new_value, IsVolatile());
+}
+
+inline bool Field::GetBoolean(const Object* object) const {
+ DCHECK_EQ(Primitive::kPrimBoolean, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ return Get32(object);
+}
+
+inline void Field::SetBoolean(Object* object, bool z) const {
+ DCHECK_EQ(Primitive::kPrimBoolean, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ Set32(object, z);
+}
+
+inline int8_t Field::GetByte(const Object* object) const {
+ DCHECK_EQ(Primitive::kPrimByte, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ return Get32(object);
+}
+
+inline void Field::SetByte(Object* object, int8_t b) const {
+ DCHECK_EQ(Primitive::kPrimByte, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ Set32(object, b);
+}
+
+inline uint16_t Field::GetChar(const Object* object) const {
+ DCHECK_EQ(Primitive::kPrimChar, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ return Get32(object);
+}
+
+inline void Field::SetChar(Object* object, uint16_t c) const {
+ DCHECK_EQ(Primitive::kPrimChar, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ Set32(object, c);
+}
+
+inline int16_t Field::GetShort(const Object* object) const {
+ DCHECK_EQ(Primitive::kPrimShort, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ return Get32(object);
+}
+
+inline void Field::SetShort(Object* object, int16_t s) const {
+ DCHECK_EQ(Primitive::kPrimShort, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ Set32(object, s);
+}
+
+inline int32_t Field::GetInt(const Object* object) const {
+#ifndef NDEBUG
+ Primitive::Type type = FieldHelper(this).GetTypeAsPrimitiveType();
+ CHECK(type == Primitive::kPrimInt || type == Primitive::kPrimFloat) << PrettyField(this);
+#endif
+ return Get32(object);
+}
+
+inline void Field::SetInt(Object* object, int32_t i) const {
+#ifndef NDEBUG
+ Primitive::Type type = FieldHelper(this).GetTypeAsPrimitiveType();
+ CHECK(type == Primitive::kPrimInt || type == Primitive::kPrimFloat) << PrettyField(this);
+#endif
+ Set32(object, i);
+}
+
+inline int64_t Field::GetLong(const Object* object) const {
+#ifndef NDEBUG
+ Primitive::Type type = FieldHelper(this).GetTypeAsPrimitiveType();
+ CHECK(type == Primitive::kPrimLong || type == Primitive::kPrimDouble) << PrettyField(this);
+#endif
+ return Get64(object);
+}
+
+inline void Field::SetLong(Object* object, int64_t j) const {
+#ifndef NDEBUG
+ Primitive::Type type = FieldHelper(this).GetTypeAsPrimitiveType();
+ CHECK(type == Primitive::kPrimLong || type == Primitive::kPrimDouble) << PrettyField(this);
+#endif
+ Set64(object, j);
+}
+
+inline float Field::GetFloat(const Object* object) const {
+ DCHECK_EQ(Primitive::kPrimFloat, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ JValue bits;
+ bits.SetI(Get32(object));
+ return bits.GetF();
+}
+
+inline void Field::SetFloat(Object* object, float f) const {
+ DCHECK_EQ(Primitive::kPrimFloat, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ JValue bits;
+ bits.SetF(f);
+ Set32(object, bits.GetI());
+}
+
+inline double Field::GetDouble(const Object* object) const {
+ DCHECK_EQ(Primitive::kPrimDouble, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ JValue bits;
+ bits.SetJ(Get64(object));
+ return bits.GetD();
+}
+
+inline void Field::SetDouble(Object* object, double d) const {
+ DCHECK_EQ(Primitive::kPrimDouble, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ JValue bits;
+ bits.SetD(d);
+ Set64(object, bits.GetJ());
+}
+
+inline Object* Field::GetObject(const Object* object) const {
+ DCHECK_EQ(Primitive::kPrimNot, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ return GetObj(object);
+}
+
+inline void Field::SetObject(Object* object, const Object* l) const {
+ DCHECK_EQ(Primitive::kPrimNot, FieldHelper(this).GetTypeAsPrimitiveType())
+ << PrettyField(this);
+ SetObj(object, l);
+}
+
} // namespace mirror
} // namespace art
diff --git a/src/mirror/field.cc b/src/mirror/field.cc
index dab7868..6e2559a 100644
--- a/src/mirror/field.cc
+++ b/src/mirror/field.cc
@@ -52,172 +52,5 @@
SetField32(OFFSET_OF_OBJECT_MEMBER(Field, offset_), num_bytes.Uint32Value(), false);
}
-uint32_t Field::Get32(const Object* object) const {
- DCHECK(object != NULL) << PrettyField(this);
- DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
- return object->GetField32(GetOffset(), IsVolatile());
-}
-
-void Field::Set32(Object* object, uint32_t new_value) const {
- DCHECK(object != NULL) << PrettyField(this);
- DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
- object->SetField32(GetOffset(), new_value, IsVolatile());
-}
-
-uint64_t Field::Get64(const Object* object) const {
- DCHECK(object != NULL) << PrettyField(this);
- DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
- return object->GetField64(GetOffset(), IsVolatile());
-}
-
-void Field::Set64(Object* object, uint64_t new_value) const {
- DCHECK(object != NULL) << PrettyField(this);
- DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
- object->SetField64(GetOffset(), new_value, IsVolatile());
-}
-
-Object* Field::GetObj(const Object* object) const {
- DCHECK(object != NULL) << PrettyField(this);
- DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
- return object->GetFieldObject<Object*>(GetOffset(), IsVolatile());
-}
-
-void Field::SetObj(Object* object, const Object* new_value) const {
- DCHECK(object != NULL) << PrettyField(this);
- DCHECK(!IsStatic() || (object == GetDeclaringClass()) || !Runtime::Current()->IsStarted());
- object->SetFieldObject(GetOffset(), new_value, IsVolatile());
-}
-
-bool Field::GetBoolean(const Object* object) const {
- DCHECK_EQ(Primitive::kPrimBoolean, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- return Get32(object);
-}
-
-void Field::SetBoolean(Object* object, bool z) const {
- DCHECK_EQ(Primitive::kPrimBoolean, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- Set32(object, z);
-}
-
-int8_t Field::GetByte(const Object* object) const {
- DCHECK_EQ(Primitive::kPrimByte, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- return Get32(object);
-}
-
-void Field::SetByte(Object* object, int8_t b) const {
- DCHECK_EQ(Primitive::kPrimByte, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- Set32(object, b);
-}
-
-uint16_t Field::GetChar(const Object* object) const {
- DCHECK_EQ(Primitive::kPrimChar, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- return Get32(object);
-}
-
-void Field::SetChar(Object* object, uint16_t c) const {
- DCHECK_EQ(Primitive::kPrimChar, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- Set32(object, c);
-}
-
-int16_t Field::GetShort(const Object* object) const {
- DCHECK_EQ(Primitive::kPrimShort, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- return Get32(object);
-}
-
-void Field::SetShort(Object* object, int16_t s) const {
- DCHECK_EQ(Primitive::kPrimShort, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- Set32(object, s);
-}
-
-int32_t Field::GetInt(const Object* object) const {
-#ifndef NDEBUG
- Primitive::Type type = FieldHelper(this).GetTypeAsPrimitiveType();
- CHECK(type == Primitive::kPrimInt || type == Primitive::kPrimFloat) << PrettyField(this);
-#endif
- return Get32(object);
-}
-
-void Field::SetInt(Object* object, int32_t i) const {
-#ifndef NDEBUG
- Primitive::Type type = FieldHelper(this).GetTypeAsPrimitiveType();
- CHECK(type == Primitive::kPrimInt || type == Primitive::kPrimFloat) << PrettyField(this);
-#endif
- Set32(object, i);
-}
-
-int64_t Field::GetLong(const Object* object) const {
-#ifndef NDEBUG
- Primitive::Type type = FieldHelper(this).GetTypeAsPrimitiveType();
- CHECK(type == Primitive::kPrimLong || type == Primitive::kPrimDouble) << PrettyField(this);
-#endif
- return Get64(object);
-}
-
-void Field::SetLong(Object* object, int64_t j) const {
-#ifndef NDEBUG
- Primitive::Type type = FieldHelper(this).GetTypeAsPrimitiveType();
- CHECK(type == Primitive::kPrimLong || type == Primitive::kPrimDouble) << PrettyField(this);
-#endif
- Set64(object, j);
-}
-
-union Bits {
- jdouble d;
- jfloat f;
- jint i;
- jlong j;
-};
-
-float Field::GetFloat(const Object* object) const {
- DCHECK_EQ(Primitive::kPrimFloat, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- Bits bits;
- bits.i = Get32(object);
- return bits.f;
-}
-
-void Field::SetFloat(Object* object, float f) const {
- DCHECK_EQ(Primitive::kPrimFloat, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- Bits bits;
- bits.f = f;
- Set32(object, bits.i);
-}
-
-double Field::GetDouble(const Object* object) const {
- DCHECK_EQ(Primitive::kPrimDouble, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- Bits bits;
- bits.j = Get64(object);
- return bits.d;
-}
-
-void Field::SetDouble(Object* object, double d) const {
- DCHECK_EQ(Primitive::kPrimDouble, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- Bits bits;
- bits.d = d;
- Set64(object, bits.j);
-}
-
-Object* Field::GetObject(const Object* object) const {
- DCHECK_EQ(Primitive::kPrimNot, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- return GetObj(object);
-}
-
-void Field::SetObject(Object* object, const Object* l) const {
- DCHECK_EQ(Primitive::kPrimNot, FieldHelper(this).GetTypeAsPrimitiveType())
- << PrettyField(this);
- SetObj(object, l);
-}
-
} // namespace mirror
} // namespace art
diff --git a/src/reflection.cc b/src/reflection.cc
index 16a5502..d678ebd 100644
--- a/src/reflection.cc
+++ b/src/reflection.cc
@@ -22,7 +22,7 @@
#include "mirror/abstract_method-inl.h"
#include "mirror/class.h"
#include "mirror/class-inl.h"
-#include "mirror/object-inl.h"
+#include "mirror/field-inl.h"
#include "mirror/object_array.h"
#include "mirror/object_array-inl.h"
#include "object_utils.h"
diff --git a/src/runtime.cc b/src/runtime.cc
index 085a9bf..ae0f49b 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -40,7 +40,7 @@
#include "mirror/array.h"
#include "mirror/class_loader.h"
#include "mirror/field.h"
-#include "mirror/object-inl.h"
+#include "mirror/field-inl.h"
#include "mirror/throwable.h"
#include "monitor.h"
#include "oat_file.h"
diff --git a/src/scoped_thread_state_change.h b/src/scoped_thread_state_change.h
index 31f178d..709109a 100644
--- a/src/scoped_thread_state_change.h
+++ b/src/scoped_thread_state_change.h
@@ -30,9 +30,9 @@
class ScopedThreadStateChange {
public:
ScopedThreadStateChange(Thread* self, ThreadState new_thread_state)
- LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_)
+ LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_) __attribute__ ((always_inline))
: self_(self), thread_state_(new_thread_state), expected_has_no_thread_(false) {
- if (self_ == NULL) {
+ if (UNLIKELY(self_ == NULL)) {
// Value chosen arbitrarily and won't be used in the destructor since thread_ == NULL.
old_thread_state_ = kTerminated;
MutexLock mu(NULL, *Locks::runtime_shutdown_lock_);
@@ -61,8 +61,8 @@
}
}
- ~ScopedThreadStateChange() LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_) {
- if (self_ == NULL) {
+ ~ScopedThreadStateChange() LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_) __attribute__ ((always_inline)) {
+ if (UNLIKELY(self_ == NULL)) {
if (!expected_has_no_thread_) {
MutexLock mu(NULL, *Locks::runtime_shutdown_lock_);
Runtime* runtime = Runtime::Current();
@@ -120,7 +120,7 @@
class ScopedObjectAccessUnchecked : public ScopedThreadStateChange {
public:
explicit ScopedObjectAccessUnchecked(JNIEnv* env)
- LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_)
+ LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_) __attribute__ ((always_inline))
: ScopedThreadStateChange(ThreadForEnv(env), kRunnable),
env_(reinterpret_cast<JNIEnvExt*>(env)), vm_(env_->vm) {
self_->VerifyStack();
@@ -131,9 +131,6 @@
: ScopedThreadStateChange(self, kRunnable),
env_(reinterpret_cast<JNIEnvExt*>(self->GetJniEnv())),
vm_(env_ != NULL ? env_->vm : NULL) {
- if (Vm() != NULL && !Vm()->work_around_app_jni_bugs && self != Thread::Current()) {
- UnexpectedThreads(self, Thread::Current());
- }
self_->VerifyStack();
}
@@ -142,6 +139,9 @@
explicit ScopedObjectAccessUnchecked(JavaVM* vm)
: ScopedThreadStateChange(), env_(NULL), vm_(reinterpret_cast<JavaVMExt*>(vm)) {}
+ ~ScopedObjectAccessUnchecked() __attribute__ ((always_inline)) {
+ }
+
JNIEnvExt* Env() const {
return env_;
}
@@ -259,21 +259,7 @@
private:
static Thread* ThreadForEnv(JNIEnv* env) {
JNIEnvExt* full_env(reinterpret_cast<JNIEnvExt*>(env));
- bool work_around_app_jni_bugs = full_env->vm->work_around_app_jni_bugs;
- Thread* env_self = full_env->self;
- Thread* self = work_around_app_jni_bugs ? Thread::Current() : env_self;
- if (!work_around_app_jni_bugs && self != env_self) {
- UnexpectedThreads(env_self, self);
- }
- return self;
- }
-
- static void UnexpectedThreads(Thread* found_self, Thread* expected_self) {
- // TODO: pass through function name so we can use it here instead of NULL...
- JniAbortF(NULL, "JNIEnv for %s used on %s",
- found_self != NULL ? ToStr<Thread>(*found_self).c_str() : "NULL",
- expected_self != NULL ? ToStr<Thread>(*expected_self).c_str() : "NULL");
-
+ return full_env->self;
}
// The full JNIEnv.
@@ -289,7 +275,7 @@
public:
explicit ScopedObjectAccess(JNIEnv* env)
LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_)
- SHARED_LOCK_FUNCTION(Locks::mutator_lock_)
+ SHARED_LOCK_FUNCTION(Locks::mutator_lock_) __attribute__ ((always_inline))
: ScopedObjectAccessUnchecked(env) {
Locks::mutator_lock_->AssertSharedHeld(Self());
}
@@ -301,7 +287,7 @@
Locks::mutator_lock_->AssertSharedHeld(Self());
}
- ~ScopedObjectAccess() UNLOCK_FUNCTION(Locks::mutator_lock_) {
+ ~ScopedObjectAccess() UNLOCK_FUNCTION(Locks::mutator_lock_) __attribute__ ((always_inline)) {
// Base class will release share of lock. Invoked after this destructor.
}
diff --git a/src/thread-inl.h b/src/thread-inl.h
index 93aa10e..cf92a1c 100644
--- a/src/thread-inl.h
+++ b/src/thread-inl.h
@@ -24,6 +24,16 @@
namespace art {
+inline ThreadState Thread::SetState(ThreadState new_state) {
+ // Cannot use this code to change into Runnable as changing to Runnable should fail if
+ // old_state_and_flags.suspend_request is true.
+ DCHECK_NE(new_state, kRunnable);
+ DCHECK_EQ(this, Thread::Current());
+ union StateAndFlags old_state_and_flags = state_and_flags_;
+ state_and_flags_.as_struct.state = new_state;
+ return static_cast<ThreadState>(old_state_and_flags.as_struct.state);
+}
+
inline void Thread::AssertThreadSuspensionIsAllowable(bool check_locks) const {
#ifdef NDEBUG
UNUSED(check_locks); // Keep GCC happy about unused parameters.
diff --git a/src/thread.cc b/src/thread.cc
index 5b1a325..37c6783 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -41,7 +41,7 @@
#include "mirror/abstract_method-inl.h"
#include "mirror/class-inl.h"
#include "mirror/class_loader.h"
-#include "mirror/object-inl.h"
+#include "mirror/field-inl.h"
#include "mirror/object_array-inl.h"
#include "mirror/stack_trace_element.h"
#include "monitor.h"
@@ -531,16 +531,6 @@
android_atomic_and(-1 ^ flag, &state_and_flags_.as_int);
}
-ThreadState Thread::SetState(ThreadState new_state) {
- // Cannot use this code to change into Runnable as changing to Runnable should fail if
- // old_state_and_flags.suspend_request is true.
- DCHECK_NE(new_state, kRunnable);
- DCHECK_EQ(this, Thread::Current());
- union StateAndFlags old_state_and_flags = state_and_flags_;
- state_and_flags_.as_struct.state = new_state;
- return static_cast<ThreadState>(old_state_and_flags.as_struct.state);
-}
-
// Attempt to rectify locks so that we dump thread list with required locks before exiting.
static void UnsafeLogFatalForSuspendCount(Thread* self, Thread* thread) NO_THREAD_SAFETY_ANALYSIS {
LOG(ERROR) << *thread << " suspend count already zero.";