JNI: Optimize string handling

A lot of code passes strings through JNI calls, and we always ended up
constructing a temporary QJniObject just to convert it to a QString, or
to get the jstring out of it. This is expensive as QJniObject allocates
a private object and maintains a reference count.

For code paths that have a jstring, or only want a jstring, and already
have a JNIEnv pointer, we can shortcut this and use JNI APIs directly
rather than going through the QJniObject detour. This was already done
when accessing elements of a QJniArray<QString>.

Add internal helper methods to the QtJniTypes::Details namespace that
convert between jstring and QString directly. Use that where we can,
including the QJniObject::to/fromString implementations.

Since the new helper for converting from a QString already returns a
local reference, split the LocalFrame::newLocalRef into a helper that
pushes the local frame, and use that to explicitly do so in the case
of QString conversion. This also removes the last use of the
QJniObject overload of LocalFrame::newLocalRef, so remove that.

Change-Id: Ib3a261efd0dcca86a1a99c2a054d50e932c3fe71
Reviewed-by: Soheil Armin <soheil.armin@qt.io>
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>

src/corelib/kernel/qjniarray.h

@@ -735,9 +735,7 @@ public:
         } else if constexpr (std::is_same_v<QString, T>) {
             jstring string = static_cast<jstring>(env->GetObjectArrayElement(arrayObject(), i));
             if (string) {
-                const auto length = env->GetStringLength(string);
+                QString res = QtJniTypes::Detail::toQString(string, env);
-                QString res(length, Qt::Uninitialized);
-                env->GetStringRegion(string, 0, length, reinterpret_cast<jchar *>(res.data()));
                 env->DeleteLocalRef(string);
                 return res;
             } else {
@@ -817,6 +815,8 @@ public:
             for (auto element : *this) {
                 if constexpr (std::is_same_v<decltype(element), QString>)
                     container.emplace_back(element);
+                else if constexpr (std::is_same_v<decltype(element), jstring>)
+                    container.emplace_back(QtJniTypes::Detail::toQString(element, env));
                 else
                     container.emplace_back(QJniObject(element).toString());
             }

src/corelib/kernel/qjniobject.cpp

@@ -1325,8 +1325,7 @@ QJniObject QJniObject::getObjectField(const char *fieldName, const char *signatu
 QJniObject QJniObject::fromString(const QString &string)
 {
     QJniEnvironment env;
-    jstring stringRef = env->NewString(reinterpret_cast<const jchar*>(string.constData()),
+    jstring stringRef = QtJniTypes::Detail::fromQString(string, env.jniEnv());
-                                                                      string.length());
     QJniObject stringObject = getCleanJniObject(stringRef, env.jniEnv());
     stringObject.d->m_className = "java/lang/String";
     return stringObject;
@@ -1352,10 +1351,7 @@ QString QJniObject::toString() const
         return QString();
 
     QJniObject string = callObjectMethod<jstring>("toString");
-    const int strLength = string.jniEnv()->GetStringLength(string.object<jstring>());
+    return QtJniTypes::Detail::toQString(string.object<jstring>(), string.jniEnv());
-    QString res(strLength, Qt::Uninitialized);
-    string.jniEnv()->GetStringRegion(string.object<jstring>(), 0, strLength, reinterpret_cast<jchar *>(res.data()));
-    return res;
 }
 
 /*!

src/corelib/kernel/qjniobject.h

@@ -31,21 +31,21 @@ class Q_CORE_EXPORT QJniObject
             if (hasFrame)
                 env->PopLocalFrame(nullptr);
         }
+        bool ensureFrame()
+        {
+            if (!hasFrame)
+                hasFrame = jniEnv()->PushLocalFrame(sizeof...(Args)) == 0;
+            return hasFrame;
+        }
         template <typename T>
         auto newLocalRef(jobject object)
         {
-            if (!hasFrame) {
+            if (!ensureFrame()) {
-                if (jniEnv()->PushLocalFrame(sizeof...(Args)) < 0)
+                // if the JVM is out of memory, avoid making matters worse
-                    return T{}; // JVM is out of memory, avoid making matters worse
+                return T{};
-                hasFrame = true;
             }
             return static_cast<T>(jniEnv()->NewLocalRef(object));
         }
-        template <typename T>
-        auto newLocalRef(const QJniObject &object)
-        {
-            return newLocalRef<T>(object.template object<T>());
-        }
         JNIEnv *jniEnv() const
         {
             if (!env)
@@ -870,7 +870,9 @@ auto QJniObject::LocalFrame<Args...>::convertToJni(T &&value)
 {
     using Type = q20::remove_cvref_t<T>;
     if constexpr (std::is_same_v<Type, QString>) {
-        return newLocalRef<jstring>(QJniObject::fromString(value));
+        if (ensureFrame()) // fromQString already returns a local reference
+            return QtJniTypes::Detail::fromQString(value, jniEnv());
+        return jstring{};
     } else if constexpr (QtJniTypes::IsJniArray<Type>::value) {
         return value.arrayObject();
     } else if constexpr (QtJniTypes::detail::isCompatibleSourceContainer<T>) {

src/corelib/kernel/qjnitypes.h

@@ -163,7 +163,7 @@ struct JNITypeForArgImpl<QString>
 
     static QString fromVarArg(Type t)
     {
-        return QJniObject(t).toString();
+        return QtJniTypes::Detail::toQString(t, QJniEnvironment::getJniEnv());
     }
 };
 

src/corelib/kernel/qjnitypes_impl.h

@@ -4,7 +4,7 @@
 #ifndef QJNITYPES_IMPL_H
 #define QJNITYPES_IMPL_H
 
-#include <QtCore/qglobal.h>
+#include <QtCore/qstring.h>
 #include <QtCore/q20type_traits.h>
 
 #if defined(Q_QDOC) || defined(Q_OS_ANDROID)
@@ -15,6 +15,22 @@ QT_BEGIN_NAMESPACE
 namespace QtJniTypes
 {
 
+namespace Detail
+{
+static inline jstring fromQString(const QString &string, JNIEnv *env)
+{
+    return env->NewString(reinterpret_cast<const jchar*>(string.constData()), string.length());
+}
+
+static inline QString toQString(jstring string, JNIEnv *env)
+{
+    const jsize length = env->GetStringLength(string);
+    QString res(length, Qt::Uninitialized);
+    env->GetStringRegion(string, 0, length, reinterpret_cast<jchar *>(res.data()));
+    return res;
+}
+} // namespace Detail
+
 // a constexpr type for string literals of any character width, aware of the length
 // of the string.
 template<size_t N_WITH_NULL, typename BaseType = char>