Modified functions to return pointers where appropriate

GetStringChars will return a pointer to the underlying char array iff
it is not movable. Otherwise, it will return a copy of the char array.
For consistency, the null terminating character has been removed as the
specification for a jchar strings are not null terminated:

http://developer.android.com/training/articles/perf-jni.html

GetStringCritical will now always return a pointer to the char array.
The char array is pinned and moving gc is disabled until the pointer is
released.

Change-Id: I19c8cbaecc1f3f723d80acec074fb8c5e2d489c3

2 files changed, 58 insertions, 18 deletions

diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index 9ca3c85106..19ee1ffe8c 100644
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -1933,32 +1933,66 @@ class JNI {
     mirror::String* s = soa.Decode<mirror::String*>(java_string);
     mirror::CharArray* chars = s->GetCharArray();
     PinPrimitiveArray(soa, chars);
-    if (is_copy != nullptr) {
-      *is_copy = JNI_TRUE;
-    }
-    int32_t char_count = s->GetLength();
-    int32_t offset = s->GetOffset();
-    jchar* bytes = new jchar[char_count + 1];
-    for (int32_t i = 0; i < char_count; i++) {
-      bytes[i] = chars->Get(i + offset);
+    gc::Heap* heap = Runtime::Current()->GetHeap();
+    if (heap->IsMovableObject(chars)) {
+      if (is_copy != nullptr) {
+        *is_copy = JNI_TRUE;
+      }
+      int32_t char_count = s->GetLength();
+      int32_t offset = s->GetOffset();
+      jchar* bytes = new jchar[char_count];
+      for (int32_t i = 0; i < char_count; i++) {
+        bytes[i] = chars->Get(i + offset);
+      }
+      return bytes;
+    } else {
+      if (is_copy != nullptr) {
+        *is_copy = JNI_FALSE;
+      }
+      return static_cast<jchar*>(chars->GetData() + s->GetOffset());
     }
-    bytes[char_count] = '\0';
-    return bytes;
   }
 
   static void ReleaseStringChars(JNIEnv* env, jstring java_string, const jchar* chars) {
     CHECK_NON_NULL_ARGUMENT_RETURN_VOID(java_string);
-    delete[] chars;
     ScopedObjectAccess soa(env);
-    UnpinPrimitiveArray(soa, soa.Decode<mirror::String*>(java_string)->GetCharArray());
+    mirror::String* s = soa.Decode<mirror::String*>(java_string);
+    mirror::CharArray* s_chars = s->GetCharArray();
+    if (chars != (s_chars->GetData() + s->GetOffset())) {
+      delete[] chars;
+    }
+    UnpinPrimitiveArray(soa, s->GetCharArray());
   }
 
   static const jchar* GetStringCritical(JNIEnv* env, jstring java_string, jboolean* is_copy) {
-    return GetStringChars(env, java_string, is_copy);
+    CHECK_NON_NULL_ARGUMENT(java_string);
+    ScopedObjectAccess soa(env);
+    mirror::String* s = soa.Decode<mirror::String*>(java_string);
+    mirror::CharArray* chars = s->GetCharArray();
+    int32_t offset = s->GetOffset();
+    PinPrimitiveArray(soa, chars);
+    gc::Heap* heap = Runtime::Current()->GetHeap();
+    if (heap->IsMovableObject(chars)) {
+      StackHandleScope<1> hs(soa.Self());
+      HandleWrapper<mirror::CharArray> h(hs.NewHandleWrapper(&chars));
+      heap->IncrementDisableMovingGC(soa.Self());
+    }
+    if (is_copy != nullptr) {
+      *is_copy = JNI_FALSE;
+    }
+    return static_cast<jchar*>(chars->GetData() + offset);
   }
 
   static void ReleaseStringCritical(JNIEnv* env, jstring java_string, const jchar* chars) {
-    return ReleaseStringChars(env, java_string, chars);
+    CHECK_NON_NULL_ARGUMENT_RETURN_VOID(java_string);
+    ScopedObjectAccess soa(env);
+    UnpinPrimitiveArray(soa, soa.Decode<mirror::String*>(java_string)->GetCharArray());
+    gc::Heap* heap = Runtime::Current()->GetHeap();
+    mirror::String* s = soa.Decode<mirror::String*>(java_string);
+    mirror::CharArray* s_chars = s->GetCharArray();
+    if (heap->IsMovableObject(s_chars)) {
+      heap->DecrementDisableMovingGC(soa.Self());
+    }
   }
 
   static const char* GetStringUTFChars(JNIEnv* env, jstring java_string, jboolean* is_copy) {
diff --git a/runtime/jni_internal_test.cc b/runtime/jni_internal_test.cc
index 342982708a..f182e950bd 100644
--- a/runtime/jni_internal_test.cc
+++ b/runtime/jni_internal_test.cc
@@ -18,6 +18,7 @@
 
 #include "common_compiler_test.h"
 #include "mirror/art_method-inl.h"
+#include "mirror/string-inl.h"
 #include "ScopedLocalRef.h"
 
 namespace art {
@@ -1071,6 +1072,8 @@ TEST_F(JniInternalTest, GetStringUTFChars_ReleaseStringUTFChars) {
 
 TEST_F(JniInternalTest, GetStringChars_ReleaseStringChars) {
   jstring s = env_->NewStringUTF("hello");
+  ScopedObjectAccess soa(env_);
+  mirror::String* s_m = soa.Decode<mirror::String*>(s);
   ASSERT_TRUE(s != nullptr);
 
   jchar expected[] = { 'h', 'e', 'l', 'l', 'o' };
@@ -1084,7 +1087,11 @@ TEST_F(JniInternalTest, GetStringChars_ReleaseStringChars) {
 
   jboolean is_copy = JNI_FALSE;
   chars = env_->GetStringChars(s, &is_copy);
-  EXPECT_EQ(JNI_TRUE, is_copy);
+  if (Runtime::Current()->GetHeap()->IsMovableObject(s_m->GetCharArray())) {
+    EXPECT_EQ(JNI_TRUE, is_copy);
+  } else {
+    EXPECT_EQ(JNI_FALSE, is_copy);
+  }
   EXPECT_EQ(expected[0], chars[0]);
   EXPECT_EQ(expected[1], chars[1]);
   EXPECT_EQ(expected[2], chars[2]);
@@ -1106,10 +1113,9 @@ TEST_F(JniInternalTest, GetStringCritical_ReleaseStringCritical) {
   EXPECT_EQ(expected[4], chars[4]);
   env_->ReleaseStringCritical(s, chars);
 
-  jboolean is_copy = JNI_FALSE;
+  jboolean is_copy = JNI_TRUE;
   chars = env_->GetStringCritical(s, &is_copy);
-  // TODO: Fix GetStringCritical to use the same mechanism as GetPrimitiveArrayElementsCritical.
-  EXPECT_EQ(JNI_TRUE, is_copy);
+  EXPECT_EQ(JNI_FALSE, is_copy);
   EXPECT_EQ(expected[0], chars[0]);
   EXPECT_EQ(expected[1], chars[1]);
   EXPECT_EQ(expected[2], chars[2]);