8216302: StackTraceElement::fill_in can use cached Class.name
authorshade
Wed, 09 Jan 2019 20:28:16 +0100
changeset 53212 bccff579c2ff
parent 53211 eda4c6456efb
child 53213 716c746165b2
8216302: StackTraceElement::fill_in can use cached Class.name Reviewed-by: coleenp, dholmes, mchung
make/hotspot/symbols/symbols-unix
src/hotspot/share/classfile/javaClasses.cpp
src/hotspot/share/classfile/javaClasses.hpp
src/hotspot/share/include/jvm.h
src/hotspot/share/prims/jvm.cpp
src/java.base/share/classes/java/lang/Class.java
src/java.base/share/native/libjava/Class.c
test/hotspot/jtreg/runtime/StackTrace/StackTraceClassCache.java
--- a/make/hotspot/symbols/symbols-unix	Wed Jan 09 10:59:37 2019 -0800
+++ b/make/hotspot/symbols/symbols-unix	Wed Jan 09 20:28:16 2019 +0100
@@ -81,7 +81,6 @@
 JVM_GetClassInterfaces
 JVM_GetClassMethodsCount
 JVM_GetClassModifiers
-JVM_GetClassName
 JVM_GetClassNameUTF
 JVM_GetClassSignature
 JVM_GetClassSigners
@@ -133,6 +132,7 @@
 JVM_HasReferencePendingList
 JVM_HoldsLock
 JVM_IHashCode
+JVM_InitClassName
 JVM_InitStackTraceElement
 JVM_InitStackTraceElementArray
 JVM_InitializeFromArchive
--- a/src/hotspot/share/classfile/javaClasses.cpp	Wed Jan 09 10:59:37 2019 -0800
+++ b/src/hotspot/share/classfile/javaClasses.cpp	Wed Jan 09 20:28:16 2019 +0100
@@ -1344,6 +1344,16 @@
   java_class->obj_field_put(_module_offset, module);
 }
 
