> On Nov 10, 2015, at 6:05 AM, Dmitry Samersoff <dmitry.samers...@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
