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 core-libs-dev
mailing list