+oop java_lang_Class::name(Handle java_class, TRAPS) {
+  assert(_name_offset != 0, "must be set");
+  oop o = java_class->obj_field(_name_offset);
+  if (o == NULL) {
+    o = StringTable::intern(java_lang_Class::as_external_name(java_class()), THREAD);
+    java_class->obj_field_put(_name_offset, o);
+  }
+  return o;
+}
+
 oop java_lang_Class::create_basic_type_mirror(const char* basic_type_name, BasicType type, TRAPS) {
   // This should be improved by adding a field at the Java level or by
   // introducing a new VM klass (see comment in ClassFileParser)
@@ -1504,7 +1514,8 @@
   macro(classRedefinedCount_offset, k, "classRedefinedCount", int_signature,         false) ; \
   macro(_class_loader_offset,       k, "classLoader",         classloader_signature, false); \
   macro(_component_mirror_offset,   k, "componentType",       class_signature,       false); \
-  macro(_module_offset,             k, "module",              module_signature,      false)
+  macro(_module_offset,             k, "module",              module_signature,      false); \
+  macro(_name_offset,               k, "name",                string_signature,      false); \
 
 void java_lang_Class::compute_offsets() {
   if (offsets_computed) {
@@ -2550,12 +2561,14 @@
                                           int version, int bci, Symbol* name, TRAPS) {
   assert(element->is_a(SystemDictionary::StackTraceElement_klass()), "sanity check");
 
+  ResourceMark rm(THREAD);
+  HandleMark hm(THREAD);
+
   // Fill in class name
-  ResourceMark rm(THREAD);
-  const char* str = holder->external_name();
-  oop classname = StringTable::intern(str, CHECK);
+  Handle java_class(THREAD, holder->java_mirror());
+  oop classname = java_lang_Class::name(java_class, CHECK);
   java_lang_StackTraceElement::set_declaringClass(element(), classname);
-  java_lang_StackTraceElement::set_declaringClassObject(element(), holder->java_mirror());
+  java_lang_StackTraceElement::set_declaringClassObject(element(), java_class());
 
   oop loader = holder->class_loader();
   if (loader != NULL) {
@@ -3966,6 +3979,7 @@
 int java_lang_Class::_component_mirror_offset;
 int java_lang_Class::_init_lock_offset;
 int java_lang_Class::_signers_offset;
+int java_lang_Class::_name_offset;
 GrowableArray<Klass*>* java_lang_Class::_fixup_mirror_list = NULL;
 GrowableArray<Klass*>* java_lang_Class::_fixup_module_field_list = NULL;
 int java_lang_Throwable::backtrace_offset;
--- a/src/hotspot/share/classfile/javaClasses.hpp	Wed Jan 09 10:59:37 2019 -0800
+++ b/src/hotspot/share/classfile/javaClasses.hpp	Wed Jan 09 20:28:16 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -240,6 +240,7 @@
   static int _class_loader_offset;
   static int _module_offset;
   static int _component_mirror_offset;
+  static int _name_offset;
 
   static bool offsets_computed;
   static int classRedefinedCount_offset;
@@ -310,6 +311,8 @@
   static void set_module(oop java_class, oop module);
   static oop module(oop java_class);
 
+  static oop name(Handle java_class, TRAPS);
+
   static int oop_size(oop java_class);
   static int oop_size_raw(oop java_class);
   static void set_oop_size(HeapWord* java_class, int size);
--- a/src/hotspot/share/include/jvm.h	Wed Jan 09 10:59:37 2019 -0800
+++ b/src/hotspot/share/include/jvm.h	Wed Jan 09 20:28:16 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -63,7 +63,7 @@
  *    class.
  */
 
-#define JVM_INTERFACE_VERSION 5
+#define JVM_INTERFACE_VERSION 6
 
 JNIEXPORT jint JNICALL
 JVM_GetInterfaceVersion(void);
@@ -450,7 +450,7 @@
  */
 
 JNIEXPORT jstring JNICALL
-JVM_GetClassName(JNIEnv *env, jclass cls);
+JVM_InitClassName(JNIEnv *env, jclass cls);
 
 JNIEXPORT jobjectArray JNICALL
 JVM_GetClassInterfaces(JNIEnv *env, jclass cls);
--- a/src/hotspot/share/prims/jvm.cpp	Wed Jan 09 10:59:37 2019 -0800
+++ b/src/hotspot/share/prims/jvm.cpp	Wed Jan 09 20:28:16 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -1059,21 +1059,14 @@
 
 // Reflection support //////////////////////////////////////////////////////////////////////////////
 
-JVM_ENTRY(jstring, JVM_GetClassName(JNIEnv *env, jclass cls))
+JVM_ENTRY(jstring, JVM_InitClassName(JNIEnv *env, jclass cls))
   assert (cls != NULL, "illegal class");
-  JVMWrapper("JVM_GetClassName");
+  JVMWrapper("JVM_InitClassName");
   JvmtiVMObjectAllocEventCollector oam;
   ResourceMark rm(THREAD);
-  const char* name;
-  if (java_lang_Class::is_primitive(JNIHandles::resolve(cls))) {
-    name = type2name(java_lang_Class::primitive_type(JNIHandles::resolve(cls)));
-  } else {
-    // Consider caching interned string in Klass
-    Klass* k = java_lang_Class::as_Klass(JNIHandles::resolve(cls));
-    assert(k->is_klass(), "just checking");
-    name = k->external_name();
-  }
-  oop result = StringTable::intern((char*) name, CHECK_NULL);
+  HandleMark hm(THREAD);
+  Handle java_class(THREAD, JNIHandles::resolve(cls));
+  oop result = java_lang_Class::name(java_class, CHECK_NULL);
   return (jstring) JNIHandles::make_local(env, result);
 JVM_END
 
--- a/src/java.base/share/classes/java/lang/Class.java	Wed Jan 09 10:59:37 2019 -0800
+++ b/src/java.base/share/classes/java/lang/Class.java	Wed Jan 09 20:28:16 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1994, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 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
@@ -795,14 +795,13 @@
      */
     public String getName() {
         String name = this.name;
-        if (name == null)
-            this.name = name = getName0();
-        return name;
+        return name != null ? name : initClassName();
     }
 
-    // cache the name to reduce the number of calls into the VM
+    // Cache the name to reduce the number of calls into the VM.
+    // This field would be set by VM itself during initClassName call.
     private transient String name;
-    private native String getName0();
+    private native String initClassName();
 
     /**
      * Returns the class loader for the class.  Some implementations may use
--- a/src/java.base/share/native/libjava/Class.c	Wed Jan 09 10:59:37 2019 -0800
+++ b/src/java.base/share/native/libjava/Class.c	Wed Jan 09 20:28:16 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1994, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 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
@@ -52,7 +52,7 @@
 #define BA  "[B"
 
 static JNINativeMethod methods[] = {
-    {"getName0",         "()" STR,          (void *)&JVM_GetClassName},
+    {"initClassName",    "()" STR,          (void *)&JVM_InitClassName},
     {"getSuperclass",    "()" CLS,          NULL},
     {"getInterfaces0",   "()[" CLS,         (void *)&JVM_GetClassInterfaces},
     {"isInterface",      "()Z",             (void *)&JVM_IsInterface},
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/StackTrace/StackTraceClassCache.java	Wed Jan 09 20:28:16 2019 +0100
@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2019, Red Hat, Inc. 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8216302
+ * @summary Check that stack trace contains proper strings even with class caching
+ * @modules java.base/java.lang:open
+ * @compile StackTraceClassCache.java
+ * @run main StackTraceClassCache
+ */
+
+import java.lang.reflect.*;
+
+public class StackTraceClassCache  {
+    public static void main(String... args) throws Exception {
+        Outer.Inner o = new Outer().new Inner();
+        Class cl = o.getClass();
+
+        // Check out of the box class name
+        try {
+            o.work();
+        } catch (Exception e) {
+            checkException(e, 42);
+        }
+
+        // Clear and populate class caches via getName
+        clearNameCache(cl);
+        cl.getName();
+        try {
+            o.work();
+        } catch (Exception e) {
+            checkException(e, 51);
+        }
+
+        // Clear and populate class caches via stack trace
+        clearNameCache(cl);
+        try {
+            o.work();
+        } catch (Exception e) {
+            checkException(e, 59);
+        }
+    }
+
+    static void checkException(Exception e, int line) throws Exception {
+        StackTraceElement[] fs = e.getStackTrace();
+
+        if (fs.length < 2) {
+            throw new IllegalStateException("Exception should have at least two frames", e);
+        }
+
+        assertCorrect("StackTraceClassCache$Outer$Inner.work(StackTraceClassCache.java:95)", fs[0].toString(), e);
+        assertCorrect("StackTraceClassCache.main(StackTraceClassCache.java:" + line + ")",   fs[1].toString(), e);
+    }
+
+    static void assertCorrect(String expected, String actual, Exception e) throws Exception {
+        if (!expected.equals(actual)) {
+            throw new IllegalStateException("Expected: " + expected + "; Actual: " + actual, e);
+        }
+    }
+
+    static void clearNameCache(Class cl) {
+        try {
+            Field f = Class.class.getDeclaredField("name");
+            f.setAccessible(true);
+            f.set(cl, null);
+        } catch (Exception e) {
+            throw new IllegalStateException(e);
+        }
+    }
+
+    static class Outer {
+       class Inner {
+           void work() throws Exception {
+               throw new Exception("Sample exception");
+           }
+       }
+    }
+
+}