On Tue, 20 Sep 2022 22:15:49 GMT, Serguei Spitsyn <[email protected]> wrote:
>> src/hotspot/share/prims/jvmtiEnvBase.cpp line 713:
>>
>>> 711: if (!jt->is_in_tmp_VTMS_transition()) {
>>> 712: jvf = check_and_skip_hidden_frames(jt, jvf);
>>> 713: }
>>
>> I think this comment needs some help. It's hard to match it up with what the
>> code is doing. I think you are saying you want to avoid skipping hidden
>> frames when in transition, although if that's the case, it's not clear to me
>> why not skipping is ok.
>>
>> Also is skipping (or not skipping) ok regardless of the
>> JvmtiEnvBase::is_cthread_with_continuation() result?
>
> Thank you for reviewing and the comment, Chris.
> I agree, this part and related comment is kind of obscure.
> I'll think how to make the comment better.
> In fact, all temporary VTMS transitions do temporary switch the `JavaThread`
> identity from virtual thread to carrier. There is no need to skip frames
> because there are no real carrier thread frames at the top. Moreover, any
> attempt to skip frames which are in transition works incorrectly and gives an
> empty stack. The check `JvmtiEnvBase::is_cthread_with_continuation()` is
> needed to make sure we have a deal with a continuation. There is no need to
> skip frames otherwise.
It's still not clear to me if the result of
`JvmtiEnvBase::is_cthread_with_continuation()` impacts if you have to possibly
skip hidden frames. In other words, do you always have to do the `if` block
that follows, no matter what `JvmtiEnvBase::is_cthread_with_continuation()`
returns? If you don't, then maybe instead of a "? :" expression, an if/else
would be better. I'm not sure if the following is correct, but something like:
javaVFrame *jvf;
if (JvmtiEnvBase::is_cthread_with_continuation(jt)) {
jvf = jt->carrier_last_java_vframe(reg_map_p);
} else {
jvf - jt->last_java_vframe(reg_map_p);
// Skipping hidden frames when jt->is_in_tmp_VTMS_transition()==true
results
// in returning jvf==NULL, and so, empty stack traces for carrier threads.
if (!jt->is_in_tmp_VTMS_transition()) {
jvf = check_and_skip_hidden_frames(jt, jvf);
}
}
-------------
PR: https://git.openjdk.org/jdk/pull/10321
