On 4/5/16 7:48 PM, Brent Christian wrote:
Thanks, Coleen. Coordinating method/function names on "to stack trace
element" is a fine thing. I've done so in the updated webrev, and
also implemented Claes's suggestion.
http://cr.openjdk.java.net/~bchristi/8153123/webrev.01/index.html
Thank you for making this change. It looks good! I've reviewed this.
Coleen
-Brent
On 04/05/2016 11:25 AM, Coleen Phillimore wrote:
A correction below.
On 4/5/16 1:29 PM, Coleen Phillimore wrote:
Also meant to include core-libs-dev in the email.
Thanks,
Coleen
On 4/5/16 1:27 PM, Coleen Phillimore wrote:
Hi, I've reviewed the hotspot changes and some of the jdk changes.
This looks really good.
One comment about the jvm function names:
I think FillInStackTraceElement is too close of a name to
Throwable::fill_in_stack_trace().
-JVM_ENTRY(void, JVM_SetMethodInfo(JNIEnv *env, jobject frame))
+JVM_ENTRY(void, JVM_FillInStackTraceElement(JNIEnv *env, jobject
frame, jobject stack))
JVMWrapper("JVM_SetMethodInfo");
- Handle stackFrame(THREAD, JNIHandles::resolve(frame));
- java_lang_StackFrameInfo::fill_methodInfo(stackFrame, THREAD);
+ Handle stack_frame_info(THREAD, JNIHandles::resolve(frame));
+ Handle stack_trace_element(THREAD, JNIHandles::resolve(stack));
+ java_lang_StackFrameInfo::fill_methodInfo(stack_frame_info,
stack_trace_element, THREAD); JVM_END
And the function is called fill_methodInfo in the javaClasses
function.
I think the JVM and the java_lang_StackFrameInfo function names
should be closer.
I wonder if the name JVM_ToStackFrameElement() and
java_lang_StackFrameInfo::to_stack_frame_element() would be better
and then it'd match the Java name.
I meant JVM_ToStackTraceElement() and
java_lang_StackFrameInfo::to_stack_trace_element(), since it's producing
a StackTraceElement.
thanks,
Coleen
Thanks!
Coleen
On 4/4/16 9:29 PM, Mandy Chung wrote:
On Apr 4, 2016, at 4:45 PM, Brent Christian
<brent.christ...@oracle.com> wrote:
Hi,
I'd like to check in some footprint and code reduction changes to
the java.lang.StackWalker implementation.
Webrev:
http://cr.openjdk.java.net/~bchristi/8153123/webrev.00/
Bug:
https://bugs.openjdk.java.net/browse/JDK-8153123
This looks good to me.
One thing to mention is that this patch is a follow-up work from the
investigation on what it takes to enable Throwable to use
StackWalker (JDK-8141239). The current built-in VM backtrace is very
compact and performant. We have identified and prototypes the
performance improvements if Throwable backtrace is generated using
stack walker. There are some performance gaps that we agree to
defer JDK-8141239 to a future release and improve the footprint
performance and GC throughput concerns when MemberNames are stored
in the throwable backtrace.
Mandy
