On Wed, 30 Oct 2024 09:44:42 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Add klass_name check for is_object_wait0
>> - Fix comment in continuation.hpp
>
> src/hotspot/share/runtime/continuation.cpp line 88:
>
>> 86: if (_target->has_async_exception_condition()) {
>> 87: _failed = true;
>> 88: }
>
> Q: I wonder why the failed conditions are not checked before the
> `start_VTMS_transition()` call. At least, it'd be nice to add a comment about
> on this.
These will be rare conditions so I don't think it matters to check them before.
But I can move them to some method that we call before and after if you prefer.
> src/hotspot/share/runtime/continuation.cpp line 115:
>
>> 113: if (jvmti_present) {
>> 114: _target->rebind_to_jvmti_thread_state_of(_target->threadObj());
>> 115: if (JvmtiExport::should_post_vthread_mount()) {
>
> This has to be `JvmtiExport::should_post_vthread_unmount()` instead of
> `JvmtiExport::should_post_vthread_mount()`.
> Also, it'd be nice to add a comment explaining why the event posting is
> postponed to the `unmount` end point.
Fixed and added comment.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1823324965
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1823323891
