8216302: StackTraceElement::fill_in can use cached Class.name
Reviewed-by: coleenp, dholmes, mchung
--- 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");
+ }
+ }
+ }
+
+}