RFR: 8299414: JVMTI FollowReferences should support references from VirtualThread stack [v9]

[Serguei Spitsyn]

On Thu, 4 May 2023 20:55:46 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

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

Sorry, I do not see how this improves readability.
Big functions with many layered conditions do not improve readability.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1185718941