Code Review for JEP 259: Stack-Walking API (original) (raw)

Mandy Chung mandy.chung at oracle.com
Fri Nov 20 16:39:38 UTC 2015

On Nov 20, 2015, at 1:59 AM, serguei.spitsyn at oracle.com wrote:

The 'int bci' is not used above.

This is weird. Daniel caught that and I took that line out already. http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03-delta/hotspot/src/share/vm/classfile/javaClasses.cpp.sdiff.html

Anyway this webrev is the up-to-date one including fixing the nits you pointed out. http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.04

Minor: The calls to setversion() and setbci() are the same for both alternatives and can be done just once before the if-else statement as below. With such refactoring it is clear what the common part is.

Moved.

webrev.03/hotspot/src/share/vm/prims/jvm.cpp Minor: Need spaces around the '+'. webrev.03/hotspot/src/share/vm/prims/stackwalk.cpp

I am not sure if that’s the convention applied consistently in VM. I fixed it anyway.

Minor: Indent at the line 115 must be +2.

I don’t see any indentation/formatting issue there.

360 for (int n=0; n < skipframes && !vfst.atend(); vfst.next(), n++) {

I prefer to keep n=0 and there are other places using that convention.

451 int count = framecount+startindex; Minor: Need spaces around the '=' and '+’.

Fixed.

Thanks Mandy

More information about the hotspot-runtime-dev mailing list