RFR: 8273143: Transition to _thread_in_vm when handling a polling page exception [v2]

[Daniel D.Daugherty]

On Mon, 10 Jan 2022 19:05:54 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Please review this small cleanup patch. Being in a state of _thread_in_vm when executing safepoint/handshake and handle_special_runtime_exit_condition() related methods is not only more appropriate, since we are already in the VM, but it also makes the code more simple and avoids some hurdles that we otherwise face in the processing logic: having to check multiple valid states for safepoint code, transition wrapper for state bookkeeping in handshake code, having to manually switch to _thread_blocked and back because we cannot use transition wrappers in wait_for_object_deoptimization(), and another switch statement based on thread state in check_and_handle_async_exceptions(). I already cleaned up most callers as part of other patches. The only one remaining which is being fixed with this patch is method handle_polling_page_exception().
>> 
>> Tested in mach5 tiers 1-6.
>> 
>> Thanks,
>> Patricio
>
> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix copyrights to 2022

Thumbs up. It took a bit of digging to determine equivalence of the
new code relative to the old code. I do have one query in there about
a call to `make_walkable()`.

It would be good to run at least one Tier7 and one Tier8 on this change.
You'll get the longer running stress tests that way.

src/hotspot/share/interpreter/zero/bytecodeInterpreter.cpp line 112:

> 110: #define RETURN_SAFEPOINT                                                                                  \
> 111:     if (SafepointMechanism::should_process(THREAD)) {                                                     \
> 112:       HandleMarkCleaner __hmc(THREAD);                                                                    \

It took me a bit to find the new HandleMarkCleaner. I found it
in VM_ENTRY_BASE which is used by JRT_ENTRY so you have
equivalence here.

src/hotspot/share/interpreter/zero/bytecodeInterpreter.cpp line 113:

> 111:     if (SafepointMechanism::should_process(THREAD)) {                                                     \
> 112:       HandleMarkCleaner __hmc(THREAD);                                                                    \
> 113:       CALL_VM(SafepointMechanism::process_if_requested_with_exit_check(THREAD, true /* check asyncs */),  \

And I found the equivalent call to `process_if_requested_with_exit_check`
in JRT_ENTRY's use of ThreadInVMfromJava. When the destructor for that
helper runs, we call `process_if_requested_with_exit_check`.

src/hotspot/share/runtime/handshake.cpp line 492:

> 490:   assert(!_handshakee->is_terminated(), "should not be a terminated thread");
> 491: 
> 492:   _handshakee->frame_anchor()->make_walkable(_handshakee);

The old code in `ThreadInVMForHandshake` protected the `make_walkable()`
call with `if (thread->has_last_Java_frame()) {`. Why/How did you decide that
you no longer need that check?

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

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7009