Code Review for JEP 259: Stack-Walking API

[serguei.spitsyn at oracle.com]

On 11/20/15 08:39, Mandy Chung wrote:
>> 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.

Thanks.


>
>> 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.

webrev.04/hotspot/src/share/vm/prims/jvm.cpp

   One place left with inconsistent formatting:

597 int limit = start_index+frame_count;

>
>>    Minor: Indent at the line 115 must be +2.
> I don’t see any indentation/formatting issue there.
    I see it fixed in new webrev. :)
>
>>   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.
   Ok.
>
>>   451   int count = frame_count+start_index;
>>    Minor: Need spaces around the '=' and '+’.
> Fixed.
Thank you for making the changes! I do not need another webrev. Amazing 
work in general! Thanks, Serguei
>
> Thanks
> Mandy