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

[Patricio Chilano Mateo]

On Tue, 11 Jan 2022 19:00:34 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> This change looks good. Less interactions depending on states.
>
Thanks for the review Coleen!

Patricio

> src/hotspot/share/interpreter/zero/bytecodeInterpreter.cpp line 115:
> 
>> 113: #define RETURN_SAFEPOINT                                    \
>> 114:     if (SafepointMechanism::should_process(THREAD)) {       \
>> 115:       CALL_VM(at_safepoint(THREAD), handle_exception);      \
> 
> Why wouldn't this call InterpreterRuntime::at_safepoint()?  Should it not handle single stepping?  The JVMTI code is compiled into a separate instance of the bytecodeInterpreter, but it still seems useful to call the same function.

There is already macro DEBUGGER_SINGLE_STEP_NOTIFY to handle that so I didn't want to change the behavior.

> src/hotspot/share/runtime/interfaceSupport.inline.hpp line 244:
> 
>> 242: 
>> 243: typedef ThreadBlockInVMPreprocess<> ThreadBlockInVM;
>> 244: 
> 
> I don't object to this change, but why did you do this?  To eliminate the default parameter?

Now we can use ThreadBlockInVM in wait_for_object_deoptimization(), but unlike the other uses, in here we need to allow suspends since we could be going back to Java. With the current definition of ThreadBlockInVM we would be forced to pass something for the PRE_PROC parameter which doesn't make sense. Rearranging the parameters for the ThreadBlockInVMPreprocess constructor would mess up the other callers, and creating a new constructor with default parameter allow_suspend and no PRE_PROC would be too confusing I think. So I think this is the cleanest solution. ThreadBlockInVM is just a type of ThreadBlockInVMPreprocess with no callback.

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

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