RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

[serguei.spitsyn at oracle.com]

Hi Chris,

The slowdebug minimal build does not fail without NOT_JVMTI_RETURN in 
the ServiceThread::enqueue_deferred_event().
I'm curious why and will check if it is really needed.
If so, will add it as well.

Thanks,
Serguei

On 1/6/20 8:05 PM, serguei.spitsyn at oracle.com wrote:
> Hi Chris,
>
> Good catch.
> I agree, for consistency the enqueue_event() is better to follow the 
> JvmtiDeferredEventQueue::enqueue() to avoid slowdebug minimal build 
> failures.
>
> New patch is:
>
> diff --git a/src/hotspot/share/prims/jvmtiThreadState.hpp 
> b/src/hotspot/share/prims/jvmtiThreadState.hpp
> --- a/src/hotspot/share/prims/jvmtiThreadState.hpp
> +++ b/src/hotspot/share/prims/jvmtiThreadState.hpp
> @@ -396,7 +396,7 @@
>    void set_should_post_on_exceptions(bool val) { 
> _thread->set_should_post_on_exceptions_flag(val ? JNI_TRUE : 
> JNI_FALSE); }
>
>    // Thread local event queue, which doesn't require taking the 
> Service_lock.
> -  void enqueue_event(JvmtiDeferredEvent* event);
> +  void enqueue_event(JvmtiDeferredEvent* event) NOT_JVMTI_RETURN;
>    void post_events(JvmtiEnv* env);
>    void run_nmethod_entry_barriers();
>  };
>
> Thanks,
> Serguei
>
> On 1/6/20 6:39 PM, Chris Plummer wrote:
>> Hi Serguei,
>>
>> JvmtiDeferredEventQueue::enqueue() is a NOP for minimal builds, so it 
>> can be called even for minimalVM builds:
>>
>>   void enqueue(JvmtiDeferredEvent event) NOT_JVMTI_RETURN;
>>
>> The changes for JDK-8212160 seem to have put some wrapper code around 
>> its use, resulting in ServiceThread::enqueue_deferred_event() and 
>> JvmtiThreadState::enqueue_event() being added. Shouldn't NOP 
>> implementations also have been done for them?
>>
>> thanks,
>>
>> Chris
>>
>> On 1/6/20 6:35 PM, Chris Plummer wrote:
>>> Hold your horses. I have questions. Working on them now. Please 
>>> don't push.
>>>
>>> thanks,
>>>
>>> Chris
>>>
>>> On 1/6/20 6:29 PM, coleen.phillimore at oracle.com wrote:
>>>> This looks trivial.  Thank you for fixing it.
>>>> Coleen
>>>>
>>>> On 1/6/20 9:18 PM, serguei.spitsyn at oracle.com wrote:
>>>>> Please, review a trivial fix for bug:
>>>>>   https://bugs.openjdk.java.net/browse/JDK-8236124
>>>>>
>>>>> Patch suggested by A. Shipilev:
>>>>>
>>>>> diff --git a/src/hotspot/share/code/nmethod.cpp 
>>>>> b/src/hotspot/share/code/nmethod.cpp
>>>>> --- a/src/hotspot/share/code/nmethod.cpp
>>>>> +++ b/src/hotspot/share/code/nmethod.cpp
>>>>> @@ -1601,7 +1601,7 @@
>>>>>        ServiceThread::enqueue_deferred_event(&event);
>>>>>      } else {
>>>>>        // This enters the nmethod barrier outside in the caller.
>>>>> -      state->enqueue_event(&event);
>>>>> +      JVMTI_ONLY(state->enqueue_event(&event));
>>>>>      }
>>>>>    }
>>>>>  }
>>>>>
>>>>>
>>>>> Summary:
>>>>>   The slowdebug build was broken by the fix of JDK-8212160 which 
>>>>> introduced new function: enqueue_event().
>>>>>   The fix is to call it only if the JVM TI is enabled.
>>>>>
>>>>> Testing:
>>>>>   Ran slowdebug build locally.
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>
>>>
>>>
>>
>>
>