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

[serguei.spitsyn at oracle.com]

Chris,

The macro NOT_JVMTI_RETURN is never used outside of the prims/ folder.
Also, there is more work to get rid of the JVMTI code in the ServiceThread.
I don't know how important is this for the minimal build.
So, I'd leave it alone for now and just fix the build issue.

Thanks,
Serguei

On 1/7/20 10:04 AM, Chris Plummer wrote:
> 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
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
>