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

[David Holmes]

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