8217442: Optimize native accesses to String.value
authorredestad
Tue, 22 Jan 2019 11:22:44 +0100
changeset 53418 bc2bb4eee477
parent 53417 126c5e7b97b1
child 53419 eac105e3ec13
8217442: Optimize native accesses to String.value Reviewed-by: shade, dholmes
src/hotspot/share/classfile/javaClasses.cpp
src/hotspot/share/classfile/javaClasses.hpp
src/hotspot/share/classfile/javaClasses.inline.hpp
src/hotspot/share/jfr/jni/jfrJavaSupport.cpp
src/hotspot/share/prims/jni.cpp
--- a/src/hotspot/share/classfile/javaClasses.cpp	Tue Jan 22 10:12:05 2019 +0100
+++ b/src/hotspot/share/classfile/javaClasses.cpp	Tue Jan 22 11:22:44 2019 +0100
@@ -421,7 +421,7 @@
   oop          obj    = java_string();
   // Typical usage is to convert all '/' to '.' in string.
   typeArrayOop value  = java_lang_String::value(obj);
-  int          length = java_lang_String::length(obj);
+  int          length = java_lang_String::length(obj, value);
   bool      is_latin1 = java_lang_String::is_latin1(obj);
 
   // First check if any from_char exist
@@ -485,7 +485,7 @@
 
 jchar* java_lang_String::as_unicode_string(oop java_string, int& length, TRAPS) {
   typeArrayOop value  = java_lang_String::value(java_string);
-               length = java_lang_String::length(java_string);
+               length = java_lang_String::length(java_string, value);
   bool      is_latin1 = java_lang_String::is_latin1(java_string);
 
   jchar* result = NEW_RESOURCE_ARRAY_RETURN_NULL(jchar, length);
@@ -506,11 +506,11 @@
 }
 
 unsigned int java_lang_String::hash_code(oop java_string) {
-  int          length = java_lang_String::length(java_string);
+  typeArrayOop value  = java_lang_String::value(java_string);
+  int          length = java_lang_String::length(java_string, value);
   // Zero length string will hash to zero with String.hashCode() function.
   if (length == 0) return 0;
 
-  typeArrayOop value  = java_lang_String::value(java_string);
   bool      is_latin1 = java_lang_String::is_latin1(java_string);
 
   if (is_latin1) {
@@ -522,7 +522,7 @@
 
 char* java_lang_String::as_quoted_ascii(oop java_string) {
   typeArrayOop value  = java_lang_String::value(java_string);
-  int          length = java_lang_String::length(java_string);
+  int          length = java_lang_String::length(java_string, value);
   bool      is_latin1 = java_lang_String::is_latin1(java_string);
 
   if (length == 0) return NULL;
@@ -547,7 +547,7 @@
 
 Symbol* java_lang_String::as_symbol(oop java_string, TRAPS) {
   typeArrayOop value  = java_lang_String::value(java_string);
-  int          length = java_lang_String::length(java_string);
+  int          length = java_lang_String::length(java_string, value);
   bool      is_latin1 = java_lang_String::is_latin1(java_string);
   if (!is_latin1) {
     jchar* base = (length == 0) ? NULL : value->char_at_addr(0);
@@ -564,7 +564,7 @@
 
 Symbol* java_lang_String::as_symbol_or_null(oop java_string) {
   typeArrayOop value  = java_lang_String::value(java_string);
-  int          length = java_lang_String::length(java_string);
+  int          length = java_lang_String::length(java_string, value);
   bool      is_latin1 = java_lang_String::is_latin1(java_string);
   if (!is_latin1) {
     jchar* base = (length == 0) ? NULL : value->char_at_addr(0);
@@ -577,23 +577,28 @@
   }
 }
 
-int java_lang_String::utf8_length(oop java_string) {
-  typeArrayOop value  = java_lang_String::value(java_string);
-  int          length = java_lang_String::length(java_string);
-  bool      is_latin1 = java_lang_String::is_latin1(java_string);
+int java_lang_String::utf8_length(oop java_string, typeArrayOop value) {
+  assert(oopDesc::equals_raw(value, java_lang_String::value(java_string)),
+         "value must be same as java_lang_String::value(java_string)");
+  int length = java_lang_String::length(java_string, value);
   if (length == 0) {
     return 0;
   }
-  if (!is_latin1) {
+  if (!java_lang_String::is_latin1(java_string)) {
     return UNICODE::utf8_length(value->char_at_addr(0), length);
   } else {
     return UNICODE::utf8_length(value->byte_at_addr(0), length);
   }
 }
 
+int java_lang_String::utf8_length(oop java_string) {
+  typeArrayOop value = java_lang_String::value(java_string);
+  return utf8_length(java_string, value);
+}
+
 char* java_lang_String::as_utf8_string(oop java_string) {
   typeArrayOop value  = java_lang_String::value(java_string);
-  int          length = java_lang_String::length(java_string);
+  int          length = java_lang_String::length(java_string, value);
   bool      is_latin1 = java_lang_String::is_latin1(java_string);
   if (!is_latin1) {
     jchar* position = (length == 0) ? NULL : value->char_at_addr(0);
@@ -604,10 +609,11 @@
   }
 }
 
-char* java_lang_String::as_utf8_string(oop java_string, char* buf, int buflen) {
-  typeArrayOop value  = java_lang_String::value(java_string);
-  int          length = java_lang_String::length(java_string);
-  bool      is_latin1 = java_lang_String::is_latin1(java_string);
+char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, char* buf, int buflen) {
+  assert(oopDesc::equals_raw(value, java_lang_String::value(java_string)),
+         "value must be same as java_lang_String::value(java_string)");
+  int     length = java_lang_String::length(java_string, value);
+  bool is_latin1 = java_lang_String::is_latin1(java_string);
   if (!is_latin1) {
     jchar* position = (length == 0) ? NULL : value->char_at_addr(0);
     return UNICODE::as_utf8(position, length, buf, buflen);
@@ -617,11 +623,15 @@
   }
 }
 
+char* java_lang_String::as_utf8_string(oop java_string, char* buf, int buflen) {
+  typeArrayOop value = java_lang_String::value(java_string);
+  return as_utf8_string(java_string, value, buf, buflen);
+}
+
 char* java_lang_String::as_utf8_string(oop java_string, int start, int len) {
   typeArrayOop value  = java_lang_String::value(java_string);
-  int          length = java_lang_String::length(java_string);
-  assert(start + len <= length, "just checking");
   bool      is_latin1 = java_lang_String::is_latin1(java_string);
+  assert(start + len <= java_lang_String::length(java_string), "just checking");
   if (!is_latin1) {
     jchar* position = value->char_at_addr(start);
     return UNICODE::as_utf8(position, len);
@@ -631,11 +641,11 @@
   }
 }
 
-char* java_lang_String::as_utf8_string(oop java_string, int start, int len, char* buf, int buflen) {
-  typeArrayOop value  = java_lang_String::value(java_string);
-  int          length = java_lang_String::length(java_string);
-  assert(start + len <= length, "just checking");
-  bool      is_latin1 = java_lang_String::is_latin1(java_string);
+char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, int start, int len, char* buf, int buflen) {
+  assert(oopDesc::equals_raw(value, java_lang_String::value(java_string)),
+         "value must be same as java_lang_String::value(java_string)");
+  assert(start + len <= java_lang_String::length(java_string), "just checking");
+  bool is_latin1 = java_lang_String::is_latin1(java_string);
   if (!is_latin1) {
     jchar* position = value->char_at_addr(start);
     return UNICODE::as_utf8(position, len, buf, buflen);
@@ -649,7 +659,7 @@
   assert(java_string->klass() == SystemDictionary::String_klass(),
          "must be java_string");
   typeArrayOop value = java_lang_String::value_no_keepalive(java_string);
-  int length = java_lang_String::length(java_string);
+  int length = java_lang_String::length(java_string, value);
   if (length != len) {
     return false;
   }
@@ -676,10 +686,10 @@
   assert(str2->klass() == SystemDictionary::String_klass(),
          "must be java String");
   typeArrayOop value1    = java_lang_String::value_no_keepalive(str1);
-  int          length1   = java_lang_String::length(str1);
+  int          length1   = java_lang_String::length(str1, value1);
   bool         is_latin1 = java_lang_String::is_latin1(str1);
   typeArrayOop value2    = java_lang_String::value_no_keepalive(str2);
-  int          length2   = java_lang_String::length(str2);
+  int          length2   = java_lang_String::length(str2, value2);
   bool         is_latin2 = java_lang_String::is_latin1(str2);
 
   if ((length1 != length2) || (is_latin1 != is_latin2)) {
@@ -707,7 +717,7 @@
     return;
   }
 
-  int length = java_lang_String::length(java_string);
+  int length = java_lang_String::length(java_string, value);
   bool is_latin1 = java_lang_String::is_latin1(java_string);
 
   st->print("\"");
--- a/src/hotspot/share/classfile/javaClasses.hpp	Tue Jan 22 10:12:05 2019 +0100
+++ b/src/hotspot/share/classfile/javaClasses.hpp	Tue Jan 22 11:22:44 2019 +0100
@@ -147,13 +147,16 @@
   static inline unsigned int hash(oop java_string);
   static inline bool is_latin1(oop java_string);
   static inline int length(oop java_string);
+  static inline int length(oop java_string, typeArrayOop string_value);
   static int utf8_length(oop java_string);
+  static int utf8_length(oop java_string, typeArrayOop string_value);
 
   // String converters
   static char*  as_utf8_string(oop java_string);
   static char*  as_utf8_string(oop java_string, char* buf, int buflen);
   static char*  as_utf8_string(oop java_string, int start, int len);
-  static char*  as_utf8_string(oop java_string, int start, int len, char* buf, int buflen);
+  static char*  as_utf8_string(oop java_string, typeArrayOop value, char* buf, int buflen);
+  static char*  as_utf8_string(oop java_string, typeArrayOop value, int start, int len, char* buf, int buflen);
   static char*  as_platform_dependent_str(Handle java_string, TRAPS);
   static jchar* as_unicode_string(oop java_string, int& length, TRAPS);
   // produce an ascii string with all other values quoted using \u####
--- a/src/hotspot/share/classfile/javaClasses.inline.hpp	Tue Jan 22 10:12:05 2019 +0100
+++ b/src/hotspot/share/classfile/javaClasses.inline.hpp	Tue Jan 22 11:22:44 2019 +0100
@@ -71,10 +71,11 @@
   assert(CompactStrings || coder == CODER_UTF16, "Must be UTF16 without CompactStrings");
   return coder == CODER_LATIN1;
 }
-int java_lang_String::length(oop java_string) {
+int java_lang_String::length(oop java_string, typeArrayOop value) {
   assert(initialized, "Must be initialized");
   assert(is_instance(java_string), "must be java_string");
-  typeArrayOop value = java_lang_String::value_no_keepalive(java_string);
+  assert(oopDesc::equals_raw(value, java_lang_String::value(java_string)),
+         "value must be same as java_lang_String::value(java_string)");
   if (value == NULL) {
     return 0;
   }
@@ -85,6 +86,12 @@
   }
   return arr_length;
 }
+int java_lang_String::length(oop java_string) {
+  assert(initialized, "Must be initialized");
+  assert(is_instance(java_string), "must be java_string");
+  typeArrayOop value = java_lang_String::value_no_keepalive(java_string);
+  return length(java_string, value);
+}
 
 bool java_lang_String::is_instance_inlined(oop obj) {
   return obj != NULL && obj->klass() == SystemDictionary::String_klass();
--- a/src/hotspot/share/jfr/jni/jfrJavaSupport.cpp	Tue Jan 22 10:12:05 2019 +0100
+++ b/src/hotspot/share/jfr/jni/jfrJavaSupport.cpp	Tue Jan 22 11:22:44 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -474,15 +474,16 @@
   }
   const char* temp = NULL;
   const oop java_string = resolve_non_null(string);
-  if (java_lang_String::value(java_string) != NULL) {
-    const size_t length = java_lang_String::utf8_length(java_string);
+  const typeArrayOop value = java_lang_String::value(java_string);
+  if (value != NULL) {
+    const size_t length = java_lang_String::utf8_length(java_string, value);
     temp = NEW_RESOURCE_ARRAY_IN_THREAD(t, const char, (length + 1));
     if (temp == NULL) {
        JfrJavaSupport::throw_out_of_memory_error("Unable to allocate thread local native memory", t);
        return NULL;
     }
     assert(temp != NULL, "invariant");
-    java_lang_String::as_utf8_string(java_string, const_cast<char*>(temp), (int) length + 1);
+    java_lang_String::as_utf8_string(java_string, value, const_cast<char*>(temp), (int) length + 1);
   }
   return temp;
 }
--- a/src/hotspot/share/prims/jni.cpp	Tue Jan 22 10:12:05 2019 +0100
+++ b/src/hotspot/share/prims/jni.cpp	Tue Jan 22 11:22:44 2019 +0100
@@ -2428,9 +2428,7 @@
   HOTSPOT_JNI_GETSTRINGLENGTH_ENTRY(env, string);
   jsize ret = 0;
   oop s = JNIHandles::resolve_non_null(string);
-  if (java_lang_String::value(s) != NULL) {
-    ret = java_lang_String::length(s);
-  }
+  ret = java_lang_String::length(s);
  HOTSPOT_JNI_GETSTRINGLENGTH_RETURN(ret);
   return ret;
 JNI_END
@@ -2444,7 +2442,7 @@
   oop s = JNIHandles::resolve_non_null(string);
   typeArrayOop s_value = java_lang_String::value(s);
   if (s_value != NULL) {
-    int s_len = java_lang_String::length(s);
+    int s_len = java_lang_String::length(s, s_value);
     bool is_latin1 = java_lang_String::is_latin1(s);
     buf = NEW_C_HEAP_ARRAY_RETURN_NULL(jchar, s_len + 1, mtInternal);  // add one for zero termination
     /* JNI Specification states return NULL on OOM */
@@ -2504,11 +2502,8 @@
 JNI_ENTRY(jsize, jni_GetStringUTFLength(JNIEnv *env, jstring string))
   JNIWrapper("GetStringUTFLength");
  HOTSPOT_JNI_GETSTRINGUTFLENGTH_ENTRY(env, string);
-  jsize ret = 0;
   oop java_string = JNIHandles::resolve_non_null(string);
-  if (java_lang_String::value(java_string) != NULL) {
-    ret = java_lang_String::utf8_length(java_string);
-  }
+  jsize ret = java_lang_String::utf8_length(java_string);
   HOTSPOT_JNI_GETSTRINGUTFLENGTH_RETURN(ret);
   return ret;
 JNI_END
@@ -2519,12 +2514,13 @@
  HOTSPOT_JNI_GETSTRINGUTFCHARS_ENTRY(env, string, (uintptr_t *) isCopy);
   char* result = NULL;
   oop java_string = JNIHandles::resolve_non_null(string);
-  if (java_lang_String::value(java_string) != NULL) {
-    size_t length = java_lang_String::utf8_length(java_string);
+  typeArrayOop s_value = java_lang_String::value(java_string);
+  if (s_value != NULL) {
+    size_t length = java_lang_String::utf8_length(java_string, s_value);
     /* JNI Specification states return NULL on OOM */
     result = AllocateHeap(length + 1, mtInternal, 0, AllocFailStrategy::RETURN_NULL);
     if (result != NULL) {
-      java_lang_String::as_utf8_string(java_string, result, (int) length + 1);
+      java_lang_String::as_utf8_string(java_string, s_value, result, (int) length + 1);
       if (isCopy != NULL) {
         *isCopy = JNI_TRUE;
       }
@@ -3097,12 +3093,12 @@
  HOTSPOT_JNI_GETSTRINGREGION_ENTRY(env, string, start, len, buf);
   DT_VOID_RETURN_MARK(GetStringRegion);
   oop s = JNIHandles::resolve_non_null(string);
-  int s_len = java_lang_String::length(s);
+  typeArrayOop s_value = java_lang_String::value(s);
+  int s_len = java_lang_String::length(s, s_value);
   if (start < 0 || len < 0 || start > s_len - len) {
     THROW(vmSymbols::java_lang_StringIndexOutOfBoundsException());
   } else {
     if (len > 0) {
-      typeArrayOop s_value = java_lang_String::value(s);
       bool is_latin1 = java_lang_String::is_latin1(s);
       if (!is_latin1) {
         ArrayAccess<>::arraycopy_to_native(s_value, typeArrayOopDesc::element_offset<jchar>(start),
@@ -3124,14 +3120,15 @@
  HOTSPOT_JNI_GETSTRINGUTFREGION_ENTRY(env, string, start, len, buf);
   DT_VOID_RETURN_MARK(GetStringUTFRegion);
   oop s = JNIHandles::resolve_non_null(string);
-  int s_len = java_lang_String::length(s);
+  typeArrayOop s_value = java_lang_String::value(s);
+  int s_len = java_lang_String::length(s, s_value);
   if (start < 0 || len < 0 || start > s_len - len) {
     THROW(vmSymbols::java_lang_StringIndexOutOfBoundsException());
   } else {
     //%note jni_7
     if (len > 0) {
       // Assume the buffer is large enough as the JNI spec. does not require user error checking
-      java_lang_String::as_utf8_string(s, start, len, buf, INT_MAX);
+      java_lang_String::as_utf8_string(s, s_value, start, len, buf, INT_MAX);
       // as_utf8_string null-terminates the result string
     } else {
       // JDK null-terminates the buffer even in len is zero
@@ -3203,7 +3200,7 @@
     ret = (jchar*) s_value->base(T_CHAR);
   } else {
     // Inflate latin1 encoded string to UTF16
-    int s_len = java_lang_String::length(s);
+    int s_len = java_lang_String::length(s, s_value);
     ret = NEW_C_HEAP_ARRAY_RETURN_NULL(jchar, s_len + 1, mtInternal);  // add one for zero termination
     /* JNI Specification states return NULL on OOM */
     if (ret != NULL) {