On Wed, 21 Sep 2022 17:25:42 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> 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);
> }
> }
I understand you question about `is_cthread_with_continuation(jt)` check.
And I've incorrectly answered you initial question about it.
It is much simpler to check if `jt->is_in_VTMS_transition()` but not tmp
transition.
I've already updated this part with renaming. Could you, please, look at it and
check if it is more clear this way?
I'll push my fix shortly.
-------------
PR: https://git.openjdk.org/jdk/pull/10321
