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

[Chris Plummer]

Hi Serguei,

The reason you don't see a build failure is because the implementation 
of ServiceThread::enqueue_deferred_event() is not in a JVMTI file that 
gets excluded from minimalVM builds, thus it is still callable and 
linkable. If you choose to use NOT_JVMTI_RETURN for it in the header 
file, then you will also need to #ifdef around the implementation.

Chris

On 1/7/20 9:16 AM, serguei.spitsyn at oracle.com wrote:
> 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
>>>>>
>>>>
>>>>
>>>
>>>
>>
>