Hi Mandy,
On 09/03/2017 04:52 AM, mandy chung wrote:
On 9/2/17 2:57 AM, Peter Levart wrote:
Hi Mandy,
The API looks fine to me.
Note that there is an opportunity for a follow-up optimization of the
StackFrameInfo::getDescriptor() case. When MemberName's 'type' field
is filled by native expandFromVM() it is usually filled with the
descriptor string. MemberName::getMethodType() then parses this
string into a MemberType, resolving all the types. So when
StackFrameInfo::getDescriptor() is called, the descriptor string is
1st parsed into MethodType and then formatted back to the descriptor.
By introducing new method into package-private MemberName - say
getMethodDescriptorString(), this intermediate conversion could often
be avoided (for example, if getMethodDescriptorString() was called
before getMethodType() on an instance of MethodName).
Good suggestion.
Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8186050/webrev.02/
That's what I had in mind, yes.
Looking at the method names, I have a second thought about the too
general StackFrame::getDescriptor(). Not looking at the javadocs, one
could ask: "what is a descriptor of a stack frame?". I don't know, maybe
getMethodDescriptor() would be more to the point or even
getMethodTypeDescriptor() (since it is a descriptor of method parameter
and return types, not containing method name). What do you and others think?
Although it is not expected for StackFrame interface to be implemented
by custom classes, it is a public interface. I have seen 3rd party code
implementing JDK interface that was not meant to be implemented by
custom classes just because the interface seemed appropriate. To keep
binary compatibility with such potential implementations, those two new
methods could be default methods throwing UOE.
nit: while you are at it, you could remove the redundant "static"
modifier from the StackWalker.StackFrame interface declaration.
Regards, Peter
Thanks
Mandy
Regards, Peter
On 09/01/2017 07:39 AM, mandy chung wrote:
Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8186050/webrev.01/index.html
This introduces two new methods, StackFrame::getMethodType and
StackFrame::getDescriptor.
Mandy
On 8/30/17 12:25 AM, Remi Forax wrote:
Hi Mandy,
thanks for taking care of this.
In my opinion, we should provide both getMethodType() and
getDescriptor(),
getDescriptor() is handy for logging (finding the right overload
when line numbers are not present) and getMethodType() is the one
you whant if you want to inspect the runtime view of the stack
frames (and by example interact with java.lang.invoke). For me,
it's the same reason that give us getDeclaringClass() and
getClassName() in the current API.
So getDescriptor() can be called with no restriction but
getMethodType() requires RETAIN_CLASS_REFERENCE.
regards,
Rémi
----- Mail original -----
De: "mandy chung" <mandy.ch...@oracle.com>
À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
Envoyé: Mardi 29 Août 2017 00:57:28
Objet: Review Request JDK-8186050: StackFrame should provide the
method signature
Method signature is missing in the StackFrame API. This proposes
to add
StackFrame::getMethodDescriptor method to return the method
descriptor
in a stack frame.
Webrev at:
http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8186050/webrev.00/index.html
There are a couple options how to present the method signature in the
API level:
1. Class<?>[] getParameterTypes() and Class<?> getReturnTypes()
similiar
to what java.lang.reflect.Method has.
2. java.lang.invoke.MethodType
3. a String representation (i) comma-separated list of the method's
formal parameter types (ii) bytecode method descriptor as
specified in JVMS
Returning Class<?> instance should require to add a new StackWalker
option to access to the parameter types and return type for option #1
and #2. StackFrame::getDeclaringClass requires the stack walker to
have
the RETAIN_CLASS_REFERENCE capability.
Option #2 returning MethodType is handy while java.lang would
reference
a type in java.lang.invoke.
Option #3 requires the caller to parse the return string and call
Class.forName to get the Class<?> instance. OTOH
MethodType::fromMethodDescriptorString method that returns MethodType
from a bytecode method descriptor string.
Method signature is for information for typical cases. Getting
Class<?>
for the parameter types and return type would be a niche case. I
think
returning the method descriptor string is a good option - keep the
API
simple and can use MethodType::fromMethodDescriptorString to get back
the types if needed.
thanks
Mandy
