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

Tue Mar 26 19:44:35 UTC 2013

Sorry, copied wrong link. It is at same link.

http://cr.openjdk.java.net/~minqi/2178143/

Thanks
Yumin

On 3/26/2013 12:42 PM, Daniel D. Daugherty wrote:
>
> On 3/26/13 1:17 PM, Yumin Qi wrote:
>> Hi, Jon
>>
>>   I made a change to the warning condition and move it after GC flags 
>> are set. I think at this time, ParallelGCThreads has a value != 0. 
>> Please take a look if it is right.  The This is the new webrev:
>>
>>   http://javaweb.us.oracle.com/~yqi/webrev/2178143/
>
> This webrev isn't accessible outside Oracle...
>
> Dan
>
>
>>
>>   Tested with platform of ncpus > 1 and no ParallelGCThreads set with 
>> turning on AssumeMP.  The warning is gone.
>>    Also tested with ncpus = 1 and the warning is on too.
>>
>> Thanks
>> Yumin
>>
>> On 3/26/2013 8:03 AM, Jon Masamitsu wrote:
>>> If I set AssumeMP on the command line and nothing else
>>> and am running on a platform where I get ParallelGCThreads > 1,
>>> am I going to see the warning?    If yes, that seems too intrusive.
>>> I can see users hearing about the flag and deciding to include
>>> it always just to be safe.
>>>
>>> Jon
>>>
>>> On 3/25/13 11:21 PM, Yumin Qi wrote:
>>>> You are right, thanks!  I misread the INCLUDE.
>>>> new web in the same link.
>>>>
>>>> Thanks
>>>> Yumin
>>>>
>>>> On 3/25/2013 11:00 PM, David Holmes wrote:
>>>>> On 26/03/2013 3:56 PM, Yumin Qi wrote:
>>>>>> David,
>>>>>>
>>>>>> On 3/25/2013 10:41 PM, David Holmes wrote:
>>>>>>> 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?
>>>>>>>
>>>>>>
>>>>>> Serial GC is set here:
>>>>>
>>>>> INCLUDE_ALL_GCS allows all GCs including Serial GC. Otherwise we 
>>>>> only have SerialGC. So you have excluded it for the case where 
>>>>> SerialGC is the only GC built into the VM, but not for the case 
>>>>> where all GCs are available and the user requests UseSerialGC.
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>
>>>>>> 3253 #if !INCLUDE_ALL_GCS
>>>>>> 3254   force_serial_gc();
>>>>>> 3255 #endif // INCLUDE_ALL_GCS
>>>>>>
>>>>>> So after it moved into
>>>>>> 3283 #if INCLUDE_ALL_GCS
>>>>>> 3284   if (AssumeMP && FLAG_IS_DEFAULT(ParallelGCThreads)) {
>>>>>> 3285     warning("If the number of processors is expected to 
>>>>>> increase
>>>>>> from one, then"
>>>>>> 3286             " you should configure the number of parallel GC
>>>>>> threads appropriately"
>>>>>> 3287             " using -XX:ParallelGCThreads=N");
>>>>>> 3288   }
>>>>>> 3289   // Set per-collector flags
>>>>>> 3290   if (UseParallelGC || UseParallelOldGC) {
>>>>>> 3291     set_parallel_gc_flags();
>>>>>> 3292   } else if (UseConcMarkSweepGC) { // should be done before 
>>>>>> ParNew
>>>>>> check below
>>>>>> 3293     set_cms_and_parnew_gc_flags();
>>>>>> 3294   } else if (UseParNewGC) {  // skipped if CMS is set above
>>>>>> 3295     set_parnew_gc_flags();
>>>>>> 3296   } else if (UseG1GC) {
>>>>>> 3297     set_g1_gc_flags();
>>>>>> 3298   }
>>>>>> 3299   check_deprecated_gcs();
>>>>>> 3300 #else // INCLUDE_ALL_GCS
>>>>>> 3301   assert(verify_serial_gc_flags(), "SerialGC unset");
>>>>>> 3302 #endif // INCLUDE_ALL_GCS
>>>>>>
>>>>>> It is excluded for serial GC.
>>>>>>
>>>>>> Thanks
>>>>>> Yumin
>>>>>>
>>>>>>> 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
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>
>>
>