RFR (S) 8203479: JFR enabled ARM32 build assertion failure

[Boris Ulasevich]

Thank you!

On 19.06.2018 06:01, David Holmes wrote:
> Hi Boris,
> 
> Looks good.
> 
> I've pushed this with the single review as it is constrained to a 
> non-Oracle port, and is very simple.
> 
> Thanks,
> David
> 
> On 18/06/2018 7:18 PM, Boris Ulasevich wrote:
>> Hi David,
>>
>> Many thanks! Here is the werbev with updated copyrights:
>> http://cr.openjdk.java.net/~bulasevich/8203479/webrev.02
>>
>> thanks,
>> Boris
>>
>> On 18.06.2018 05:39, David Holmes wrote:
>>> Hi Boris,
>>>
>>> On 15/06/2018 8:44 PM, Boris Ulasevich wrote:
>>>> Hi,
>>>>
>>>> Please review the following patch:
>>>>    http://cr.openjdk.java.net/~bulasevich/8203479/webrev.01
>>>>    https://bugs.openjdk.java.net/browse/JDK-8203479
>>>>
>>>> Assertion fires in JFR codes on first VM thread setup because VM 
>>>> globals are not yet initialized (and supports_cx8 property is not 
>>>> predefined for ARM32 platform). I propose to exploit 
>>>> early_initialize() method to set up supports_cx8 property on early 
>>>> stage of VM initialization.
>>>
>>> Your fix looks good. Please update copyright years.
>>> Just some additional commentary ... from the bug report:
>>>
>>> First _supports_cx8 usage:
>>>    Threads::create_vm -> JavaThread::JavaThread(bool) --> 
>>> Thread::Thread() --> JfrThreadLocal::JfrThreadLocal() --> 
>>> atomic_inc(unsigned long long volatile*)
>>>
>>> I'm not sure when this was introduced but it's risky to perform 
>>> atomic operations on jlong early during VM initialization. 
>>
>> For me it comes from revision when JFR was opensourced:
>>
>> changeset:   50113:caf115bb98ad
>> user:        egahlin
>> date:        Tue May 15 20:24:34 2018 +0200
>> summary:     8199712: Flight Recorder
>>
>>> Apart from the problem you encountered with ARM32, on some platforms 
>>> it may require that the stub-generator has been initialized. (Though 
>>> in that case we may harmlessly fallback to using non-atomic 
>>> operations - which is okay when creating the initial JavaThread.)
>>
>> Good point! Yes, we discussed as an optional solution: (1) reworking 
>> jfr next_thread_id() function to increase counter without atomic_inc 
>> function call when counter value is zero, and (2) disable assertion 
>> when !is_init_completed(). We decided that for our case initialization 
>> sequence reorder works better.
>>
>>> Thanks,
>>> David
>>>
>>>> Thanks
>>>> Boris