> On Apr 28, 2016, at 11:37 AM, Brent Christian <brent.christ...@oracle.com> > wrote: > > Hi, Mandy > > It looks good to me. A few minor things: > > StackFrameInfo.java: > > Should getByteCodeIndex() be final ? >
It’s a package-private class and so no subclass outside this package anyway. So it doesn’t really matter. I didn’t catch the inconsistency there - some methods in this class are final and some are not. I may clean them up. > > StackWalker.java, line 136: > * change ';' to ‘,' > ok. > > JavaLangInvokeAccess.java, line 40: > > For the isNative() docs, I would prefer "Returns true if..." to "Tests if..." > > Both conventions are used. I can change that. > Some test code for getByteCodeIndex() would be good - sounds like you plan to > add that. yes. Will send out for review next. thanks for the review. Mandy > > Thanks, > -Brent > On 04/27/2016 11:31 AM, Mandy Chung wrote: >> Updated webrev: >> >> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.01/index.html >> >> I added a new StackFrame::getByteCodeIndex method to return bci and >> updatedgetFileName and getLineNumber to have the same returned type as in >> StackTraceElement. From usage perspective, the caller has to prepare and >> handle the information is unavailable and I think it would typically log >> some token to represent the unavailable case and might well use null and >> negative value. This patch would save the creation of short-lived Optional >> object that might help logging filename/linenumber case. >> >> I’m working on a new test to verify bci and line number to be included in >> the next revision. >> >> Mandy >> >>> On Apr 11, 2016, at 2:22 PM, Mandy Chung <mandy.ch...@oracle.com> wrote: >>> >>> Webrev at: >>> >>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.00/index.html >>> >>> StackFrame::getFileName and StackFrame::getLineNumber are originally >>> proposed with the view of any stack walking code can migrate to the >>> StackWalker API without the use of StackTraceElement. >>> >>> File name and line number are useful for debugging and troubleshooting >>> purpose. It has additional overhead to map from a method and BCI to look up >>> the file name and line number. >>> >>> StackFrame::toStackTraceElement method returns StackTraceElement that >>> includes the file name and line number. There is no particular benefit to >>> duplicate getFileName and getLineNumber methods in StackFrame. It is >>> equivalently convenient to call >>> StackFrame.toStackTraceElement().getFileName() (or getLineNumber). >>> >>> This patch proposes to remove StackFrame::getFileName and >>> StackFrame::getLineNumber methods since such information can be obtained >>> from StackFrame.toStackTraceElement(). >>> >>> Mandy >> >
