RFR: 2178143: VM crashes if the number of bound CPUs changed during runtime

[David Holmes]

On 26/03/2013 3:30 PM, Yumin Qi wrote:
> David and All,
>
> Changed as suggested by David.
>
> David, I did not move the warning into  set_parallel_gc_flags() since
> except for serial gc, all other GCs will check this flag, so I moved it
> into
> #if INCLUDE_ALL_GCS
>
> before call set GC flags.

I didn't realize all the other GCs would utilize ParallelGCThreads. Can 
we at least exclude it for serial GC case?

Otherwise looks good.

Thanks,
David

> new webrev: http://cr.openjdk.java.net/~minqi/2178143/
>
> Thanks
> Yumin
>
>
>
> On 3/25/2013 5:44 PM, David Holmes wrote:
>> Hi Yumin,
>>
>> I have a few suggested changes.
>>
>> First in os.hpp I suggest
>>
>>  return _processor_count > 1 || AssumeMP ;
>>
>> That way we don't bypass the assertion and don't encounter unnecessary
>> overhead on the common path.
>>
>> In arguments.cpp:
>>
>> +   if (AssumeMP && FLAG_IS_DEFAULT(ParallelGCThreads)) {
>> +     warning("With AssumeMP, recommend run with
>> -XX:ParallelGCThreads=<num_of_gc_threads>, where"
>> +             "  num_of_gc_threads can be set to number of cores");
>> +   }
>>
>> I'm not convinced issuing a warning is necessary or desirable here.
>> With a uniprocessor startup will we even default to using parallel GC?
>> The above should only be issued if using parallel GC - so better to
>> move it into set_parallel_gc_flags().
>>
>> In any case it isn't clear what the user should supply here. If they
>> use the maximum expected number of cores initially then while they
>> only have 1 core their VM may not even function adequately due to GC
>> overhead. I think all you can say here is something like:
>>
>> warning("If the number of processors is expected to increase from one,
>> then you should configure the number of parallel GC threads
>> appropriately using -XX:ParallelGCThreads=N");
>>
>>
>> In globals.hpp:
>>
>> +   product(bool, AssumeMP, false,       \
>> +           "Assume run on multiple processors always")       \
>>
>> Was there any discussion on making this default to true instead?
>>
>> Suggested re-wording:
>>
>>  "Instruct the VM to assume multiple processors are available"
>>
>> Thanks,
>> David
>>
>> On 26/03/2013 9:15 AM, Yumin Qi wrote:
>>> It should be "AssumeMP".
>>>
>>> /Yumin
>>>
>>> On 3/25/2013 3:32 PM, Yumin Qi wrote:
>>>> Hi,
>>>>
>>>>   New webrev to use "-XX:+AssumMP" flag. Also add warning to recommend
>>>> use this flag with "-XX:ParallelGCThreads" to remind user to avoid
>>>> running with only one GC thread. This is verified by Oleksandr with
>>>> the test case running on Linux which is not Zone configured.
>>>>
>>>>   Same link.
>>>>
>>>> Thanks
>>>> Yumin
>>>>
>>>> On 3/20/2013 2:27 PM, Yumin Qi wrote:
>>>>>
>>>>> 2178143:  VM crashes if the number of bound CPUs changed during
>>>>> runtime.
>>>>>
>>>>> Situation: Customer first configure only one CPU online and turn
>>>>> others offline to run java application, after java program started,
>>>>> bring more CPUs back online. Since VM started on a single CPU,
>>>>> os::is_MP() will return false, but after more CPUs available, OS will
>>>>> schedule the app run on multiple CPUs, this caused SEGV in various
>>>>> places where data consistency was broken. The solution is supply a
>>>>> flag to assume it is running on MP, so lock is forced to be called.
>>>>>
>>>>> http://cr.openjdk.java.net/~minqi/2178143/
>>>>>
>>>>> Thanks
>>>>> Yumin
>>>>
>>>
>