Skip to content

perf(android): cache the thread's JNIEnv instead of calling GetEnv per JEnv #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ammarahm-ed wants to merge 1 commit into from
Open

perf(android): cache the thread's JNIEnv instead of calling GetEnv per JEnv #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions NativeScript/ffi/jni/jsi/jni/JEnv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
using namespace tns;
using namespace std;

JEnv::JEnv()
: m_env(nullptr) {
JNIEnv *JEnv::GetEnvSlow() {
JNIEnv *env = nullptr;
jint ret = s_jvm->GetEnv(reinterpret_cast<void **>(&env), JNI_VERSION_1_6);

Expand All @@ -18,22 +17,17 @@ JEnv::JEnv()
assert(env != nullptr);
}

m_env = env;
return env;
}

JEnv::JEnv(JNIEnv *jniEnv) {
jint ret = s_jvm->GetEnv(reinterpret_cast<void **>(&jniEnv), JNI_VERSION_1_6);

if ((ret != JNI_OK) || (jniEnv == nullptr)) {
ret = s_jvm->AttachCurrentThread(&jniEnv, nullptr);
assert(ret == JNI_OK);
assert(jniEnv != nullptr);
}

m_env = jniEnv;
void JEnv::PinThreadEnv(JNIEnv *env) {
s_threadEnv = env;
s_threadEnvPinned = true;
}

JEnv::~JEnv() {
void JEnv::UnpinThreadEnv() {
s_threadEnv = nullptr;
s_threadEnvPinned = false;
}

JEnv::operator JNIEnv* () const {
Expand Down Expand Up @@ -861,6 +855,8 @@ void JEnv::CheckForJavaException() {
}

JavaVM *JEnv::s_jvm = nullptr;
thread_local JNIEnv *JEnv::s_threadEnv = nullptr;
thread_local bool JEnv::s_threadEnvPinned = false;
robin_hood::unordered_map<string, jclass> JEnv::s_classCache;
robin_hood::unordered_map<string, jthrowable> JEnv::s_missingClasses;
jclass JEnv::RUNTIME_CLASS = nullptr;
Expand Down
38 changes: 35 additions & 3 deletions NativeScript/ffi/jni/jsi/jni/JEnv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@
namespace tns {
class JEnv {
public:
JEnv();
JEnv() : m_env(s_threadEnv != nullptr ? s_threadEnv : GetEnvSlow()) {}

JEnv(JNIEnv *jniEnv);
JEnv(JNIEnv *jniEnv) : m_env(jniEnv != nullptr ? jniEnv : JEnv().m_env) {}

// Wrap an already-obtained JNIEnv* WITHOUT re-querying the JavaVM
// (no GetEnv). Use only when the pointer is known to belong to the
// current attached thread (e.g. threaded down from a callback prologue).
enum class Adopt { Trusted };
JEnv(JNIEnv *jniEnv, Adopt) : m_env(jniEnv) {}

~JEnv();
~JEnv() = default;

operator JNIEnv *() const;

Expand Down Expand Up @@ -447,11 +447,43 @@ namespace tns {

static void Init(JavaVM *jvm);

static void PinThreadEnv(JNIEnv *env);

static void UnpinThreadEnv();

class EntryScope {
public:
explicit EntryScope(JNIEnv *env) : m_active(!s_threadEnvPinned), m_previous(s_threadEnv) {
if (m_active) {
s_threadEnv = env;
}
}

~EntryScope() {
if (m_active && !s_threadEnvPinned) {
s_threadEnv = m_previous;
}
}

EntryScope(const EntryScope &) = delete;

EntryScope &operator=(const EntryScope &) = delete;

private:
bool m_active;
JNIEnv *m_previous;
};

private:
static JNIEnv *GetEnvSlow();

void CheckForJavaException();

JNIEnv *m_env;

static thread_local JNIEnv *s_threadEnv;
static thread_local bool s_threadEnvPinned;

static JavaVM *s_jvm;

static jclass RUNTIME_CLASS;
Expand Down
24 changes: 10 additions & 14 deletions NativeScript/ffi/jni/napi/jni/JEnv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
using namespace tns;
using namespace std;

JEnv::JEnv()
: m_env(nullptr) {
JNIEnv *JEnv::GetEnvSlow() {
JNIEnv *env = nullptr;
jint ret = s_jvm->GetEnv(reinterpret_cast<void **>(&env), JNI_VERSION_1_6);

Expand All @@ -18,22 +17,17 @@ JEnv::JEnv()
assert(env != nullptr);
}

m_env = env;
return env;
}

JEnv::JEnv(JNIEnv *jniEnv) {
jint ret = s_jvm->GetEnv(reinterpret_cast<void **>(&jniEnv), JNI_VERSION_1_6);

if ((ret != JNI_OK) || (jniEnv == nullptr)) {
ret = s_jvm->AttachCurrentThread(&jniEnv, nullptr);
assert(ret == JNI_OK);
assert(jniEnv != nullptr);
}

m_env = jniEnv;
void JEnv::PinThreadEnv(JNIEnv *env) {
s_threadEnv = env;
s_threadEnvPinned = true;
}

JEnv::~JEnv() {
void JEnv::UnpinThreadEnv() {
s_threadEnv = nullptr;
s_threadEnvPinned = false;
}

JEnv::operator JNIEnv* () const {
Expand Down Expand Up @@ -861,6 +855,8 @@ void JEnv::CheckForJavaException() {
}

JavaVM *JEnv::s_jvm = nullptr;
thread_local JNIEnv *JEnv::s_threadEnv = nullptr;
thread_local bool JEnv::s_threadEnvPinned = false;
robin_hood::unordered_map<string, jclass> JEnv::s_classCache;
robin_hood::unordered_map<string, jthrowable> JEnv::s_missingClasses;
jclass JEnv::RUNTIME_CLASS = nullptr;
Expand Down
38 changes: 35 additions & 3 deletions NativeScript/ffi/jni/napi/jni/JEnv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@
namespace tns {
class JEnv {
public:
JEnv();
JEnv() : m_env(s_threadEnv != nullptr ? s_threadEnv : GetEnvSlow()) {}

JEnv(JNIEnv *jniEnv);
JEnv(JNIEnv *jniEnv) : m_env(jniEnv != nullptr ? jniEnv : JEnv().m_env) {}

// Wrap an already-obtained JNIEnv* WITHOUT re-querying the JavaVM
// (no GetEnv). Use only when the pointer is known to belong to the
// current attached thread (e.g. threaded down from a callback prologue).
enum class Adopt { Trusted };
JEnv(JNIEnv *jniEnv, Adopt) : m_env(jniEnv) {}

~JEnv();
~JEnv() = default;

operator JNIEnv *() const;

Expand Down Expand Up @@ -447,11 +447,43 @@ namespace tns {

static void Init(JavaVM *jvm);

static void PinThreadEnv(JNIEnv *env);

static void UnpinThreadEnv();

class EntryScope {
public:
explicit EntryScope(JNIEnv *env) : m_active(!s_threadEnvPinned), m_previous(s_threadEnv) {
if (m_active) {
s_threadEnv = env;
}
}

~EntryScope() {
if (m_active && !s_threadEnvPinned) {
s_threadEnv = m_previous;
}
}

EntryScope(const EntryScope &) = delete;

EntryScope &operator=(const EntryScope &) = delete;

private:
bool m_active;
JNIEnv *m_previous;
};

private:
static JNIEnv *GetEnvSlow();

void CheckForJavaException();

JNIEnv *m_env;

static thread_local JNIEnv *s_threadEnv;
static thread_local bool s_threadEnvPinned;

static JavaVM *s_jvm;

static jclass RUNTIME_CLASS;
Expand Down
2 changes: 2 additions & 0 deletions NativeScript/runtime/android/jsi/Runtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ Runtime::Init(JNIEnv *_env, jobject obj, int runtimeId, jstring filesPath, jstri
void Runtime::Init(JNIEnv *_env, jstring filesPath, jstring nativeLibsDir,
bool verboseLoggingEnabled, bool isDebuggable, jstring packageName,
jobjectArray args, jstring callingDir, int maxLogcatObjectSize, bool forceLog) {
JEnv::PinThreadEnv(_env);

LogEnabled = verboseLoggingEnabled;
auto filesRoot = ArgConverter::jstringToString(filesPath);
Expand Down Expand Up @@ -576,6 +577,7 @@ void Runtime::DestroyRuntime() {
m_finalizerQueue = nullptr;
}
is_destroying = true;
JEnv::UnpinThreadEnv();
engine::Runtime &rt = engineHost->GetRuntime();
if (m_looperTasks != nullptr) {
m_looperTasks->Terminate();
Expand Down
5 changes: 5 additions & 0 deletions NativeScript/runtime/android/jsi/com_tns_Runtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ extern "C" JNIEXPORT void Java_com_tns_Runtime_setManualInstrumentationModeLegac
// is interop that both flavours need, so only this one is compiled out.
#if !defined(NS_JSI_HOST_RUNTIME)
extern "C" JNIEXPORT void Java_com_tns_Runtime_initNativeScript(JNIEnv* _env, jobject obj, jint runtimeId, jstring filesPath, jstring nativeLibDir, jboolean verboseLoggingEnabled, jboolean isDebuggable, jstring packageName, jobjectArray args, jstring callingDir, jint maxLogcatObjectSize, jboolean forceLog) {
JEnv::EntryScope envScope(_env);
try {
DEBUG_WRITE("NativeScript Initializing!");
Runtime::Init(_env, obj, runtimeId, filesPath, nativeLibDir, verboseLoggingEnabled, isDebuggable, packageName, args, callingDir, maxLogcatObjectSize, forceLog);
Expand Down Expand Up @@ -145,6 +146,7 @@ Runtime* TryGetRuntime(int runtimeId) {
// embedder's bundler owns module loading.
#if !defined(NS_JSI_HOST_RUNTIME)
extern "C" JNIEXPORT void Java_com_tns_Runtime_runModule(JNIEnv* _env, jobject obj, jint runtimeId, jstring scriptFile) {
JEnv::EntryScope envScope(_env);
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
return;
Expand All @@ -171,6 +173,7 @@ extern "C" JNIEXPORT void Java_com_tns_Runtime_runModule(JNIEnv* _env, jobject o

#if !defined(NS_JSI_HOST_RUNTIME)
extern "C" JNIEXPORT jobject Java_com_tns_Runtime_runScript(JNIEnv* _env, jobject obj, jint runtimeId, jstring scriptFile) {
JEnv::EntryScope envScope(_env);
jobject result = nullptr;

auto runtime = TryGetRuntime(runtimeId);
Expand Down Expand Up @@ -198,6 +201,7 @@ extern "C" JNIEXPORT jobject Java_com_tns_Runtime_runScript(JNIEnv* _env, jobjec


extern "C" JNIEXPORT jobject Java_com_tns_Runtime_callJSMethodNative(JNIEnv* _env, jobject obj, jint runtimeId, jint javaObjectID, jclass claz, jstring methodName,jint retType, jboolean isConstructor, jobjectArray packagedArgs) {
JEnv::EntryScope envScope(_env);
jobject result = nullptr;
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) return result;
Expand All @@ -222,6 +226,7 @@ extern "C" JNIEXPORT jobject Java_com_tns_Runtime_callJSMethodNative(JNIEnv* _en


extern "C" JNIEXPORT void Java_com_tns_Runtime_createJSInstanceNative(JNIEnv* _env, jobject obj, jint runtimeId, jobject javaObject, jint javaObjectID, jstring className) {
JEnv::EntryScope envScope(_env);
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) return;

Expand Down
1 change: 1 addition & 0 deletions NativeScript/runtime/android/jsi/workers/WorkerWrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ void WorkerWrapper::BackgroundLooper(std::shared_ptr<WorkerWrapper> self) {

// ART aborts if a native thread exits while still attached. This must be
// the very last JNI-touching action on this thread.
JEnv::UnpinThreadEnv();
jvm->DetachCurrentThread();
}

Expand Down
2 changes: 2 additions & 0 deletions NativeScript/runtime/android/napi/Runtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ napi_value Runtime::GlobalAccessorCallback(napi_env env, napi_callback_info inf
void Runtime::Init(JNIEnv *_env, jstring filesPath, jstring nativeLibsDir,
bool verboseLoggingEnabled, bool isDebuggable, jstring packageName,
jobjectArray args, jstring callingDir, int maxLogcatObjectSize, bool forceLog) {
JEnv::PinThreadEnv(_env);

LogEnabled = verboseLoggingEnabled;
auto filesRoot = ArgConverter::jstringToString(filesPath);
Expand Down Expand Up @@ -508,6 +509,7 @@ void Runtime::DestroyRuntime() {
return;
}
is_destroying = true;
JEnv::UnpinThreadEnv();
delete m_loopTimer;
m_loopTimer = nullptr;
napi_status status;
Expand Down
5 changes: 5 additions & 0 deletions NativeScript/runtime/android/napi/com_tns_Runtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ extern "C" JNIEXPORT void Java_com_tns_Runtime_setManualInstrumentationModeLegac
}

extern "C" JNIEXPORT void Java_com_tns_Runtime_initNativeScript(JNIEnv* _env, jobject obj, jint runtimeId, jstring filesPath, jstring nativeLibDir, jboolean verboseLoggingEnabled, jboolean isDebuggable, jstring packageName, jobjectArray args, jstring callingDir, jint maxLogcatObjectSize, jboolean forceLog) {
JEnv::EntryScope envScope(_env);
try {
DEBUG_WRITE("NativeScript Initializing!");
Runtime::Init(_env, obj, runtimeId, filesPath, nativeLibDir, verboseLoggingEnabled, isDebuggable, packageName, args, callingDir, maxLogcatObjectSize, forceLog);
Expand Down Expand Up @@ -130,6 +131,7 @@ Runtime* TryGetRuntime(int runtimeId) {
}

extern "C" JNIEXPORT void Java_com_tns_Runtime_runModule(JNIEnv* _env, jobject obj, jint runtimeId, jstring scriptFile) {
JEnv::EntryScope envScope(_env);
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
return;
Expand All @@ -153,6 +155,7 @@ extern "C" JNIEXPORT void Java_com_tns_Runtime_runModule(JNIEnv* _env, jobject o
}

extern "C" JNIEXPORT jobject Java_com_tns_Runtime_runScript(JNIEnv* _env, jobject obj, jint runtimeId, jstring scriptFile) {
JEnv::EntryScope envScope(_env);
jobject result = nullptr;

auto runtime = TryGetRuntime(runtimeId);
Expand All @@ -178,6 +181,7 @@ extern "C" JNIEXPORT jobject Java_com_tns_Runtime_runScript(JNIEnv* _env, jobjec
}

extern "C" JNIEXPORT jobject Java_com_tns_Runtime_callJSMethodNative(JNIEnv* _env, jobject obj, jint runtimeId, jint javaObjectID, jclass claz, jstring methodName,jint retType, jboolean isConstructor, jobjectArray packagedArgs) {
JEnv::EntryScope envScope(_env);
jobject result = nullptr;
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) return result;
Expand All @@ -202,6 +206,7 @@ extern "C" JNIEXPORT jobject Java_com_tns_Runtime_callJSMethodNative(JNIEnv* _en


extern "C" JNIEXPORT void Java_com_tns_Runtime_createJSInstanceNative(JNIEnv* _env, jobject obj, jint runtimeId, jobject javaObject, jint javaObjectID, jstring className) {
JEnv::EntryScope envScope(_env);
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) return;

Expand Down
1 change: 1 addition & 0 deletions NativeScript/runtime/android/napi/workers/WorkerWrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ void WorkerWrapper::BackgroundLooper(std::shared_ptr<WorkerWrapper> self) {

// ART aborts if a native thread exits while still attached. This must be
// the very last JNI-touching action on this thread.
JEnv::UnpinThreadEnv();
jvm->DetachCurrentThread();
}

Expand Down
Loading