Code Review for JEP 259: Stack-Walking API

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 set_version() and set_bci() 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 < skip_frames && !vfst.at_end(); vfst.next(), n++) {
> 

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

>  451   int count = frame_count+start_index;
>   Minor: Need spaces around the '=' and '+’.

Fixed.

Thanks
Mandy


More information about the hotspot-runtime-dev mailing list