Do not create HandleScope for JNI transitions.
We previously crated a HandleScope in the JNI transition
frame to hold references passed as jobject (jclass, etc.)
to the native function and these references were actually
spilled twice during the transition.
We now construct the jobject as a pointer to the reference
spilled in the reserved out vreg area in the caller's frame.
And the jclass for static methods is just a pointer to the
method's declaring class. This reduces the amount of work
required in the JNI transition, both on entry (in compiled
stubs) and exit (in JniMethodEnd*).
Some additional work is required when GC visits references
of a native method as we need to walk over the method's
shorty which was unnecessary for a HandleScope.
Also fix Thread::InitStackHwm() to calculate correct stack
size needed by the new Thread::IsJniTransitionReference().
The results for StringToBytesBenchmark on blueline little
cores running at fixed frequency 1420800 are approximately
arm64 (medians from 3 runs) before after
timeGetBytesAscii EMPTY 447.33 436.86
timeGetBytesIso88591 EMPTY 440.52 431.13
timeGetBytesUtf8 EMPTY 432.31 409.82
arm (medians from 3 runs) before after
timeGetBytesAscii EMPTY 500.53 490.87
timeGetBytesIso88591 EMPTY 496.45 495.30
timeGetBytesUtf8 EMPTY 488.84 472.68
Test: m test-art-host-gtest
Test: testrunner.py --host
Test: testrunner.py --host --gcstress
Test: testrunner.py --host --jit-on-first-use
Test: testrunner.py --host --jit-on-first-use --gcstress
Test: run-gtests.sh
Test: testrunner.py --target --optimizing
Test: boots.
Bug: 172332525
Change-Id: I658f9d87071587b3e89f31c65feca976a11e9cc2
diff --git a/compiler/jni/jni_compiler_test.cc b/compiler/jni/jni_compiler_test.cc
index 3ee7e0e..dc5304c 100644
--- a/compiler/jni/jni_compiler_test.cc
+++ b/compiler/jni/jni_compiler_test.cc
@@ -21,6 +21,7 @@
#include "art_method-inl.h"
#include "base/bit_utils.h"
+#include "base/casts.h"
#include "base/mem_map.h"
#include "class_linker.h"
#include "common_compiler_test.h"
@@ -28,6 +29,7 @@
#include "dex/dex_file.h"
#include "gtest/gtest.h"
#include "indirect_reference_table.h"
+#include "java_frame_root_info.h"
#include "jni/java_vm_ext.h"
#include "jni/jni_internal.h"
#include "mirror/class-inl.h"
@@ -154,19 +156,6 @@
jcharArray, jfloatArray, jshortArray, jdoubleArray, jlongArray>::value;
};
-template <typename ... Args>
-struct count_refs_helper {
- using value_type = size_t;
- static constexpr const size_t value = 0;
-};
-
-template <typename Arg, typename ... Args>
-struct count_refs_helper<Arg, Args ...> {
- using value_type = size_t;
- static constexpr size_t value =
- (jni_type_traits<Arg>::is_ref ? 1 : 0) + count_refs_helper<Args ...>::value;
-};
-
// Base case: No parameters = 0 refs.
size_t count_nonnull_refs_helper() {
return 0;
@@ -399,12 +388,89 @@
jmethodID jmethod_;
private:
+ class ScopedSynchronizedEntryPointOverrides {
+ public:
+ ScopedSynchronizedEntryPointOverrides() {
+ QuickEntryPoints* qpoints = &Thread::Current()->tlsPtr_.quick_entrypoints;
+ jni_method_start_synchronized_original_ = qpoints->pJniMethodStartSynchronized;
+ qpoints->pJniMethodStartSynchronized = JniMethodStartSynchronizedOverride;
+ jni_method_end_synchronized_original_ = qpoints->pJniMethodEndSynchronized;
+ qpoints->pJniMethodEndSynchronized = JniMethodEndSynchronizedOverride;
+ jni_method_end_with_reference_synchronized_original_ =
+ qpoints->pJniMethodEndWithReferenceSynchronized;
+ qpoints->pJniMethodEndWithReferenceSynchronized =
+ JniMethodEndWithReferenceSynchronizedOverride;
+ }
+
+ ~ScopedSynchronizedEntryPointOverrides() {
+ QuickEntryPoints* qpoints = &Thread::Current()->tlsPtr_.quick_entrypoints;
+ qpoints->pJniMethodStartSynchronized = jni_method_start_synchronized_original_;
+ qpoints->pJniMethodEndSynchronized = jni_method_end_synchronized_original_;
+ qpoints->pJniMethodEndWithReferenceSynchronized =
+ jni_method_end_with_reference_synchronized_original_;
+ }
+ };
+
+ static uint32_t JniMethodStartSynchronizedOverride(jobject to_lock, Thread* self);
+ static void JniMethodEndSynchronizedOverride(uint32_t saved_local_ref_cookie,
+ jobject locked,
+ Thread* self);
+ static mirror::Object* JniMethodEndWithReferenceSynchronizedOverride(
+ jobject result,
+ uint32_t saved_local_ref_cookie,
+ jobject locked,
+ Thread* self);
+
+ using StartSynchronizedType = uint32_t (*)(jobject, Thread*);
+ using EndSynchronizedType = void (*)(uint32_t, jobject, Thread*);
+ using EndWithReferenceSynchronizedType = mirror::Object* (*)(jobject, uint32_t, jobject, Thread*);
+
+ static StartSynchronizedType jni_method_start_synchronized_original_;
+ static EndSynchronizedType jni_method_end_synchronized_original_;
+ static EndWithReferenceSynchronizedType jni_method_end_with_reference_synchronized_original_;
+ static uint32_t saved_local_ref_cookie_;
+ static jobject locked_object_;
+
bool check_generic_jni_;
};
jclass JniCompilerTest::jklass_;
jobject JniCompilerTest::jobj_;
jobject JniCompilerTest::class_loader_;
+JniCompilerTest::StartSynchronizedType JniCompilerTest::jni_method_start_synchronized_original_;
+JniCompilerTest::EndSynchronizedType JniCompilerTest::jni_method_end_synchronized_original_;
+JniCompilerTest::EndWithReferenceSynchronizedType
+ JniCompilerTest::jni_method_end_with_reference_synchronized_original_;
+uint32_t JniCompilerTest::saved_local_ref_cookie_;
+jobject JniCompilerTest::locked_object_;
+
+uint32_t JniCompilerTest::JniMethodStartSynchronizedOverride(jobject to_lock, Thread* self) {
+ locked_object_ = to_lock;
+ uint32_t cookie = jni_method_start_synchronized_original_(to_lock, self);
+ saved_local_ref_cookie_ = cookie;
+ return cookie;
+}
+
+void JniCompilerTest::JniMethodEndSynchronizedOverride(uint32_t saved_local_ref_cookie,
+ jobject locked,
+ Thread* self) {
+ EXPECT_EQ(saved_local_ref_cookie_, saved_local_ref_cookie);
+ EXPECT_EQ(locked_object_, locked);
+ jni_method_end_synchronized_original_(saved_local_ref_cookie, locked, self);
+}
+
+mirror::Object* JniCompilerTest::JniMethodEndWithReferenceSynchronizedOverride(
+ jobject result,
+ uint32_t saved_local_ref_cookie,
+ jobject locked,
+ Thread* self) {
+ EXPECT_EQ(saved_local_ref_cookie_, saved_local_ref_cookie);
+ EXPECT_EQ(locked_object_, locked);
+ return jni_method_end_with_reference_synchronized_original_(result,
+ saved_local_ref_cookie,
+ locked,
+ self);
+}
// Test the normal compiler and normal generic JNI only.
// The following features are unsupported in @FastNative:
@@ -553,42 +619,56 @@
BaseHandleScope* const handle_scope_;
};
-// Number of references allocated in JNI ShadowFrames on the given thread.
-static size_t NumJniShadowFrameReferences(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) {
- return self->GetManagedStack()->NumJniShadowFrameReferences();
-}
-
-// Number of references in handle scope on the given thread.
-static size_t NumHandleReferences(Thread* self) {
- size_t count = 0;
- for (BaseHandleScope* cur = self->GetTopHandleScope(); cur != nullptr; cur = cur->GetLink()) {
- count += cur->NumberOfReferences();
+class CountReferencesVisitor : public RootVisitor {
+ public:
+ void VisitRoots(mirror::Object*** roots ATTRIBUTE_UNUSED,
+ size_t count,
+ const RootInfo& info) override
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (info.GetType() == art::RootType::kRootJavaFrame) {
+ const JavaFrameRootInfo& jrfi = static_cast<const JavaFrameRootInfo&>(info);
+ if (jrfi.GetVReg() == JavaFrameRootInfo::kNativeReferenceArgument) {
+ DCHECK_EQ(count, 1u);
+ num_references_ += count;
+ }
+ }
}
- return count;
-}
+
+ void VisitRoots(mirror::CompressedReference<mirror::Object>** roots ATTRIBUTE_UNUSED,
+ size_t count ATTRIBUTE_UNUSED,
+ const RootInfo& info) override
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ CHECK_NE(info.GetType(), art::RootType::kRootJavaFrame);
+ }
+
+ size_t NumReferences() const {
+ return num_references_;
+ }
+
+ private:
+ size_t num_references_ = 0u;
+};
// Number of references allocated in handle scopes & JNI shadow frames on this thread.
static size_t NumStackReferences(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) {
- return NumHandleReferences(self) + NumJniShadowFrameReferences(self);
+ CountReferencesVisitor visitor;
+ self->VisitRoots(&visitor, kVisitRootFlagAllRoots);
+ return visitor.NumReferences();
}
-static void expectNumStackReferences(size_t val1, size_t val2) {
+static void expectNumStackReferences(size_t expected) {
// In rare cases when JNI functions call themselves recursively,
// disable this test because it will have a false negative.
if (!IsCurrentJniCritical() && ScopedDisableCheckNumStackReferences::sCheckNumStackReferences) {
/* @CriticalNative doesn't build a HandleScope, so this test is meaningless then. */
ScopedObjectAccess soa(Thread::Current());
- size_t actual_num = NumStackReferences(Thread::Current());
- // XX: Not too sure what's going on.
- // Sometimes null references get placed and sometimes they don't?
- EXPECT_TRUE(val1 == actual_num || val2 == actual_num)
- << "expected either " << val1 << " or " << val2
- << " number of stack references, but got: " << actual_num;
+ size_t num_references = NumStackReferences(Thread::Current());
+ EXPECT_EQ(expected, num_references);
}
}
-#define EXPECT_NUM_STACK_REFERENCES(val1, val2) expectNumStackReferences(val1, val2)
+#define EXPECT_NUM_STACK_REFERENCES(expected) expectNumStackReferences(expected)
template <typename T, T* fn>
struct make_jni_test_decorator;
@@ -600,9 +680,9 @@
EXPECT_THREAD_STATE_FOR_CURRENT_JNI();
EXPECT_MUTATOR_LOCK_FOR_CURRENT_JNI();
EXPECT_JNI_ENV_AND_CLASS_FOR_CURRENT_JNI(env, kls);
- // All incoming parameters + the jclass get put into the transition's StackHandleScope.
- EXPECT_NUM_STACK_REFERENCES(count_nonnull_refs(kls, args...),
- (count_refs_helper<jclass, Args...>::value));
+ // All incoming parameters get spilled into the JNI transition frame.
+ // The `jclass` is just a reference to the method's declaring class field.
+ EXPECT_NUM_STACK_REFERENCES(count_nonnull_refs(args...));
return fn(env, kls, args...);
}
@@ -615,9 +695,8 @@
EXPECT_THREAD_STATE_FOR_CURRENT_JNI();
EXPECT_MUTATOR_LOCK_FOR_CURRENT_JNI();
EXPECT_JNI_ENV_AND_OBJECT_FOR_CURRENT_JNI(env, thisObj);
- // All incoming parameters + the implicit 'this' get put into the transition's StackHandleScope.
- EXPECT_NUM_STACK_REFERENCES(count_nonnull_refs(thisObj, args...),
- (count_refs_helper<jobject, Args...>::value));
+ // All incoming parameters + the implicit 'this' get spilled into the JNI transition frame.
+ EXPECT_NUM_STACK_REFERENCES(count_nonnull_refs(thisObj, args...));
return fn(env, thisObj, args...);
}
@@ -804,6 +883,7 @@
void JniCompilerTest::CompileAndRun_fooJJ_synchronizedImpl() {
SetUpForTest(false, "fooJJ_synchronized", "(JJ)J",
CURRENT_JNI_WRAPPER(Java_MyClassNatives_fooJJ_synchronized));
+ ScopedSynchronizedEntryPointOverrides ssepo;
EXPECT_EQ(0, gJava_MyClassNatives_fooJJ_synchronized_calls[gCurrentJni]);
jlong a = 0x1000000020000000ULL;
@@ -1103,6 +1183,7 @@
SetUpForTest(true, "fooSSIOO",
"(ILjava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;",
CURRENT_JNI_WRAPPER(Java_MyClassNatives_fooSSIOO));
+ ScopedSynchronizedEntryPointOverrides ssepo;
EXPECT_EQ(0, gJava_MyClassNatives_fooSSIOO_calls[gCurrentJni]);
jobject result = env_->CallStaticObjectMethod(jklass_, jmethod_, 0, nullptr, nullptr);
@@ -1343,6 +1424,7 @@
void JniCompilerTest::GetSinkPropertiesNativeImpl() {
SetUpForTest(false, "getSinkPropertiesNative", "(Ljava/lang/String;)[Ljava/lang/Object;",
CURRENT_JNI_WRAPPER(Java_MyClassNatives_GetSinkProperties));
+ ScopedSynchronizedEntryPointOverrides ssepo;
EXPECT_EQ(0, gJava_MyClassNatives_GetSinkProperties_calls[gCurrentJni]);
jarray result = down_cast<jarray>(