Code Review for JEP 259: Stack-Walking API
Mandy Chung
mandy.chung at oracle.com
Wed Nov 11 19:32:23 UTC 2015
> On Nov 10, 2015, at 6:05 AM, Dmitry Samersoff <dmitry.samersoff at oracle.com> wrote:
>
> Mandy,
>
> Native part looks good for me.
>
> javaClasses.cpp:
>
> 1. It might be good to create a helper inline function for
>
> int bci_version = stackFrame->int_field(bci_offset);
> int version = BackTrace::version_at(bci_version);
>
Coleen gave me some suggestion to use injected fields. This will be changed in the next revision.
> 2. java_lang_StackFrameInfo::fill_methodInfo
>
> CHECK macro left methodInfo partially initialized, not sure it's OK.
>
Any exception here will cause StackWalker::walk to fail. I expect the exception thrown here would be OOME and other internal errors. So it should be okay.
> javaClasses.inline.hpp:
>
> 78 You can use the same pattern as in assert:
>
> if ((jushort)version != version) version = USHRT_MAX;
>
> 117 Is it possible to add comment about +1000000 magic?
>
This is existing code refactored from javaClasses.hpp. I don’t know why it added +1000000. For hidden frames, keeping line number to -1 is good to me since it means unavailable.
Coleen - do you know why this is done this way?
> jvm.cpp:
>
> 627 missed space around =
Fixed.
Mandy
More information about the core-libs-dev
mailing list