[10] RFR(S): 8185572: Enable AssumeMP by default on SPARC machines

[David Holmes]

Hi Poonam,

On 11/08/2017 3:44 AM, Poonam Parhar wrote:
> Thanks Vladimir.
> 
> Since the SPARC machines are always multi-cores, we can safely set AssumeMP to true on these.

I'm still unclear about the reported problem here. As Vladimir pointed 
out in the bug report the is_MP checks uses _processor_count which is 
set to the number of cpus on the machine _not_ the number of cpus 
currently available to the VM.

void os::Solaris::initialize_system_info() {
   set_processor_count(sysconf(_SC_NPROCESSORS_CONF));

So all this discussion about containers and dynamic changes to available 
cpus should be moot. So the only way this can fail is if the number of 
configured processors on the machine dynamically changed _and_ it was 
initially 1 - which seems to me to be impossible with sparc unless the 
hardware info is being incorrectly reported (ie virtualization bug?)

That aside I have no issue with the fix as we will likely be assuming MP 
always in the future.

Thanks,
David

> Adding my comments from the previous mail here again for better readability:
> -------------------------------------
> Bug: https://bugs.openjdk.java.net/browse/JDK-8185572: Enable AssumeMP by default on SPARC machines
> Webrev: http://cr.openjdk.java.net/~poonam/8185572/webrev.00/
> 
> This change enables AssumeMP by default on SPARC machines. On Sparc T7, to finalize BIS instructions the server compiler needs to add a 'membar' instruction at the end. But the generation of 'membar' is guarded by os::is_MP(), and os::is_MP() returns false when there is a single cpu available on the system. Now, in virtualized/container environments, the number of processors allocated to a virtual machine can dynamically change during the application runtime. That could lead to incorrect generation of BIS instructions and can cause JVM crashes. Enabling AssumeMP makes is_MP() always return true on SPARC systems.
> 
> In future, we may consider making generation of 'membar' unconditional with the enhancement request: https://bugs.openjdk.java.net/browse/JDK-8150715.
> 
> Thanks,
> Poonam
> 
> 
>> -----Original Message-----
>> From: Vladimir Kozlov
>> Sent: Thursday, August 10, 2017 9:47 AM
>> To: Poonam Parhar; hotspot-compiler-dev at openjdk.java.net
>> Cc: hotspot-runtime-dev at openjdk.java.net runtime
>> Subject: Re: [10] RFR(S): 8185572: Enable AssumeMP by default on SPARC
>> machines
>>
>> CCing to Runtime.
>>
>> Can you add comment explaining why it set to true on SPARC?
>>
>> Thanks,
>> Vladimir
>>
>> On 8/10/17 6:26 AM, Poonam Parhar wrote:
>>> Hello,
>>>
>>> Please review this simple patch:
>>>
>>> Bug:_JDK-8185572_<https://bugs.openjdk.java.net/browse/JDK-
>> 8185572>:En
>>> able
>>> AssumeMP by default on SPARC machines
>>>
>>> Webrev:http://cr.openjdk.java.net/~poonam/8185572/webrev.00/
>>>
>>> This change enables AssumeMP by default on SPARC machines. On Sparc
>>> T7, to finalize BIS instructions the server compiler needs toadd
>>> a'membar'instruction at the end.But the generation of'membar'is
>>> guarded byos::is_MP(), andos::is_MP()returns false when there isa
>>> singlecpu available on the system.
>>> Now,invirtualized/containerenvironments, the number
>>> ofprocessorsallocated to a virtual machine can dynamically change
>>> during the application runtime.That could lead to incorrect
>> generation
>>> of BIS instructions and can cause JVM crashes.Enabling AssumeMP makes
>>> is_MP() always return true on SPARC systems.
>>>
>>> In future, we may consider makinggeneration of'membar'unconditional
>>> withtheenhancementrequest:_JDK-
>> 8150715_<https://bugs.openjdk.java.net/browse/JDK-8150715>.
>>>
>>> Thanks,
>>>
>>> Poonam
>>>