Issue 4442 - Cache class and ctors references in JNI_Load/UnLoad

libadblockplus-android/jni/JniWebRequest.cpp

Index: libadblockplus-android/jni/JniWebRequest.cpp
diff --git a/libadblockplus-android/jni/JniWebRequest.cpp b/libadblockplus-android/jni/JniWebRequest.cpp
index 5d46b8f60fd754ee433ad703c856aaa68f12fc2f..57f1bbfb266f6d77635964a14b9f7f57c2d4d486 100644
--- a/libadblockplus-android/jni/JniWebRequest.cpp
+++ b/libadblockplus-android/jni/JniWebRequest.cpp
@@ -17,6 +17,32 @@
 
 #include "JniCallbacks.h"
 #include "Utils.h"
+#include "JniWebRequest.h"
+
+// precached in JNI_OnLoad and released in JNI_OnUnload
+JniGlobalReference<jclass>* headerEntryClass;
+JniGlobalReference<jclass>* serverResponseClass;
+
+void JniWebRequest_OnLoad(JavaVM* vm, JNIEnv* env, void* reserved)
+{
+  headerEntryClass = new JniGlobalReference<jclass>(env, env->FindClass(PKG("HeaderEntry")));
+  serverResponseClass = new JniGlobalReference<jclass>(env, env->FindClass(PKG("ServerResponse")));
+}
+
+void JniWebRequest_OnUnload(JavaVM* vm, JNIEnv* env, void* reserved)
+{
+  if (headerEntryClass)
+  {
+    delete headerEntryClass;
+    headerEntryClass = NULL;
+  }
+
+  if (serverResponseClass)
+  {
+    delete serverResponseClass;
+    serverResponseClass = NULL;
+  }
+}
 
 static jlong JNICALL JniCtor(JNIEnv* env, jclass clazz, jobject callbackObject)
 {
@@ -33,9 +59,7 @@ static void JNICALL JniDtor(JNIEnv* env, jclass clazz, jlong ptr)
 }
 
 JniWebRequest::JniWebRequest(JNIEnv* env, jobject callbackObject)
-  : JniCallbackBase(env, callbackObject), AdblockPlus::WebRequest(),
-    tupleClass(new JniGlobalReference<jclass>(env, env->FindClass(PKG("HeaderEntry")))),
-    serverResponseClass(new JniGlobalReference<jclass>(env, env->FindClass(PKG("ServerResponse"))))
+  : JniCallbackBase(env, callbackObject), AdblockPlus::WebRequest()
 {
 }
 
@@ -56,13 +80,13 @@ AdblockPlus::ServerResponse JniWebRequest::GET(const std::string& url,
   if (method)
   {
     JniLocalReference<jobject> arrayList(*env, NewJniArrayList(*env));

[anton, 2016/09/23 13:05:25]

Consider using LinkedList instead of ArrayList - for adding one-by-one use case
it's more efficient. Also since we know what list class we create, we can find
the method once during searching list class and skip using
"JniGetAddToListMethod" at all 

[diegocarloslima, 2016/11/04 11:34:19]

Yeah, LinkedList are better for insertion while ArraysList are better for
retrieving elements. If we do more insertions that retrieval operations for this
list, I do agree that it makes more sense, but the performance gain is not
really relevant if the list don't get really big.

+    jmethodID addMethod = JniGetAddToListMethod(*env, *arrayList);

[anton, 2016/09/23 13:05:25]

Now searching for the method once

 
     for (AdblockPlus::HeaderList::const_iterator it = requestHeaders.begin(),
         end = requestHeaders.end(); it != end; it++)
     {
-      JniLocalReference<jobject> tuple(*env,
-          NewTuple(*env, it->first, it->second));
-      JniAddObjectToList(*env, *arrayList, *tuple);

[anton, 2016/09/23 13:05:25]

here "add" methods is found headers size times which is extremely bad for
performance!

+      JniLocalReference<jobject> headerEntry(*env, NewHeaderEntry(*env, it->first, it->second));
+      JniAddObjectToList(*env, *arrayList, addMethod, *headerEntry);
     }
 
     JniLocalReference<jobject> response(*env,
@@ -75,13 +99,19 @@ AdblockPlus::ServerResponse JniWebRequest::GET(const std::string& url,
       sResponse.status = JniGetLongField(*env, serverResponseClass->Get(),
           *response, "status");
       sResponse.responseStatus = JniGetIntField(*env,
-          serverResponseClass->Get(), *response, "responseStatus");
+                                                serverResponseClass->Get(),
+                                                *response,
+                                                "responseStatus");
       sResponse.responseText = JniGetStringField(*env,
-          serverResponseClass->Get(), *response, "response");
+                                                 serverResponseClass->Get(),
+                                                 *response,
+                                                 "response");
 
       // map headers
       jobjectArray responseHeadersArray = JniGetStringArrayField(*env,
-        serverResponseClass->Get(), *response, "headers");
+                                                                 serverResponseClass->Get(),
+                                                                 *response,
+                                                                 "headers");
 
       if (responseHeadersArray)
       {
@@ -106,16 +136,16 @@ AdblockPlus::ServerResponse JniWebRequest::GET(const std::string& url,
   return sResponse;
 }
 
-jobject JniWebRequest::NewTuple(JNIEnv* env, const std::string& a,
+jobject JniWebRequest::NewHeaderEntry(JNIEnv* env, const std::string& a,
     const std::string& b) const
 {
-  jmethodID factory = env->GetMethodID(tupleClass->Get(), "<init>",
+  jmethodID factory = env->GetMethodID(headerEntryClass->Get(), "<init>",
       "(Ljava/lang/String;Ljava/lang/String;)V");
 
   JniLocalReference<jstring> strA(env, env->NewStringUTF(a.c_str()));
   JniLocalReference<jstring> strB(env, env->NewStringUTF(b.c_str()));
 
-  return env->NewObject(tupleClass->Get(), factory, *strA, *strB);
+  return env->NewObject(headerEntryClass->Get(), factory, *strA, *strB);
 }
 
 static JNINativeMethod methods[] =