On Thu, 4 May 2023 01:44:36 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> refactored.
>
> It'd be nice to do even more factoring + renaming.
> The lines 2326-2345 can be refactored to a function:
>
> bool StackRootCollector::report_native_frame_refs(jmethodID method) {
> _blk->set_context(_thread_tag, _tid, _depth, method);
> if (_is_top_frame) {
> // JNI locals for the top frame.
> assert(_java_thread != nullptr, "sanity");
> _java_thread->active_handles()->oops_do(_blk);
> if (_blk->stopped()) {
> return false;
> }
> } else {
> if (_last_entry_frame != nullptr) {
> // JNI locals for the entry frame
> assert(_last_entry_frame->is_entry_frame(), "checking");
>
> _last_entry_frame->entry_frame_call_wrapper()->handles()->oops_do(_blk);
> if (_blk->stopped()) {
> return false;
> }
> }
> }
> return true;
> }
>
>
> The function `report_stack_refs` can be renamed to `report_java_frame_refs`
> to make function name more consistent.
JNI local reporting uses this tricky _is_top_frame/_last_entry_frame stuff
I think it would be better to have it in the main do_frame method for better
readability
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1185504637
